[Qemu-devel] [Bug 504368] Re: sdl window intermittently scales instead of resizing

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  sdl window intermittently scales instead of resizing

Status in QEMU:
  Expired
Status in qemu-kvm package in Ubuntu:
  Expired

Bug description:
  Binary package hint: qemu-kvm

  Normally, the SDL output window for a VM resizes to match the VM's
  resolution.  However, intermittently the output is instead scaled
  within the window.  I can't seem to find any pattern to when the
  output is scaled versus when the window is resized.  I would prefer
  that the window be resized as needed to display the VM in a 1:1
  manner.

  ProblemType: Bug
  Architecture: amd64
  Date: Thu Jan  7 10:30:10 2010
  DistroRelease: Ubuntu 9.10
  InstallationMedia: Ubuntu 9.10 "Karmic Koala" - Release amd64 (20091027)
  KvmCmdLine:
   UIDPID  PPID  CSZ   RSS PSR STIME TTY  TIME CMD
   root 27618 1 38 241752 804668 1 10:05 ?00:09:39 /usr/bin/kvm 
-S -M pc-0.11 -cpu qemu32 -m 768 -smp 1 -name win2k3 -uuid 
da414aa0-f18a-7a02-3d1b-1dbf13137bc9 -monitor 
unix:/var/run/libvirt/qemu/win2k3.monitor,server,nowait -localtime -boot c 
-drive file=/media/qpc-devel/testing/win2k3/testing.ovl,if=ide,index=0,boot=on 
-drive 
file=/media/qpc-devel/testing/win2k3/../../isos/en_win_srv_2003_r2_standard_cd1.iso,if=ide,media=cdrom,index=2
 -net nic,macaddr=00:16:3e:d6:f5:60,vlan=0,model=ne2k_pci,name=ne2k_pci.0 -net 
tap,fd=18,vlan=0,name=tap.0 -serial pty -parallel none -usb -usbdevice tablet 
-vga cirrus
   root 28306 1 54 177732 545520 1 10:28 ?00:00:49 /usr/bin/kvm 
-S -M pc-0.11 -cpu qemu32 -m 512 -smp 1 -name win2k -uuid 
153d6125-acb5-70bc-c7d2-bcbf87c5be86 -monitor 
unix:/var/run/libvirt/qemu/win2k.monitor,server,nowait -localtime -boot c 
-drive file=/media/qpc-devel/testing/win2k/testing.ovl,if=ide,index=0,boot=on 
-drive 
file=/media/qpc-devel/testing/win2k/../../isos/windows_2000.iso,if=ide,media=cdrom,index=2
 -net nic,macaddr=68:29:6b:13:50:c6,vlan=0,model=ne2k_pci,name=ne2k_pci.0 -net 
tap,fd=19,vlan=0,name=tap.0 -serial pty -parallel none -usb -usbdevice tablet 
-vga cirrus
  NonfreeKernelModules: nvidia
  Package: kvm 1:84+dfsg-0ubuntu16+0.11.0+0ubuntu6.3
  PccardctlIdent:
   Socket 0:
 no product info available
  PccardctlStatus:
   Socket 0:
 no card
  ProcCmdLine: BOOT_IMAGE=/boot/vmlinuz-2.6.31-16-generic 
root=UUID=30218f9a-6f90-4eab-9ba5-f54897e842cb ro quiet splash
  ProcEnviron:
   PATH=(custom, user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcVersionSignature: Ubuntu 2.6.31-16.53-generic
  SourcePackage: qemu-kvm
  Uname: Linux 2.6.31-16-generic x86_64
  dmi.bios.date: 02/20/2008
  dmi.bios.vendor: LENOVO
  dmi.bios.version: 7LETB2WW (2.12 )
  dmi.board.vendor: LENOVO
  dmi.board.version: Not Available
  dmi.chassis.asset.tag: No Asset Information
  dmi.chassis.type: 10
  dmi.chassis.vendor: LENOVO
  dmi.chassis.version: Not Available
  dmi.modalias: 
dmi:bvnLENOVO:bvr7LETB2WW(2.12):bd02/20/2008:svnLENOVO:pn:pvrThinkPadT61p:rvnLENOVO:rn:rvrNotAvailable:cvnLENOVO:ct10:cvrNotAvailable:
  dmi.product.version: ThinkPad T61p
  dmi.sys.vendor: LENOVO

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

[Qemu-devel] [Bug 1313816] Re: qemu should close sound device when no more needs.

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  qemu should close sound device when no more needs.

Status in QEMU:
  Expired

Bug description:
  I use alsa directly or via pulseaudio on qemu.
  And I use xmms2 as well as qemu.

  When I use alsa, one of xmms2 or qemu can play sound.
  When I use pulseaudio with qemu and pulseaudio -k, and pulseaudio --start,
  qemu can't play sound.

  I think that:
  - qemu should open sound device when needs.
  - qemu should close sound device when no more needs.

  One of xmms2 or qemu can play sound, but both of them rarely play sound
  at the same time.
  qemu occurs error on pulseaudio -k, but once close and open the device,
  the error will be recovered, I hope.

  Host: slackware64 14.1, linux kernel 3.14.2
  Qemu: 2.0.0
  QEMU_AUDIO_DRV=pa /usr/local/bin/qemu-system-x86_64 -enable-kvm -hda 
/dosc/win8_x64.img -soundhw hda -boot c -m 2G -cpu host -usb -usbdevice tablet 
-display sdl -rtc base=localtime
  Guest: Windows 8.1 x64

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

[Qemu-devel] [Bug 504368] Re: sdl window intermittently scales instead of resizing

[Launchpad Bug Tracker]

[Expired for qemu-kvm (Ubuntu) because there has been no activity for 60
days.]

** Changed in: qemu-kvm (Ubuntu)
   Status: Incomplete => Expired

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

Title:
  sdl window intermittently scales instead of resizing

Status in QEMU:
  Expired
Status in qemu-kvm package in Ubuntu:
  Expired

Bug description:
  Binary package hint: qemu-kvm

  Normally, the SDL output window for a VM resizes to match the VM's
  resolution.  However, intermittently the output is instead scaled
  within the window.  I can't seem to find any pattern to when the
  output is scaled versus when the window is resized.  I would prefer
  that the window be resized as needed to display the VM in a 1:1
  manner.

  ProblemType: Bug
  Architecture: amd64
  Date: Thu Jan  7 10:30:10 2010
  DistroRelease: Ubuntu 9.10
  InstallationMedia: Ubuntu 9.10 "Karmic Koala" - Release amd64 (20091027)
  KvmCmdLine:
   UIDPID  PPID  CSZ   RSS PSR STIME TTY  TIME CMD
   root 27618 1 38 241752 804668 1 10:05 ?00:09:39 /usr/bin/kvm 
-S -M pc-0.11 -cpu qemu32 -m 768 -smp 1 -name win2k3 -uuid 
da414aa0-f18a-7a02-3d1b-1dbf13137bc9 -monitor 
unix:/var/run/libvirt/qemu/win2k3.monitor,server,nowait -localtime -boot c 
-drive file=/media/qpc-devel/testing/win2k3/testing.ovl,if=ide,index=0,boot=on 
-drive 
file=/media/qpc-devel/testing/win2k3/../../isos/en_win_srv_2003_r2_standard_cd1.iso,if=ide,media=cdrom,index=2
 -net nic,macaddr=00:16:3e:d6:f5:60,vlan=0,model=ne2k_pci,name=ne2k_pci.0 -net 
tap,fd=18,vlan=0,name=tap.0 -serial pty -parallel none -usb -usbdevice tablet 
-vga cirrus
   root 28306 1 54 177732 545520 1 10:28 ?00:00:49 /usr/bin/kvm 
-S -M pc-0.11 -cpu qemu32 -m 512 -smp 1 -name win2k -uuid 
153d6125-acb5-70bc-c7d2-bcbf87c5be86 -monitor 
unix:/var/run/libvirt/qemu/win2k.monitor,server,nowait -localtime -boot c 
-drive file=/media/qpc-devel/testing/win2k/testing.ovl,if=ide,index=0,boot=on 
-drive 
file=/media/qpc-devel/testing/win2k/../../isos/windows_2000.iso,if=ide,media=cdrom,index=2
 -net nic,macaddr=68:29:6b:13:50:c6,vlan=0,model=ne2k_pci,name=ne2k_pci.0 -net 
tap,fd=19,vlan=0,name=tap.0 -serial pty -parallel none -usb -usbdevice tablet 
-vga cirrus
  NonfreeKernelModules: nvidia
  Package: kvm 1:84+dfsg-0ubuntu16+0.11.0+0ubuntu6.3
  PccardctlIdent:
   Socket 0:
 no product info available
  PccardctlStatus:
   Socket 0:
 no card
  ProcCmdLine: BOOT_IMAGE=/boot/vmlinuz-2.6.31-16-generic 
root=UUID=30218f9a-6f90-4eab-9ba5-f54897e842cb ro quiet splash
  ProcEnviron:
   PATH=(custom, user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  ProcVersionSignature: Ubuntu 2.6.31-16.53-generic
  SourcePackage: qemu-kvm
  Uname: Linux 2.6.31-16-generic x86_64
  dmi.bios.date: 02/20/2008
  dmi.bios.vendor: LENOVO
  dmi.bios.version: 7LETB2WW (2.12 )
  dmi.board.vendor: LENOVO
  dmi.board.version: Not Available
  dmi.chassis.asset.tag: No Asset Information
  dmi.chassis.type: 10
  dmi.chassis.vendor: LENOVO
  dmi.chassis.version: Not Available
  dmi.modalias: 
dmi:bvnLENOVO:bvr7LETB2WW(2.12):bd02/20/2008:svnLENOVO:pn:pvrThinkPadT61p:rvnLENOVO:rn:rvrNotAvailable:cvnLENOVO:ct10:cvrNotAvailable:
  dmi.product.version: ThinkPad T61p
  dmi.sys.vendor: LENOVO

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

[Qemu-devel] [Bug 1617114] Re: Qemu 2.6.0 freezes with windows guests

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Qemu 2.6.0 freezes with windows guests

Status in QEMU:
  Expired

Bug description:
  When launching qemu with the same command line as before 2.6.0, with
  SDL display, with virtio, for a win-10 guest:

  qemu-system-x86_64 -enable-kvm -name win-10 -machine type=pc,accel=kvm
  -cpu host -smp cores=1,threads=2,sockets=1 -m 2.7G -balloon virtio
  -drive
  file=/home//.qemu/imgs/win10-coe.qcow2,index=0,media=disk,if=virtio
  -drive file=/usr/share/virtio/virtio-win.iso,index=1,media=cdrom
  -drive file=/usr/share/spice-guest-tools/spice-guest-
  tools.iso,index=2,media=cdrom -net nic,model=virtio -net
  tap,ifname=tap0,script=no,downscript=no,vhost=on -usbdevice tablet
  -usb -display sdl -vga qxl -soundhw ac97 -rtc base=localtime
  -usbdevice host:0b0e:0032 -usbdevice host:0b0e:0348 -usbdevice
  host:0b0e:0410

  Qemu at some point just freezes with no error message at all with
  newer version 2.6.0-1.

  Reverting to prior version 2.5.1-1, things go back to normal.

  A simple way to accelerate the freeze is to have qemu launch in a
  workspace/desktop, and then move to a different workspace/desktop, and
  then move back to the qemu workspace/desktop, and you'll find out it's
  frozen.

  BTW, there's no way to get into qemu monitor mode terminal at all once
  frozen. The monitor terminal shows up, but does nothing...

  Perhaps it's useful to notice that I have up to date win-10 virtio
  drivers for ethernet, scsi/storage, qxl-dod, balloon, and serial
  interface drivers. The ISO version used is 0.1.118.1 (virtio-win AUR
  package).

  Using the standard (std) qemu video driver, rather than the qxl-dod
  one makes no difference BTW.

  Just in case, running on up to date x86-64 Arch, plain qemu command
  line.

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

[Qemu-devel] [Bug 1730099] Re: Sometimes, when not touching the SDL window, the guest freezes

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Sometimes, when not touching the SDL window, the guest freezes

Status in QEMU:
  Expired

Bug description:
  I often just run some development guest machine, and leave its SDL
  window on a workspace I don’t touch, and only interact with it via
  TCP.

  And sometimes, the guest just freezes.

  After it gets the focus back, it comes back to life (starts responding
  via network).

  QEMU release version: 2.8.1.1

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

[Qemu-devel] [Bug 1730101] Re: The guest is only starting after its SDL window gets focus

[Launchpad Bug Tracker]

[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  The guest is only starting after its SDL window gets focus

Status in QEMU:
  Expired

Bug description:
  I’m using i3wm and have workspace assigning rules that make QEMU’s SDL
  window be assigned to a workspace I don’t really switch to.

  When I run start a guest machine, its SDL window is moved to that
  workspace (I never see it); but the machine freezes after displaying
  that black window. It only starts booting after I switch to the
  workspace and view the window.

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

[Qemu-devel] Question about wrong ram-node0 reference

[liujunjie (A)]

Hi, I have met a problem:

The QEMU version is 2.8.1, the virtual machine is configured with 1G huge 
pages, two NUMA nodes and four pass-through NVME SSDs.

After we started the VM, in addition to some QMP queries nothing more has been 
done, the QEMU aborted after some months later.
After that, the VM is restarted, and the problem does not reproduce yet.
And The backtrace of the RCU thread is as follows:
(gdb) bt
#0  0x7fd2695f0197 in raise () from /usr/lib64/libc.so.6
#1  0x7fd2695f1888 in abort () from /usr/lib64/libc.so.6
#2  0x7fd2695e9206 in __assert_fail_base () from /usr/lib64/libc.so.6
#3  0x7fd2695e92b2 in __assert_fail () from /usr/lib64/libc.so.6
#4  0x00476a84 in memory_region_finalize (obj=)
at /home/abuild/rpmbuild/BUILD/qemu-kvm-2.8.1/memory.c:1512
#5  0x00763105 in object_deinit (obj=obj@entry=0x1dc1fd0,
type=type@entry=0x1d065b0) at qom/object.c:448
#6  0x00763153 in object_finalize (data=0x1dc1fd0) at qom/object.c:462
#7  0x007627cc in object_property_del_all (obj=obj@entry=0x1dc1f70)
at qom/object.c:399
#8  0x00763148 in object_finalize (data=0x1dc1f70) at qom/object.c:461
#9  0x00764426 in object_unref (obj=) at qom/object.c:897
#10 0x00473b6b in memory_region_unref (mr=)
at /home/abuild/rpmbuild/BUILD/qemu-kvm-2.8.1/memory.c:1560
#11 0x00473bc7 in flatview_destroy (view=0x7fc188b9cb90)
at /home/abuild/rpmbuild/BUILD/qemu-kvm-2.8.1/memory.c:289
#12 0x00843be0 in call_rcu_thread (opaque=)
at util/rcu.c:279
#13 0x008325c2 in qemu_thread_start (args=args@entry=0x1d00810)
at util/qemu_thread_posix.c:496
#14 0x7fd269983dc5 in start_thread () from /usr/lib64/libpthread.so.0
#15 0x7fd2696b27bd in clone () from /usr/lib64/libc.so.6

In this core, I found that the reference of "/objects/ram-node0"( the type of 
ram-node0 is struct "HostMemoryBackendFile") equals to 0 , while the reference 
of "/objects/ram-node1" equals to 129, more details can be seen at the end of 
this email.

I searched through the community, and found a case that had the same error 
report: https://mail.coreboot.org/pipermail/seabios/2017-September/011799.html
However, I did not configure pcie_pci_bridge. Besides, qemu aborted in device 
initialization phase in this case.

Also, I try to find out which can reference "/objects/ram-node0" so as to look 
for the one that may un reference improperly, most of them lie in the function 
of "render_memory_region" or "phys_section_add" when memory topology changes.
Later, the temporary flatviews are destroyed by RCU thread, so un reference 
happened and the backtrace is similar to the one shown above.
But I am not familiar with the detail of these process, it is hard to keep 
trace of these memory topology changes.

My question is:
How can ram-node0's reference comes down to 0 when the virtual machine is still 
running?

Maybe someone who is familiar with memory_region_ref or memory-backend-file can 
help me figure out.
Any idea is appreciated.

---
(gdb) p *((HostMemoryBackendFile *) 0x1dc1f70)
$24 = {parent_obj = {parent = {class = 0x1d70880, free = 0x7fd26a812580 
, properties = 0x1db7920, ref = 0, parent = 0x1da9710}, size = 
68719476736, merge = true, dump = false, prealloc = true, force_prealloc = 
false, is_mapped = true, host_nodes = {1, 0, 0}, policy = HOST_MEM_POLICY_BIND, 
mr = {parent_obj = {class = 0x1d6d790, free = 0x0, properties = 0x1db79e0, ref 
= 0, parent = 0x0}, romd_mode = true, ram = true, subpage = false, readonly = 
false, rom_device = false, flush_coalesced_mmio = false, global_locking = true, 
dirty_log_mask = 0 '\000', ram_block = 0x1dc2960, owner = 0x1dc1f70, iommu_ops 
= 0x0, ops = 0xcb0fe0 , opaque = 0x0, container = 
0x200d4c0, size = 0x0010, addr = 0, destructor = 
0x470800 , align = 1073741824, terminates = true, 
ram_device = false, enabled = true, warning_printed = false, vga_logging_count 
= 0 '\000', alias = 0x0, alias_offset = 0, priority = 0, subregions = 
{tqh_first = 0x0, tqh_last = 0x1dc2078}, subregions_link = {tqe_next = 0x0, 
tqe_prev = 0x1dc2c68}, coalesced = {tqh_first = 0x0, tqh_last = 0x1dc2098}, 
name = 0x1dc27a0 "/objects/ram-node0", ioeventfd_nb = 0, ioeventfds = 0x0, 
iommu_notify = {lh_first = 0x0}, iommu_notify_flags = IOMMU_NOTIFIER_NONE}}, 
share = true, mem_path = 0x1dc2350 
"/dev/hugepages/libvirt/qemu/118-instance-00025bf8"}

(gdb) p *((HostMemoryBackendFile *) 0x1dc2b50)
$205 = {parent_obj = {parent = {class = 0x1d70880, free = 0x7fd26a812580 
, properties = 0x1db7a40, ref = 129, parent = 0x1da9710}, size = 
68719476736, merge = true, dump = false, prealloc = true, force_prealloc = 
false, is_mapped = true, host_nodes = {2, 0, 0}, policy = HOST_MEM_POLICY_BIND, 
mr = {parent_obj = {class = 0x1d6d790, free = 0x0, properties = 0x1db7aa0, ref 
= 1, parent = 0x1dc2b50}, romd_mode = true, ram = true, subpage = false, 
readonly = false, rom_device = false, 

[Qemu-devel] [PATCH 1/2] Implement Floating Point flag Fraction Rounded

[John Arbuckle]

Signed-off-by: John Arbuckle 
---
 fpu/softfloat.c   | 15 ---
 include/fpu/softfloat-types.h |  1 +
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 2ba36ec370..ac34f6a2de 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -702,7 +702,7 @@ static FloatParts round_canonical(FloatParts p, 
float_status *s,
 const uint64_t roundeven_mask = parm->roundeven_mask;
 const int exp_max = parm->exp_max;
 const int frac_shift = parm->frac_shift;
-uint64_t frac, inc;
+uint64_t frac, inc, rounded;
 int exp, flags = 0;
 bool overflow_norm;
 
@@ -744,7 +744,12 @@ static FloatParts round_canonical(FloatParts p, 
float_status *s,
 if (likely(exp > 0)) {
 if (frac & round_mask) {
 flags |= float_flag_inexact;
-frac += inc;
+rounded = frac + inc;
+if ((rounded ^ frac) & frac_lsb) {
+flags |= float_flag_rounded;
+}
+frac = rounded;
+
 if (frac & DECOMPOSED_OVERFLOW_BIT) {
 frac >>= 1;
 exp++;
@@ -793,7 +798,11 @@ static FloatParts round_canonical(FloatParts p, 
float_status *s,
 break;
 }
 flags |= float_flag_inexact;
-frac += inc;
+rounded = frac + inc;
+if ((rounded ^ frac) & frac_lsb) {
+flags |= float_flag_rounded;
+}
+frac = rounded;
 }
 
 exp = (frac & DECOMPOSED_IMPLICIT_BIT ? 1 : 0);
diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index 2aae6a89b1..bee576e0fd 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -147,6 +147,7 @@ enum {
 
 enum {
 float_flag_invalid   =  1,
+float_flag_rounded   =  2,
 float_flag_divbyzero =  4,
 float_flag_overflow  =  8,
 float_flag_underflow = 16,
-- 
2.14.3 (Apple Git-98)

[Qemu-devel] [PATCH 0/2] Implement PowerPC FPSCR flag Fraction Rounded

[John Arbuckle]

In IEEE 754 math, the arithmetic, rounding, and conversion instructions produce 
an intermediate result that can be regarded as having infinite precision and 
unbounded exponent range. When the final result has its fraction part 
incremented is when the Fraction Rounded bit is set.

This patch implements the PowerPC FPSCR flag Fraction Rounded.

Note: there are still functions in softfloat that need to be adjusted so that 
float_flag_rounded is fully supported. These include round_to_int(), and all 
legacy roundAndPack* functions. So basically anywhere that sets the 
float_flag_inexact.

John Arbuckle (2):
  Implement Floating Point flag Fraction Rounded
  Implement the PowerPC Floating Point Status and Control Register
Fraction Rounded bit

 fpu/softfloat.c   | 15 ---
 include/fpu/softfloat-types.h |  1 +
 target/ppc/fpu_helper.c   |  4 
 3 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.14.3 (Apple Git-98)

[Qemu-devel] [PATCH 2/2] Implement the PowerPC Floating Point Status and Control Register Fraction Rounded bit

[John Arbuckle]

Signed-off-by: John Arbuckle 
---
 target/ppc/fpu_helper.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 0b7308f539..0baf1ce8e4 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -630,6 +630,10 @@ static void do_float_check_status(CPUPPCState *env, 
uintptr_t raddr)
 env->fpscr &= ~(1 << FPSCR_FI); /* clear the FPSCR[FI] bit */
 }
 
+/* Set or clear the Fraction Rounded bit */
+env->fpscr = deposit32(env->fpscr, FPSCR_FR, 1,
+   (status & float_flag_rounded) != 0);
+
 if (cs->exception_index == POWERPC_EXCP_PROGRAM &&
 (env->error_code & POWERPC_EXCP_FP)) {
 /* Differred floating-point exception after target FPR update */
-- 
2.14.3 (Apple Git-98)

[Qemu-devel] [RFC v1 01/23] target/riscv: Don't set write permissions on dirty PTEs

[Alistair Francis]

Setting write permission on dirty PTEs results in userspace inside a
Hypervisor guest (VU) becoming corrupted. This appears to be becuase it
ends up with write permission in the second stage translation in cases
where we aren't doing a store.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b1bee3d45d..872835177a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -326,10 +326,8 @@ restart:
 if ((pte & PTE_X)) {
 *prot |= PAGE_EXEC;
 }
-/* add write permission on stores or if the page is already dirty,
-   so that we TLB miss on later writes to update the dirty bit */
-if ((pte & PTE_W) &&
-(access_type == MMU_DATA_STORE || (pte & PTE_D))) {
+/* add write permission on stores */
+if ((pte & PTE_W) && (access_type == MMU_DATA_STORE)) {
 *prot |= PAGE_WRITE;
 }
 return TRANSLATE_SUCCESS;
-- 
2.21.0

[Qemu-devel] [RFC v1 03/23] target/riscv: Add the virtulisation mode

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h|  4 
 target/riscv/cpu_bits.h   |  6 ++
 target/riscv/cpu_helper.c | 23 +++
 3 files changed, 33 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3337d1aef3..de4843b879 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -132,6 +132,8 @@ struct CPURISCVState {
 
 #ifndef CONFIG_USER_ONLY
 target_ulong priv;
+/* This contains QEMU specific information about the virt state. */
+target_ulong virt;
 target_ulong resetvec;
 
 target_ulong mhartid;
@@ -278,6 +280,8 @@ void riscv_cpu_do_interrupt(CPUState *cpu);
 int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
+bool riscv_cpu_virt_enabled(CPURISCVState *env);
+void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
 hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index dc9d53d4be..07c95e8d2c 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -417,6 +417,12 @@
 #define PRV_H 2 /* Reserved */
 #define PRV_M 3
 
+/* Virtulisation modes */
+#define VIRT_OFF0
+#define VIRT_ON 1
+#define VIRT_MODE_SHIFT 0
+#define VIRT_MODE_MASK  (1 << VIRT_MODE_SHIFT)
+
 /* RV32 satp CSR field masks */
 #define SATP32_MODE 0x8000
 #define SATP32_ASID 0x7fc0
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 872835177a..5912ae63b7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -71,6 +71,29 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
 
 #if !defined(CONFIG_USER_ONLY)
 
+bool riscv_cpu_virt_enabled(CPURISCVState *env)
+{
+bool tmp;
+
+if (!riscv_has_ext(env, RVH)) {
+return false;
+}
+
+tmp = (env->virt & VIRT_MODE_MASK) >> VIRT_MODE_SHIFT;
+
+return tmp == VIRT_ON;
+}
+
+void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
+{
+if (!riscv_has_ext(env, RVH)) {
+return;
+}
+
+env->virt &= ~VIRT_MODE_MASK;
+env->virt |= enable << VIRT_MODE_SHIFT;
+}
+
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
 {
 CPURISCVState *env = >env;
-- 
2.21.0

[Qemu-devel] [RFC v1 05/23] target/riscv: Add the Hypervisor CSRs to CPUState

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h | 17 +
 1 file changed, 17 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index eeb3756c91..b99d2b7af2 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -169,12 +169,29 @@ struct CPURISCVState {
 target_ulong mcause;
 target_ulong mtval;  /* since: priv-1.10.0 */
 
+/* Hypervisor CSRs */
+target_ulong hstatus;
+target_ulong hedeleg;
+target_ulong hideleg;
+target_ulong hgatp;
+
 target_ulong scounteren;
 target_ulong mcounteren;
 
 target_ulong sscratch;
 target_ulong mscratch;
 
+/* Background CSRs */
+target_ulong bsstatus;
+target_ulong bsip;
+target_ulong bsie;
+target_ulong bstvec;
+target_ulong bsscratch;
+target_ulong bsepc;
+target_ulong bscause;
+target_ulong bstval;
+target_ulong bsatp;
+
 /* temporary htif regs */
 uint64_t mfromhost;
 uint64_t mtohost;
-- 
2.21.0

[Qemu-devel] [RFC v1 06/23] target/riscv: Dump Hypervisor registers if enabled

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 65556ac543..c1495ef037 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -220,14 +220,41 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, 
int flags)
 #ifndef CONFIG_USER_ONLY
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid);
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bstatus ", env->bsstatus);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip ",
  (target_ulong)atomic_read(>mip));
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bsip",
+ (target_ulong)atomic_read(>bsip));
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie ", env->mie);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bsie", env->bsie);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mideleg ", env->mideleg);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hideleg ", env->hideleg);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "medeleg ", env->medeleg);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hedeleg ", env->hedeleg);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mtvec   ", env->mtvec);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bstvec  ", env->bstvec);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mepc", env->mepc);
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "sepc", env->sepc);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bsepc   ", env->bsepc);
+}
 qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mcause  ", env->mcause);
+if (riscv_has_ext(env, RVH)) {
+qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "bscause ", env->bscause);
+}
 #endif
 
 for (i = 0; i < 32; i++) {
-- 
2.21.0

[Qemu-devel] [RFC v1 09/23] target/riscv: Add Hypervisor CSR access functions

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/csr.c | 68 ++
 1 file changed, 68 insertions(+)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c1fcb795cd..c52fde6e7f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -82,6 +82,20 @@ static int smode(CPURISCVState *env, int csrno)
 return -!riscv_has_ext(env, RVS);
 }
 
+static int hmode(CPURISCVState *env, int csrno)
+{
+if (riscv_has_ext(env, RVS) &&
+riscv_has_ext(env, RVH)) {
+/* Hypervisor extension is supported */
+if ((env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
+env->priv == PRV_M) {
+return 0;
+}
+}
+
+return -1;
+}
+
 static int pmp(CPURISCVState *env, int csrno)
 {
 return -!riscv_feature(env, RISCV_FEATURE_PMP);
@@ -727,6 +741,55 @@ static int write_satp(CPURISCVState *env, int csrno, 
target_ulong val)
 return 0;
 }
 
+/* Hypervisor Extensions */
+static int read_hstatus(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->hstatus;
+return 0;
+}
+
+static int write_hstatus(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->hstatus = val;
+return 0;
+}
+
+static int read_hedeleg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->hedeleg;
+return 0;
+}
+
+static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->hedeleg = val;
+return 0;
+}
+
+static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->hideleg;
+return 0;
+}
+
+static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->hideleg = val;
+return 0;
+}
+
+static int read_hgatp(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->hgatp;
+return 0;
+}
+
+static int write_hgatp(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->hgatp = val;
+return 0;
+}
+
 /* Physical Memory Protection */
 static int read_pmpcfg(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -910,6 +973,11 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 /* Supervisor Protection and Translation */
 [CSR_SATP] ={ smode, read_satp,write_satp},
 
+[CSR_HSTATUS] = { hmode,   read_hstatus, write_hstatus
},
+[CSR_HEDELEG] = { hmode,   read_hedeleg, write_hedeleg
},
+[CSR_HIDELEG] = { hmode,   read_hideleg, write_hideleg
},
+[CSR_HGATP] =   { hmode,   read_hgatp,   write_hgatp  
},
+
 /* Physical Memory Protection */
 [CSR_PMPCFG0  ... CSR_PMPADDR9] =  { pmp,   read_pmpcfg,  write_pmpcfg   },
 [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp,   read_pmpaddr, write_pmpaddr  },
-- 
2.21.0

[Qemu-devel] [RFC v1 02/23] target/riscv: Add the Hypervisor extension

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8937bda918..3337d1aef3 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -81,6 +81,7 @@
 #define RVC RV('C')
 #define RVS RV('S')
 #define RVU RV('U')
+#define RVH RV('H')
 
 /* S extension denotes that Supervisor mode exists, however it is possible
to have a core that support S mode but does not have an MMU and there
-- 
2.21.0

[Qemu-devel] [RFC v1 11/23] target/riscv: Add background register swapping function

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h|  1 +
 target/riscv/cpu_bits.h   |  5 
 target/riscv/cpu_helper.c | 52 +++
 3 files changed, 58 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index b99d2b7af2..9897392ab7 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -319,6 +319,7 @@ void riscv_cpu_list(void);
 #define cpu_mmu_index riscv_cpu_mmu_index
 
 #ifndef CONFIG_USER_ONLY
+void riscv_cpu_swap_background_regs(CPURISCVState *env);
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts);
 uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value);
 #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 9c27727e6f..28117bdd32 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -550,3 +550,8 @@
 #define SIP_SSIP   MIP_SSIP
 #define SIP_STIP   MIP_STIP
 #define SIP_SEIP   MIP_SEIP
+
+/* MIE masks */
+#define MIE_SEIE   (1 << IRQ_S_EXT)
+#define MIE_STIE   (1 << IRQ_S_TIMER)
+#define MIE_SSIE   (1 << IRQ_S_SOFT)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1f466effcf..0128546e6a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -86,6 +86,58 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
 
 #if !defined(CONFIG_USER_ONLY)
 
+void riscv_cpu_swap_background_regs(CPURISCVState *env)
+{
+RISCVCPU *cpu = riscv_env_get_cpu(env);
+target_ulong tmp;
+target_ulong mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
+MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE;
+target_ulong sie_mask = MIE_SEIE | MIE_STIE | MIE_SSIE;
+
+g_assert(riscv_has_ext(env, RVH));
+
+#if defined(TARGET_RISCV64)
+mstatus_mask |= MSTATUS64_UXL;
+#endif
+
+tmp = env->bsstatus & mstatus_mask;
+env->bsstatus = env->mstatus & mstatus_mask;
+env->mstatus = (env->mstatus & ~mstatus_mask) | tmp;
+
+tmp = env->bsie & sie_mask;
+env->bsie = env->mie & sie_mask;
+env->mie = (env->mie & ~sie_mask) | tmp;
+
+tmp = env->bstvec;
+env->bstvec = env->stvec;
+env->stvec = tmp;
+
+tmp = env->bsscratch;
+env->bsscratch = env->sscratch;
+env->sscratch = tmp;
+
+tmp = env->bsepc;
+env->bsepc = env->sepc;
+env->sepc = tmp;
+
+tmp = env->bscause;
+env->bscause = env->scause;
+env->scause = tmp;
+
+tmp = env->bstval;
+env->bstval = env->sbadaddr;
+env->sbadaddr = tmp;
+
+tmp = env->bsatp;
+env->bsatp = env->satp;
+env->satp = tmp;
+
+tmp = (target_ulong)atomic_read(>bsip);
+tmp = riscv_cpu_update_mip(cpu, (MIP_SSIP | MIP_STIP | MIP_SEIP), tmp);
+tmp &= MIP_SSIP | MIP_STIP | MIP_SEIP;
+atomic_set(>bsip, tmp);
+}
+
 bool riscv_cpu_virt_enabled(CPURISCVState *env)
 {
 bool tmp;
-- 
2.21.0

[Qemu-devel] [RFC v1 12/23] target/ricsv: Flush the TLB on virtulisation mode changes

[Alistair Francis]

To ensure our TLB isn't out-of-date we flush it on all virt mode
changes. Unlike priv mode this isn't saved in the mmu_idx as all
guests share V=1. The easiest option is just to flush on all changes.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 0128546e6a..81f1cc83e5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -157,6 +157,11 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 return;
 }
 
+/* Flush the TLB on all virt mode changes. */
+if (((env->virt & VIRT_MODE_MASK) >> VIRT_MODE_SHIFT) != enable) {
+tlb_flush(CPU(riscv_env_get_cpu(env)));
+}
+
 env->virt &= ~VIRT_MODE_MASK;
 env->virt |= enable << VIRT_MODE_SHIFT;
 }
-- 
2.21.0

[Qemu-devel] [RFC v1 13/23] target/riscv: Generate illegal instruction on WFI when V=1

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/op_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 644d0fb35f..e08bb8dd5a 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -130,9 +130,10 @@ void helper_wfi(CPURISCVState *env)
 {
 CPUState *cs = CPU(riscv_env_get_cpu(env));
 
-if (env->priv == PRV_S &&
+if ((env->priv == PRV_S &&
 env->priv_ver >= PRIV_VERSION_1_10_0 &&
-get_field(env->mstatus, MSTATUS_TW)) {
+get_field(env->mstatus, MSTATUS_TW)) ||
+riscv_cpu_virt_enabled(env)) {
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
 } else {
 cs->halted = 1;
-- 
2.21.0

[Qemu-devel] [RFC v1 07/23] target/riscv: Remove strict perm checking for CSR R/W

[Alistair Francis]

The privledge check based on the CSR address mask 0x300 doesn't work
when using Hypervisor extensions so remove the check

Signed-off-by: Alistair Francis 
---
 target/riscv/csr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e6d68a9956..c1fcb795cd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -771,9 +771,8 @@ int riscv_csrrw(CPURISCVState *env, int csrno, target_ulong 
*ret_value,
 
 /* check privileges and return -1 if check fails */
 #if !defined(CONFIG_USER_ONLY)
-int csr_priv = get_field(csrno, 0x300);
 int read_only = get_field(csrno, 0xC00) == 3;
-if ((write_mask && read_only) || (env->priv < csr_priv)) {
+if (write_mask && read_only) {
 return -1;
 }
 #endif
-- 
2.21.0

[Qemu-devel] [RFC v1 14/23] riscv: plic: Remove unused interrupt functions

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 hw/riscv/sifive_plic.c | 12 
 include/hw/riscv/sifive_plic.h |  3 ---
 2 files changed, 15 deletions(-)

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index 07a032d93d..1e7e4c8d51 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -159,18 +159,6 @@ static void sifive_plic_update(SiFivePLICState *plic)
 }
 }
 
-void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq)
-{
-sifive_plic_set_pending(plic, irq, true);
-sifive_plic_update(plic);
-}
-
-void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq)
-{
-sifive_plic_set_pending(plic, irq, false);
-sifive_plic_update(plic);
-}
-
 static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid)
 {
 int i, j;
diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h
index ce8907f6aa..3b8a623919 100644
--- a/include/hw/riscv/sifive_plic.h
+++ b/include/hw/riscv/sifive_plic.h
@@ -69,9 +69,6 @@ typedef struct SiFivePLICState {
 uint32_t aperture_size;
 } SiFivePLICState;
 
-void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq);
-void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq);
-
 DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
 uint32_t num_sources, uint32_t num_priorities,
 uint32_t priority_base, uint32_t pending_base,
-- 
2.21.0

[Qemu-devel] [RFC v1 00/23] Add RISC-V Hypervisor Extension

[Alistair Francis]

This patch series adds the RISC-V Hypervisor extension 0.3. This is the
latest draft spec of the Hypervisor extension.

This series applies ontop of the RISC-V tree as it requires the previous
Hypervisor extension patches as well as the CPU parsing patches, both of
which have been accepted to the RISC-V tree. The full Hypervisor support
is avaliable at my GitHub (see below) which includes all required patches.
This series won't apply ontop of master.

The Hypervisor extension is disabled by default, so this series should
result in no changes to anyone using QEMU unless they enable the
extension. The extention can be enabled with the -cpu property (see
below).

At the moment the spec does not include information about the mstatush
register. As it is not in the spec I haven't added it to QEMU. This
means the extension won't work correctly for 32-bit guests. This should
be a small fix to add the CSR once the spec is updated.

All testing of this implementation has been done by using the baremetal
Xvisor Hypervisor. We are able to run two Linux guests (that's all I
have tried) as guests.

At the moment this spec is in a draft state and is subject to change. As
QEMU is extreamly useful in early bring up I think it makes sense for
QEMU to support non-frozen extensions. I would like to decide with this
series how QEMU will handle all future non-frozen extensions. That is a
standard way that QEMU users can test future RISC-V extensions while
still understanding things will change. One idea is just to disable it by
default, another option is to maybe use the Kconfig to make it a compile
time option which developers can use. Should we also display a warning
when running non-frozen extensions?

Thanks to Anup for doing the initial port of Xvisor. The port is avaliable here:
https://github.com/avpatel/xvisor-next and will run on QEMU.

Also thanks to Atish for implementing the SBI call support in Xvisor and
for lots of help debugging.

To run this yourself:
 1. Apply this patch series to QEMU. The latest branch can be found here:
  
https://github.com/alistair23/qemu/tree/mainline/alistair/riscv-hyp-work.next
 2. Get the version of OpenSBI that supports the H extenstion. This can
be found here:
  https://github.com/riscv/opensbi/tree/hyp_ext_changes_v1
 3. Build the next release of Xvisor. It is avaliable here:
  https://github.com/avpatel/xvisor-next
 4. Make sure you build the Xvisor tests, see here for details:
  
https://github.com/avpatel/xvisor-next/tree/master/tests/riscv/virt64/linux
 5. Run QEMU:
 ./riscv64-softmmu/qemu-system-riscv64 -nographic \
   -machine virt -cpu rv64,h=true\
   -serial mon:stdio -serial null -m 4G \
   -device loader,file=vmm.bin,addr=0x8020 \
   -kernel fw_jump.elf \
   -initrd vmm-disk-linux.img \
   -append "vmm.console=uart@1000 vmm.bootcmd=\"vfs mount initrd /;vfs 
run /boot.xscript;vfs cat /system/banner.txt\""

   Once you get to the prompt you can start the geust by running:
 guest kick guest0
   You can then bind to the serial port using:
 vserial bind guest0/uart0
   Then you can start Linux using:
 autoexec

 This was all tested with the mainline 5.1 kernel. I don't know if it
 will work on older kernels.

So far all of the QEMU work has been tested on Xvisor.

Known Issues/TODO:
 - Add mstatush to support 32-bit Hypervisors
 - Add support for bsstatus.FS and sstatus.FS from the Hypervisor spec
 - Fix the random hang that sometimes appears when running a Hypervisor guest

There is also on going work from Anup to port KVM.
We have code complete implementation of RISC-V KVM kernel module and
RISC-V KVMTOOL. Currently, we are debugging KVM on QEMU and we will
send-out RFC PATCHES for KVM in June/July 2019.
The KVM RISC-V kernel module is available in riscv_kvm_v1
branch at: https://github.com/avpatel/linux.git
The KVMTOOL RISC-V port is available in riscv_v1 branch of
https://github.com/avpatel/kvmtool.git

There is very early work on a Xen port as well which is avaliable here:
https://github.com/alistair23/xen/tree/alistair/riscv-port

Alistair Francis (23):
  target/riscv: Don't set write permissions on dirty PTEs
  target/riscv: Add the Hypervisor extension
  target/riscv: Add the virtulisation mode
  target/riscv: Add the force HS exception mode
  target/riscv: Add the Hypervisor CSRs to CPUState
  target/riscv: Dump Hypervisor registers if enabled
  target/riscv: Remove strict perm checking for CSR R/W
  target/riscv: Add support for background interrupt setting
  target/riscv: Add Hypervisor CSR access functions
  target/riscv: Add background CSRs accesses
  target/riscv: Add background register swapping function
  target/ricsv: Flush the TLB on virtulisation mode changes
  target/riscv: Generate illegal instruction on WFI when V=1
  riscv: plic: Remove unused interrupt functions
  riscv: plic: Always set sip.SEIP bit for HS
  target/riscv: Add hypvervisor trap support
  target/riscv: Add Hypervisor trap 

[Qemu-devel] [RFC v1 18/23] target/riscv: Add hfence instructions

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/insn32.decode| 23 ++-
 .../riscv/insn_trans/trans_privileged.inc.c   | 40 +++
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 6f3ab7aa52..6c79c5fd0c 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -59,20 +59,25 @@
 @r2_rm   ...   . . ... . ... %rs1 %rm %rd
 @r2  ...   . . ... . ... %rs1 %rd
 
+@hfence_gvma ... . .   ... . ... %rs2 %rs1
+@hfence_bvma ... . .   ... . ... %rs2 %rs1
+
 @sfence_vma ... . .   ... . ... %rs2 %rs1
 @sfence_vm  ... . .   ... . ... %rs1
 
 
 # *** Privileged Instructions ***
-ecall   0 000 0 1110011
-ebreak 0001 0 000 0 1110011
-uret   00000010 0 000 0 1110011
-sret   000100000010 0 000 0 1110011
-hret   00100010 0 000 0 1110011
-mret   001100000010 0 000 0 1110011
-wfi000100000101 0 000 0 1110011
-sfence_vma 0001001. . 000 0 1110011 @sfence_vma
-sfence_vm  000100000100 . 000 0 1110011 @sfence_vm
+ecall    0 000 0 1110011
+ebreak  0001 0 000 0 1110011
+uret00000010 0 000 0 1110011
+sret000100000010 0 000 0 1110011
+hret00100010 0 000 0 1110011
+mret001100000010 0 000 0 1110011
+wfi 000100000101 0 000 0 1110011
+hfence_gvma 1010001. . 000 0 1110011 @hfence_gvma
+hfence_bvma 0010001. . 000 0 1110011 @hfence_bvma
+sfence_vma  0001001. . 000 0 1110011 @sfence_vma
+sfence_vm   000100000100 . 000 0 1110011 @sfence_vm
 
 # *** RV32I Base Instruction Set ***
 lui     . 0110111 @u
diff --git a/target/riscv/insn_trans/trans_privileged.inc.c 
b/target/riscv/insn_trans/trans_privileged.inc.c
index 664d6ba3f2..ac953ad30d 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -108,3 +108,43 @@ static bool trans_sfence_vm(DisasContext *ctx, 
arg_sfence_vm *a)
 #endif
 return false;
 }
+
+static bool trans_hfence_gvma(DisasContext *ctx, arg_sfence_vma *a)
+{
+#ifndef CONFIG_USER_ONLY
+if (ctx->priv_ver == PRIV_VERSION_1_10_0 &&
+has_ext(ctx, RVH)) {
+/* Hpervisor extensions exist */
+/*
+ * if (env->priv == PRV_M ||
+ *   (env->priv == PRV_S &&
+ *!riscv_cpu_virt_enabled(env) &&
+ *get_field(ctx->mstatus_fs, MSTATUS_TVM))) {
+ */
+gen_helper_tlb_flush(cpu_env);
+return true;
+/* } */
+}
+#endif
+return false;
+}
+
+static bool trans_hfence_bvma(DisasContext *ctx, arg_sfence_vma *a)
+{
+#ifndef CONFIG_USER_ONLY
+if (ctx->priv_ver == PRIV_VERSION_1_10_0 &&
+has_ext(ctx, RVH)) {
+/* Hpervisor extensions exist */
+/*
+ * if (env->priv == PRV_M ||
+ *   (env->priv == PRV_S &&
+ *!riscv_cpu_virt_enabled(env) &&
+ *get_field(ctx->mstatus_fs, MSTATUS_TVM))) {
+ */
+gen_helper_tlb_flush(cpu_env);
+return true;
+/* } */
+}
+#endif
+return false;
+}
-- 
2.21.0

[Qemu-devel] [RFC v1 08/23] target/riscv: Add support for background interrupt setting

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 0fdc81f71f..1f466effcf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -38,12 +38,27 @@ static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 {
 target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
 target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
+target_ulong bsstatus_sie = get_field(env->bsstatus, MSTATUS_SIE);
+
 target_ulong pending = atomic_read(>mip) & env->mie;
-target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M && 
mstatus_mie);
-target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S && 
mstatus_sie);
+target_ulong hspending = atomic_read(>bsip) & env->bsie;
+
+target_ulong mie  = env->priv < PRV_M || (env->priv == PRV_M && 
mstatus_mie);
+target_ulong sie  = env->priv < PRV_S || (env->priv == PRV_S && 
mstatus_sie);
+target_ulong bsie = env->priv < PRV_S || (env->priv == PRV_S && 
bsstatus_sie);
+
 target_ulong irqs = (pending & ~env->mideleg & -mie) |
 (pending &  env->mideleg & -sie);
 
+if (riscv_cpu_virt_enabled(env)) {
+target_ulong pending_hs_irq = hspending & -bsie;
+
+if (pending_hs_irq) {
+riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
+return ctz64(pending_hs_irq);
+}
+}
+
 if (irqs) {
 return ctz64(irqs); /* since non-zero */
 } else {
-- 
2.21.0

[Qemu-devel] [RFC v1 15/23] riscv: plic: Always set sip.SEIP bit for HS

[Alistair Francis]

When the PLIC generates an interrupt ensure we always set it for the SIP
CSR that corresponds to the HS (V=0) register.

Signed-off-by: Alistair Francis 
---
 hw/riscv/sifive_plic.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index 1e7e4c8d51..25da29fa3d 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -147,7 +147,17 @@ static void sifive_plic_update(SiFivePLICState *plic)
 riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_MEIP, 
BOOL_TO_MASK(level));
 break;
 case PLICMode_S:
-riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
BOOL_TO_MASK(level));
+if (riscv_cpu_virt_enabled(env)) {
+if (level) {
+atomic_or(>bsip, MIP_SEIP);
+g_assert(riscv_cpu_virt_enabled(env));
+} else {
+atomic_and(>bsip, ~MIP_SEIP);
+g_assert(riscv_cpu_virt_enabled(env));
+}
+} else {
+riscv_cpu_update_mip(RISCV_CPU(cpu), MIP_SEIP, 
BOOL_TO_MASK(level));
+}
 break;
 default:
 break;
-- 
2.21.0

[Qemu-devel] [RFC v1 19/23] target/riscv: Allow specifying MMU stage

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 35 +++
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b7e47b97f5..3c963d26bc 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -261,10 +261,19 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong 
newpriv)
  *
  * Adapted from Spike's mmu_t::translate and mmu_t::walk
  *
+ * @env: CPURISCVState
+ * @physical: This will be set to the calculated physical address
+ * @prot: The returned protection attributes
+ * @addr: The virtual address to be translated
+ * @access_type: The type of MMU access
+ * @mmu_idx: Indicates current privilege level
+ * @first_stage: Are we in first stage translation?
+ *   Second stage is used for hypervisor guest translation
  */
 static int get_physical_address(CPURISCVState *env, hwaddr *physical,
 int *prot, target_ulong addr,
-int access_type, int mmu_idx)
+int access_type, int mmu_idx,
+bool first_stage)
 {
 /* NOTE: the env->pc value visible here will not be
  * correct, but the value visible to the exception handler
@@ -455,12 +464,20 @@ restart:
 }
 
 static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
-MMUAccessType access_type)
+MMUAccessType access_type, bool first_stage)
 {
 CPUState *cs = CPU(riscv_env_get_cpu(env));
-int page_fault_exceptions =
-(env->priv_ver >= PRIV_VERSION_1_10_0) &&
-get_field(env->satp, SATP_MODE) != VM_1_10_MBARE;
+int page_fault_exceptions;
+if (first_stage) {
+page_fault_exceptions =
+(env->priv_ver >= PRIV_VERSION_1_10_0) &&
+get_field(env->satp, SATP_MODE) != VM_1_10_MBARE;
+riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
+} else {
+page_fault_exceptions =
+get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE;
+riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
+}
 switch (access_type) {
 case MMU_INST_FETCH:
 cs->exception_index = page_fault_exceptions ?
@@ -487,7 +504,8 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr 
addr)
 int prot;
 int mmu_idx = cpu_mmu_index(>env, false);
 
-if (get_physical_address(>env, _addr, , addr, 0, mmu_idx)) {
+if (get_physical_address(>env, _addr, , addr, 0, mmu_idx,
+ true)) {
 return -1;
 }
 return phys_addr;
@@ -547,7 +565,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
   __func__, address, access_type, mmu_idx);
 
-ret = get_physical_address(env, , , address, access_type, mmu_idx);
+ret = get_physical_address(env, , , address, access_type, mmu_idx,
+   true);
 
 qemu_log_mask(CPU_LOG_MMU,
   "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
@@ -564,7 +583,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 } else if (probe) {
 return false;
 } else {
-raise_mmu_exception(env, address, access_type);
+raise_mmu_exception(env, address, access_type, true);
 riscv_raise_exception(env, cs->exception_index, retaddr);
 }
 #else
-- 
2.21.0

[Qemu-devel] [RFC v1 04/23] target/riscv: Add the force HS exception mode

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.h|  2 ++
 target/riscv/cpu_bits.h   |  6 ++
 target/riscv/cpu_helper.c | 23 +++
 3 files changed, 31 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de4843b879..eeb3756c91 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -282,6 +282,8 @@ int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t 
*buf, int reg);
 bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
 bool riscv_cpu_virt_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
+bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env);
+void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable);
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
 hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 07c95e8d2c..c898bb1102 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -423,6 +423,12 @@
 #define VIRT_MODE_SHIFT 0
 #define VIRT_MODE_MASK  (1 << VIRT_MODE_SHIFT)
 
+/* HS-level exceptions modes */
+#define CLEAR_HS_EXCEP0
+#define FORCE_HS_EXCEP1
+#define FORCE_HS_EXCEP_SHIFT  1
+#define FORCE_HS_EXCEP_MASK   (1 << FORCE_HS_EXCEP_SHIFT)
+
 /* RV32 satp CSR field masks */
 #define SATP32_MODE 0x8000
 #define SATP32_ASID 0x7fc0
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 5912ae63b7..0fdc81f71f 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -94,6 +94,29 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 env->virt |= enable << VIRT_MODE_SHIFT;
 }
 
+bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env)
+{
+bool tmp;
+
+if (!riscv_has_ext(env, RVH)) {
+return false;
+}
+
+tmp = (env->virt & FORCE_HS_EXCEP_MASK) >> FORCE_HS_EXCEP_SHIFT;
+
+return tmp == FORCE_HS_EXCEP;
+}
+
+void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable)
+{
+if (!riscv_has_ext(env, RVH)) {
+return;
+}
+
+env->virt &= ~FORCE_HS_EXCEP_MASK;
+env->virt |= enable << FORCE_HS_EXCEP_SHIFT;
+}
+
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
 {
 CPURISCVState *env = >env;
-- 
2.21.0

[Qemu-devel] [RFC v1 16/23] target/riscv: Add hypvervisor trap support

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_bits.h   |  4 +--
 target/riscv/cpu_helper.c | 71 +--
 target/riscv/csr.c|  4 +--
 3 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 28117bdd32..8966c1bff6 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -508,8 +508,8 @@
 #define RISCV_EXCP_STORE_AMO_ADDR_MIS  0x6
 #define RISCV_EXCP_STORE_AMO_ACCESS_FAULT  0x7
 #define RISCV_EXCP_U_ECALL 0x8
-#define RISCV_EXCP_S_ECALL 0x9
-#define RISCV_EXCP_H_ECALL 0xa
+#define RISCV_EXCP_HS_ECALL0x9
+#define RISCV_EXCP_VS_ECALL0xa
 #define RISCV_EXCP_M_ECALL 0xb
 #define RISCV_EXCP_INST_PAGE_FAULT 0xc /* since: priv-1.10.0 */
 #define RISCV_EXCP_LOAD_PAGE_FAULT 0xd /* since: priv-1.10.0 */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 81f1cc83e5..b7e47b97f5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -595,6 +595,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 
 RISCVCPU *cpu = RISCV_CPU(cs);
 CPURISCVState *env = >env;
+target_ulong s;
 
 /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
  * so we mask off the MSB and separate into trap type and cause.
@@ -604,13 +605,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 target_ulong deleg = async ? env->mideleg : env->medeleg;
 target_ulong tval = 0;
 
-static const int ecall_cause_map[] = {
-[PRV_U] = RISCV_EXCP_U_ECALL,
-[PRV_S] = RISCV_EXCP_S_ECALL,
-[PRV_H] = RISCV_EXCP_H_ECALL,
-[PRV_M] = RISCV_EXCP_M_ECALL
-};
-
 if (!async) {
 /* set tval to badaddr for traps with address information */
 switch (cause) {
@@ -631,7 +625,16 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 /* ecall is dispatched as one cause so translate based on mode */
 if (cause == RISCV_EXCP_U_ECALL) {
 assert(env->priv <= 3);
-cause = ecall_cause_map[env->priv];
+
+if (env->priv == PRV_M) {
+cause = RISCV_EXCP_M_ECALL;
+} else if (env->priv == PRV_S && riscv_cpu_virt_enabled(env)) {
+cause = RISCV_EXCP_VS_ECALL;
+} else if (env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) {
+cause = RISCV_EXCP_HS_ECALL;
+} else if (env->priv == PRV_U) {
+cause = RISCV_EXCP_U_ECALL;
+}
 }
 }
 
@@ -641,7 +644,42 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 if (env->priv <= PRV_S &&
 cause < TARGET_LONG_BITS && ((deleg >> cause) & 1)) {
 /* handle the trap in S-mode */
-target_ulong s = env->mstatus;
+if (riscv_has_ext(env, RVH)) {
+target_ulong hdeleg = async ? env->hideleg : env->hedeleg;
+
+if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1) &&
+!riscv_cpu_force_hs_excep_enabled(env)) {
+/* Trap to VS mode */
+} else if (riscv_cpu_virt_enabled(env)) {
+/* Trap into HS mode, from virt */
+riscv_cpu_swap_background_regs(env);
+env->hstatus = set_field(env->hstatus, HSTATUS_SP2V,
+ get_field(env->hstatus, HSTATUS_SPV));
+env->hstatus = set_field(env->hstatus, HSTATUS_SP2P,
+ get_field(env->mstatus, SSTATUS_SPP));
+env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
+ riscv_cpu_virt_enabled(env));
+
+if (riscv_cpu_force_hs_excep_enabled(env)) {
+env->hstatus = set_field(env->hstatus, HSTATUS_STL, 1);
+} else {
+env->hstatus = set_field(env->hstatus, HSTATUS_STL, 0);
+}
+
+riscv_cpu_set_virt_enabled(env, VIRT_OFF);
+riscv_cpu_set_force_hs_excep(env, CLEAR_HS_EXCEP);
+} else {
+/* Trap into HS mode */
+env->hstatus = set_field(env->hstatus, HSTATUS_SP2V,
+ get_field(env->hstatus, HSTATUS_SPV));
+env->hstatus = set_field(env->hstatus, HSTATUS_SP2P,
+ get_field(env->mstatus, SSTATUS_SPP));
+env->hstatus = set_field(env->hstatus, HSTATUS_SPV,
+ riscv_cpu_virt_enabled(env));
+}
+}
+
+s = env->mstatus;
 s = set_field(s, MSTATUS_SPIE, env->priv_ver >= PRIV_VERSION_1_10_0 ?
 get_field(s, MSTATUS_SIE) : get_field(s, MSTATUS_UIE << 
env->priv));
 s = set_field(s, MSTATUS_SPP, env->priv);
@@ -655,7 +693,20 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 

[Qemu-devel] [RFC v1 17/23] target/riscv: Add Hypervisor trap return support

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/op_helper.c | 66 
 1 file changed, 54 insertions(+), 12 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index e08bb8dd5a..60dcd73fc7 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -73,6 +73,8 @@ target_ulong helper_csrrc(CPURISCVState *env, target_ulong 
src,
 
 target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
 {
+target_ulong prev_priv, prev_virt, mstatus;
+
 if (!(env->priv >= PRV_S)) {
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
 }
@@ -87,16 +89,46 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
 }
 
-target_ulong mstatus = env->mstatus;
-target_ulong prev_priv = get_field(mstatus, MSTATUS_SPP);
-mstatus = set_field(mstatus,
-env->priv_ver >= PRIV_VERSION_1_10_0 ?
-MSTATUS_SIE : MSTATUS_UIE << prev_priv,
-get_field(mstatus, MSTATUS_SPIE));
-mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
-mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
+mstatus = env->mstatus;
+
+if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
+/* We support Hypervisor extensions and virtulisation is disabled */
+target_ulong hstatus = env->hstatus;
+
+prev_priv = get_field(mstatus, MSTATUS_SPP);
+prev_virt = get_field(hstatus, HSTATUS_SPV);
+
+hstatus = set_field(hstatus, HSTATUS_SPV,
+ get_field(hstatus, HSTATUS_SP2V));
+mstatus = set_field(mstatus, MSTATUS_SPP,
+get_field(hstatus, HSTATUS_SP2P));
+hstatus = set_field(hstatus, HSTATUS_SP2V, 0);
+hstatus = set_field(hstatus, HSTATUS_SP2P, 0);
+mstatus = set_field(mstatus, SSTATUS_SIE,
+get_field(mstatus, SSTATUS_SPIE));
+mstatus = set_field(mstatus, SSTATUS_SPIE, 1);
+
+env->mstatus = mstatus;
+env->hstatus = hstatus;
+
+if (prev_virt == VIRT_ON) {
+riscv_cpu_swap_background_regs(env);
+}
+
+riscv_cpu_set_virt_enabled(env, prev_virt);
+} else {
+prev_priv = get_field(mstatus, MSTATUS_SPP);
+
+mstatus = set_field(mstatus,
+env->priv_ver >= PRIV_VERSION_1_10_0 ?
+MSTATUS_SIE : MSTATUS_UIE << prev_priv,
+get_field(mstatus, MSTATUS_SPIE));
+mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
+mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
+env->mstatus = mstatus;
+}
+
 riscv_cpu_set_mode(env, prev_priv);
-env->mstatus = mstatus;
 
 return retpc;
 }
@@ -114,14 +146,24 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
 
 target_ulong mstatus = env->mstatus;
 target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
+target_ulong prev_virt = get_field(mstatus, MSTATUS_MPV);
 mstatus = set_field(mstatus,
 env->priv_ver >= PRIV_VERSION_1_10_0 ?
 MSTATUS_MIE : MSTATUS_UIE << prev_priv,
 get_field(mstatus, MSTATUS_MPIE));
-mstatus = set_field(mstatus, MSTATUS_MPIE, 0);
-mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
-riscv_cpu_set_mode(env, prev_priv);
+mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
+mstatus = set_field(mstatus, MSTATUS_MPP, 0);
+mstatus = set_field(mstatus, MSTATUS_MPV, 0);
 env->mstatus = mstatus;
+riscv_cpu_set_mode(env, prev_priv);
+
+if (riscv_has_ext(env, RVH)) {
+if (prev_virt == VIRT_ON) {
+riscv_cpu_swap_background_regs(env);
+}
+
+riscv_cpu_set_virt_enabled(env, prev_virt);
+}
 
 return retpc;
 }
-- 
2.21.0

[Qemu-devel] [RFC v1 10/23] target/riscv: Add background CSRs accesses

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_bits.h |  11 
 target/riscv/csr.c  | 119 
 2 files changed, 130 insertions(+)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index c898bb1102..9c27727e6f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -169,6 +169,17 @@
 #define CSR_SPTBR   0x180
 #define CSR_SATP0x180
 
+/* Background CSRs */
+#define CSR_BSSTATUS0x200
+#define CSR_BSIE0x204
+#define CSR_BSTVEC  0x205
+#define CSR_BSSCRATCH   0x240
+#define CSR_BSEPC   0x241
+#define CSR_BSCAUSE 0x242
+#define CSR_BSTVAL  0x243
+#define CSR_BSIP0x244
+#define CSR_BSATP   0x280
+
 /* Physical Memory Protection */
 #define CSR_PMPCFG0 0x3a0
 #define CSR_PMPCFG1 0x3a1
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index c52fde6e7f..908e166426 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -790,6 +790,115 @@ static int write_hgatp(CPURISCVState *env, int csrno, 
target_ulong val)
 return 0;
 }
 
+/* Background CSR Registers */
+static int read_bsstatus(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bsstatus;
+return 0;
+}
+
+static int write_bsstatus(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bsstatus = val;
+return 0;
+}
+
+static int read_bsie(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bsie;
+return 0;
+}
+
+static int write_bsie(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bsie = val;
+return 0;
+}
+
+static int read_bstvec(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bstvec;
+return 0;
+}
+
+static int write_bstvec(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bstvec = val;
+return 0;
+}
+
+static int read_bsscratch(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bsscratch;
+return 0;
+}
+
+static int write_bsscratch(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bsscratch = val;
+return 0;
+}
+
+static int read_bsepc(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bsepc;
+return 0;
+}
+
+static int write_bsepc(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bsepc = val;
+return 0;
+}
+
+static int read_bscause(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bscause;
+return 0;
+}
+
+static int write_bscause(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bscause = val;
+return 0;
+}
+
+static int read_bstval(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bstval;
+return 0;
+}
+
+static int write_bstval(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bstval = val;
+return 0;
+}
+
+static int read_bsip(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = (target_ulong)atomic_read(>bsip);
+return 0;
+}
+
+static int write_bsip(CPURISCVState *env, int csrno, target_ulong val)
+{
+atomic_set(>bsip, val);
+return 0;
+}
+
+static int read_bsatp(CPURISCVState *env, int csrno, target_ulong *val)
+{
+*val = env->bsatp;
+return 0;
+}
+
+static int write_bsatp(CPURISCVState *env, int csrno, target_ulong val)
+{
+env->bsatp = val;
+return 0;
+}
+
 /* Physical Memory Protection */
 static int read_pmpcfg(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -978,6 +1087,16 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 [CSR_HIDELEG] = { hmode,   read_hideleg, write_hideleg
},
 [CSR_HGATP] =   { hmode,   read_hgatp,   write_hgatp  
},
 
+[CSR_BSSTATUS] ={ hmode,   read_bsstatus,write_bsstatus   
},
+[CSR_BSIE] ={ hmode,   read_bsie,write_bsie   
},
+[CSR_BSTVEC] =  { hmode,   read_bstvec,  write_bstvec 
},
+[CSR_BSSCRATCH] =   { hmode,   read_bsscratch,   write_bsscratch  
},
+[CSR_BSEPC] =   { hmode,   read_bsepc,   write_bsepc  
},
+[CSR_BSCAUSE] = { hmode,   read_bscause, write_bscause
},
+[CSR_BSTVAL] =  { hmode,   read_bstval,  write_bstval 
},
+[CSR_BSIP] ={ hmode,   read_bsip,write_bsip   
},
+[CSR_BSATP] =   { hmode,   read_bsatp,   write_bsatp  
},
+
 /* Physical Memory Protection */
 [CSR_PMPCFG0  ... CSR_PMPADDR9] =  { pmp,   read_pmpcfg,  write_pmpcfg   },
 [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp,   read_pmpaddr, write_pmpaddr  },
-- 
2.21.0

[Qemu-devel] [RFC v1 20/23] target/riscv: Allow specifying number of MMU stages

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 3c963d26bc..f57e49c973 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -273,7 +273,7 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong 
newpriv)
 static int get_physical_address(CPURISCVState *env, hwaddr *physical,
 int *prot, target_ulong addr,
 int access_type, int mmu_idx,
-bool first_stage)
+bool first_stage, bool two_stage)
 {
 /* NOTE: the env->pc value visible here will not be
  * correct, but the value visible to the exception handler
@@ -505,9 +505,10 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr 
addr)
 int mmu_idx = cpu_mmu_index(>env, false);
 
 if (get_physical_address(>env, _addr, , addr, 0, mmu_idx,
- true)) {
+ true, false)) {
 return -1;
 }
+
 return phys_addr;
 }
 
@@ -566,7 +567,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
   __func__, address, access_type, mmu_idx);
 
 ret = get_physical_address(env, , , address, access_type, mmu_idx,
-   true);
+   true, false);
 
 qemu_log_mask(CPU_LOG_MMU,
   "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
-- 
2.21.0

[Qemu-devel] [RFC v1 22/23] target/riscv: Call the second stage MMU in virtualisation mode

[Alistair Francis]

The qemu_log_mask(CPU_LOG_MMU,... calls trigger false positive
checkpatch errors which are being ignored.

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 118 --
 1 file changed, 99 insertions(+), 19 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 387c12547b..99091ed0fd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -569,15 +569,23 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,
 hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
 {
 RISCVCPU *cpu = RISCV_CPU(cs);
+CPURISCVState *env = >env;
 hwaddr phys_addr;
 int prot;
 int mmu_idx = cpu_mmu_index(>env, false);
 
-if (get_physical_address(>env, _addr, , addr, 0, mmu_idx,
- true, false)) {
+if (get_physical_address(env, _addr, , addr, 0, mmu_idx,
+ true, riscv_cpu_virt_enabled(env))) {
 return -1;
 }
 
+if (riscv_cpu_virt_enabled(env)) {
+if (get_physical_address(env, _addr, , phys_addr,
+ 0, mmu_idx, false, true)) {
+return -1;
+}
+}
+
 return phys_addr;
 }
 
@@ -628,34 +636,106 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 #ifndef CONFIG_USER_ONLY
 RISCVCPU *cpu = RISCV_CPU(cs);
 CPURISCVState *env = >env;
+vaddr im_address;
 hwaddr pa = 0;
 int prot;
+bool m_mode_two_stage = false;
+bool hs_mode_two_stage = false;
 int ret = TRANSLATE_FAIL;
 
 qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
   __func__, address, access_type, mmu_idx);
 
-ret = get_physical_address(env, , , address, access_type, mmu_idx,
-   true, false);
+/*
+ * Determine if we are in M mode and MPRV is set or in HS mode and SPRV is
+ * set and we want to access a virtulisation address.
+ */
+if (riscv_has_ext(env, RVH)) {
+m_mode_two_stage = env->priv == PRV_M &&
+   access_type != MMU_INST_FETCH &&
+   get_field(env->mstatus, MSTATUS_MPRV) &&
+   get_field(env->mstatus, MSTATUS_MPV);
+
+hs_mode_two_stage = env->priv == PRV_S &&
+!riscv_cpu_virt_enabled(env) &&
+access_type != MMU_INST_FETCH &&
+get_field(env->hstatus, HSTATUS_SPRV) &&
+get_field(env->hstatus, HSTATUS_SPV);
+}
+
+if (riscv_cpu_virt_enabled(env) || m_mode_two_stage || hs_mode_two_stage) {
+/* Two stage lookup */
+ret = get_physical_address(env, , , address, access_type,
+   mmu_idx, true, true);
 
-qemu_log_mask(CPU_LOG_MMU,
-  "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
-  " prot %d\n", __func__, address, ret, pa, prot);
+qemu_log_mask(CPU_LOG_MMU,
+  "%s 1st-stage address=%" VADDR_PRIx " ret %d physical "
+  TARGET_FMT_plx " prot %d\n",
+  __func__, address, ret, pa, prot);
 
-if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-!pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type)) {
-ret = TRANSLATE_FAIL;
-}
-if (ret == TRANSLATE_SUCCESS) {
-tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
- prot, mmu_idx, TARGET_PAGE_SIZE);
-return true;
-} else if (probe) {
-return false;
+if (ret == TRANSLATE_FAIL) {
+if (!probe) {
+raise_mmu_exception(env, address, access_type, true);
+riscv_raise_exception(env, cs->exception_index, retaddr);
+}
+return ret;
+}
+
+/* Second stage lookup */
+im_address = pa;
+
+ret = get_physical_address(env, , , im_address, access_type, 
mmu_idx,
+   false, true);
+
+qemu_log_mask(CPU_LOG_MMU,
+"%s 2nd-stage address=%" VADDR_PRIx " ret %d physical "
+TARGET_FMT_plx " prot %d\n",
+__func__, im_address, ret, pa, prot);
+
+if (riscv_feature(env, RISCV_FEATURE_PMP) &&
+!pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type)) {
+ret = TRANSLATE_FAIL;
+}
+
+if (ret == TRANSLATE_FAIL) {
+/*
+ * Guest physical address translation failed, this is a HS
+ * level exception
+ */
+if (!probe) {
+raise_mmu_exception(env, im_address | (address & 
(TARGET_PAGE_SIZE - 1)), access_type, false);
+riscv_raise_exception(env, cs->exception_index, retaddr);
+}
+return ret;
+}
 } else {
-

[Qemu-devel] [RFC v1 21/23] target/riscv: Implement second stage MMU

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 87 +++
 1 file changed, 78 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f57e49c973..387c12547b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -280,13 +280,40 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
  * (riscv_cpu_do_interrupt) is correct */
 
 int mode = mmu_idx;
-
+bool use_background = false;
+
+/*
+ * Check if we should use the background registers for the two
+ * stage translation. We don't need to check if we actually need
+ * two stage translation as that happened before this function
+ * was called. Background registers will be used if the guest has
+ * forced a two stage translation to be on (in HS or M mode).
+ */
 if (mode == PRV_M && access_type != MMU_INST_FETCH) {
 if (get_field(env->mstatus, MSTATUS_MPRV)) {
 mode = get_field(env->mstatus, MSTATUS_MPP);
+
+if (riscv_has_ext(env, RVH) &&
+get_field(env->mstatus, MSTATUS_MPV)) {
+use_background = true;
+}
+}
+}
+
+if (mode == PRV_S && access_type != MMU_INST_FETCH &&
+riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
+if (get_field(env->hstatus, HSTATUS_SPRV)) {
+mode = get_field(env->mstatus, SSTATUS_SPP);
+use_background = true;
 }
 }
 
+if (first_stage == false) {
+/* We are in stage 2 translation, this is similar to stage 1. */
+/* Stage 2 is always taken as U-mode */
+mode = PRV_U;
+}
+
 if (mode == PRV_M || !riscv_feature(env, RISCV_FEATURE_MMU)) {
 *physical = addr;
 *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -296,13 +323,30 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
 *prot = 0;
 
 target_ulong base;
-int levels, ptidxbits, ptesize, vm, sum;
-int mxr = get_field(env->mstatus, MSTATUS_MXR);
+int levels, ptidxbits, ptesize, vm, sum, mxr, widened;
+
+if (first_stage == true) {
+mxr = get_field(env->mstatus, MSTATUS_MXR);
+} else {
+mxr = get_field(env->bsstatus, MSTATUS_MXR);
+}
 
 if (env->priv_ver >= PRIV_VERSION_1_10_0) {
-base = get_field(env->satp, SATP_PPN) << PGSHIFT;
+if (first_stage == true) {
+if (use_background) {
+base = get_field(env->bsatp, SATP_PPN) << PGSHIFT;
+vm = get_field(env->bsatp, SATP_MODE);
+} else {
+base = get_field(env->satp, SATP_PPN) << PGSHIFT;
+vm = get_field(env->satp, SATP_MODE);
+}
+widened = 0;
+} else {
+base = get_field(env->hgatp, HGATP_PPN) << PGSHIFT;
+vm = get_field(env->hgatp, HGATP_MODE);
+widened = 2;
+}
 sum = get_field(env->mstatus, MSTATUS_SUM);
-vm = get_field(env->satp, SATP_MODE);
 switch (vm) {
 case VM_1_10_SV32:
   levels = 2; ptidxbits = 10; ptesize = 4; break;
@@ -320,6 +364,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
   g_assert_not_reached();
 }
 } else {
+widened = 0;
 base = env->sptbr << PGSHIFT;
 sum = !get_field(env->mstatus, MSTATUS_PUM);
 vm = get_field(env->mstatus, MSTATUS_VM);
@@ -340,7 +385,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 
 CPUState *cs = CPU(riscv_env_get_cpu(env));
-int va_bits = PGSHIFT + levels * ptidxbits;
+int va_bits = PGSHIFT + levels * ptidxbits + widened;
 target_ulong mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
 target_ulong masked_msbs = (addr >> (va_bits - 1)) & mask;
 if (masked_msbs != 0 && masked_msbs != mask) {
@@ -354,11 +399,30 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
 restart:
 #endif
 for (i = 0; i < levels; i++, ptshift -= ptidxbits) {
-target_ulong idx = (addr >> (PGSHIFT + ptshift)) &
+target_ulong idx;
+if (i == 0) {
+idx = (addr >> (PGSHIFT + ptshift)) &
+   ((1 << (ptidxbits + widened)) - 1);
+} else {
+idx = (addr >> (PGSHIFT + ptshift)) &
((1 << ptidxbits) - 1);
+}
 
 /* check that physical address of PTE is legal */
-target_ulong pte_addr = base + idx * ptesize;
+target_ulong pte_addr;
+
+if (two_stage && first_stage) {
+hwaddr vbase;
+
+/* Do the second stage translation on the base PTE address. */
+get_physical_address(env, , prot, base, access_type,
+ mmu_idx, false, true);
+
+pte_addr = vbase + idx * ptesize;
+} else {
+pte_addr 

[Qemu-devel] [RFC v1 23/23] target/riscv: Allow enabling the Hypervisor extension

[Alistair Francis]

Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c | 4 
 target/riscv/cpu.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c1495ef037..b17dfb86c6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -436,6 +436,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 if (cpu->cfg.ext_u) {
 target_misa |= RVU;
 }
+if (cpu->cfg.ext_h) {
+target_misa |= RVH;
+}
 
 set_misa(env, RVXLEN | target_misa);
 }
@@ -472,6 +475,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("c", RISCVCPU, cfg.ext_c, true),
 DEFINE_PROP_BOOL("s", RISCVCPU, cfg.ext_s, true),
 DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true),
+DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, false),
 DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
 DEFINE_PROP_STRING("user_spec", RISCVCPU, cfg.user_spec),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 9897392ab7..d2cfc69e9a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -259,6 +259,7 @@ typedef struct RISCVCPU {
 bool ext_c;
 bool ext_s;
 bool ext_u;
+bool ext_h;
 
 char *priv_spec;
 char *user_spec;
-- 
2.21.0

Re: [Qemu-devel] [RFC v1 00/23] Add RISC-V Hypervisor Extension

[Alistair Francis]

On Fri, May 24, 2019 at 4:47 PM Alistair Francis
 wrote:
>
> This patch series adds the RISC-V Hypervisor extension 0.3. This is the
> latest draft spec of the Hypervisor extension.

Argh! I forgot to CC Atish and Anup, they are CCed now.

Just an FYI I will be on vacation next week, so any replys to this
series might be a little delayed.

Alistair

>
> This series applies ontop of the RISC-V tree as it requires the previous
> Hypervisor extension patches as well as the CPU parsing patches, both of
> which have been accepted to the RISC-V tree. The full Hypervisor support
> is avaliable at my GitHub (see below) which includes all required patches.
> This series won't apply ontop of master.
>
> The Hypervisor extension is disabled by default, so this series should
> result in no changes to anyone using QEMU unless they enable the
> extension. The extention can be enabled with the -cpu property (see
> below).
>
> At the moment the spec does not include information about the mstatush
> register. As it is not in the spec I haven't added it to QEMU. This
> means the extension won't work correctly for 32-bit guests. This should
> be a small fix to add the CSR once the spec is updated.
>
> All testing of this implementation has been done by using the baremetal
> Xvisor Hypervisor. We are able to run two Linux guests (that's all I
> have tried) as guests.
>
> At the moment this spec is in a draft state and is subject to change. As
> QEMU is extreamly useful in early bring up I think it makes sense for
> QEMU to support non-frozen extensions. I would like to decide with this
> series how QEMU will handle all future non-frozen extensions. That is a
> standard way that QEMU users can test future RISC-V extensions while
> still understanding things will change. One idea is just to disable it by
> default, another option is to maybe use the Kconfig to make it a compile
> time option which developers can use. Should we also display a warning
> when running non-frozen extensions?
>
> Thanks to Anup for doing the initial port of Xvisor. The port is avaliable 
> here:
> https://github.com/avpatel/xvisor-next and will run on QEMU.
>
> Also thanks to Atish for implementing the SBI call support in Xvisor and
> for lots of help debugging.
>
> To run this yourself:
>  1. Apply this patch series to QEMU. The latest branch can be found here:
>   
> https://github.com/alistair23/qemu/tree/mainline/alistair/riscv-hyp-work.next
>  2. Get the version of OpenSBI that supports the H extenstion. This can
> be found here:
>   https://github.com/riscv/opensbi/tree/hyp_ext_changes_v1
>  3. Build the next release of Xvisor. It is avaliable here:
>   https://github.com/avpatel/xvisor-next
>  4. Make sure you build the Xvisor tests, see here for details:
>   
> https://github.com/avpatel/xvisor-next/tree/master/tests/riscv/virt64/linux
>  5. Run QEMU:
>  ./riscv64-softmmu/qemu-system-riscv64 -nographic \
>-machine virt -cpu rv64,h=true\
>-serial mon:stdio -serial null -m 4G \
>-device loader,file=vmm.bin,addr=0x8020 \
>-kernel fw_jump.elf \
>-initrd vmm-disk-linux.img \
>-append "vmm.console=uart@1000 vmm.bootcmd=\"vfs mount initrd 
> /;vfs run /boot.xscript;vfs cat /system/banner.txt\""
>
>Once you get to the prompt you can start the geust by running:
>  guest kick guest0
>You can then bind to the serial port using:
>  vserial bind guest0/uart0
>Then you can start Linux using:
>  autoexec
>
>  This was all tested with the mainline 5.1 kernel. I don't know if it
>  will work on older kernels.
>
> So far all of the QEMU work has been tested on Xvisor.
>
> Known Issues/TODO:
>  - Add mstatush to support 32-bit Hypervisors
>  - Add support for bsstatus.FS and sstatus.FS from the Hypervisor spec
>  - Fix the random hang that sometimes appears when running a Hypervisor guest
>
> There is also on going work from Anup to port KVM.
> We have code complete implementation of RISC-V KVM kernel module and
> RISC-V KVMTOOL. Currently, we are debugging KVM on QEMU and we will
> send-out RFC PATCHES for KVM in June/July 2019.
> The KVM RISC-V kernel module is available in riscv_kvm_v1
> branch at: https://github.com/avpatel/linux.git
> The KVMTOOL RISC-V port is available in riscv_v1 branch of
> https://github.com/avpatel/kvmtool.git
>
> There is very early work on a Xen port as well which is avaliable here:
> https://github.com/alistair23/xen/tree/alistair/riscv-port
>
> Alistair Francis (23):
>   target/riscv: Don't set write permissions on dirty PTEs
>   target/riscv: Add the Hypervisor extension
>   target/riscv: Add the virtulisation mode
>   target/riscv: Add the force HS exception mode
>   target/riscv: Add the Hypervisor CSRs to CPUState
>   target/riscv: Dump Hypervisor registers if enabled
>   target/riscv: Remove strict perm checking for CSR R/W
>   target/riscv: Add support for background interrupt setting
>   target/riscv: Add Hypervisor CSR 

Re: [Qemu-devel] Patch causing qemu-system-ppc to crash

[Programmingkid]

> On May 24, 2019, at 2:56 AM, Mark Cave-Ayland  
> wrote:
> 
> On 24/05/2019 03:48, Programmingkid wrote:
> 
>> Recently I have noticed that qemu-system-ppc is crashing while booting up my 
>> Mac OS X VM. A bit of git bisecting shows this is the patch that causes this 
>> issue:
>> 
>> commit 1e262b49b5331441f697461e4305fe06719758a7
>> Author: Richard Henderson 
>> Date:   Mon Mar 18 12:02:54 2019 -0700
>> 
>>tcg/i386: Implement tcg_out_dupm_vec
>> 
>>At the same time, improve tcg_out_dupi_vec wrt broadcast
>>from the constant pool.
>> 
>>Signed-off-by: Richard Henderson  
> I think this is the same issue I reported last week - does the patch at
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg03767.html help at 
> all?
> 
> 
> ATB,
> 
> Mark.

I think it fixed the problem. I just did a 'git pull' and the result was a 
working qemu-system-ppc. This patch is present in the history.

Thank you.

Re: [Qemu-devel] [PATCH 2/3] block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps

[John Snow]

On 5/23/19 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Firstly, no reason to optimize failure path. Then, function name is
> ambiguous: it checks for readonly and similar things, but someone may
> think that it will ignore normal bitmaps which was just unchanged, and
> this is in bad relation with the fact that we should drop IN_USE flag
> for unchanged bitmaps in the image.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  include/block/dirty-bitmap.h |  1 -
>  block/dirty-bitmap.c | 12 
>  block/qcow2-bitmap.c | 23 +--
>  3 files changed, 13 insertions(+), 23 deletions(-)
> 
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8044ace63e..816022972b 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -105,7 +105,6 @@ bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
>  bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
>  bool bdrv_dirty_bitmap_get_persistence(BdrvDirtyBitmap *bitmap);
>  bool bdrv_dirty_bitmap_inconsistent(const BdrvDirtyBitmap *bitmap);
> -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs);
>  BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>  BdrvDirtyBitmap *bitmap);
>  char *bdrv_dirty_bitmap_sha256(const BdrvDirtyBitmap *bitmap, Error **errp);
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 59e6ebb861..eca2eed0bf 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -775,18 +775,6 @@ bool bdrv_dirty_bitmap_inconsistent(const 
> BdrvDirtyBitmap *bitmap)
>  return bitmap->inconsistent;
>  }
>  
> -bool bdrv_has_changed_persistent_bitmaps(BlockDriverState *bs)
> -{
> -BdrvDirtyBitmap *bm;
> -QLIST_FOREACH(bm, >dirty_bitmaps, list) {
> -if (bm->persistent && !bm->readonly && !bm->migration) {

The loop down below has these conditionals for skipping bitmaps:

if (!bdrv_dirty_bitmap_get_persistence(bitmap) ||
bdrv_dirty_bitmap_readonly(bitmap) ||
bdrv_dirty_bitmap_inconsistent(bitmap)) {
continue;
}

It looks like a semantic change, but hiding inside of get_persistence is
this:

bitmap->persistent && !bitmap->migration;

So this is equivalent, actually. It's not readily apparent at a glance.

> -return true;
> -}
> -}
> -
> -return false;
> -}
> -
>  BdrvDirtyBitmap *bdrv_dirty_bitmap_next(BlockDriverState *bs,
>  BdrvDirtyBitmap *bitmap)
>  {
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 8a75366c92..2b84bfa007 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1457,16 +1457,7 @@ void 
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>  Qcow2Bitmap *bm;
>  QSIMPLEQ_HEAD(, Qcow2BitmapTable) drop_tables;
>  Qcow2BitmapTable *tb, *tb_next;
> -
> -if (!bdrv_has_changed_persistent_bitmaps(bs)) {
> -/* nothing to do */
> -return;
> -}
> -
> -if (!can_write(bs)) {
> -error_setg(errp, "No write access");
> -return;
> -}
> +bool need_write = false;
>  
>  QSIMPLEQ_INIT(_tables);
>  
> @@ -1494,6 +1485,8 @@ void 
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>  continue;
>  }
>  
> +need_write = true;
> +
>  if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) {
>  error_prepend(errp, "Bitmap '%s' doesn't satisfy the 
> constraints: ",
>name);
> @@ -1532,6 +1525,15 @@ void 
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>  bm->dirty_bitmap = bitmap;
>  }
>  
> +if (!need_write) {
> +goto success;
> +}
> +
> +if (!can_write(bs)) {
> +error_setg(errp, "No write access");
> +goto fail;
> +}
> +
>  /* allocate clusters and store bitmaps */
>  QSIMPLEQ_FOREACH(bm, bm_list, entry) {
>  if (bm->dirty_bitmap == NULL) {
> @@ -1573,6 +1575,7 @@ void 
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
>  bdrv_release_dirty_bitmap(bs, bm->dirty_bitmap);
>  }
>  
> +success:
>  bitmap_list_free(bm_list);
>  return;
>  
> 

Alright, interesting. You're right that the function you're removing is
pretty badly named for what it actually does. I got confused by that not
too long ago.

The rationale against optimizing the error path works for me, too.

Seems like it's equivalent before and after, so:

Reviewed-by: John Snow 

Re: [Qemu-devel] [PATCH 1/3] iotests: add test 255 to check bitmap life after snapshot + commit

[John Snow]

On 5/23/19 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Two testcases with persistent bitmaps are not added here, as there are
> bugs to be fixed soon.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  python/qemu/__init__.py|  4 +-
>  tests/qemu-iotests/255 | 81 ++
>  tests/qemu-iotests/255.out | 17 
>  tests/qemu-iotests/group   |  1 +
>  4 files changed, 102 insertions(+), 1 deletion(-)
>  create mode 100755 tests/qemu-iotests/255
>  create mode 100644 tests/qemu-iotests/255.out
> 
> diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
> index 81d9657ec0..4c4317a55e 100644
> --- a/python/qemu/__init__.py
> +++ b/python/qemu/__init__.py
> @@ -402,7 +402,7 @@ class QEMUMachine(object):
>  self._qmp.clear_events()
>  return events
>  
> -def event_wait(self, name, timeout=60.0, match=None):
> +def event_wait(self, name, timeout=60.0, match=None, fail_on=None):
>  """
>  Wait for specified timeout on named event in QMP; optionally filter
>  results by match.
> @@ -430,6 +430,7 @@ class QEMUMachine(object):
>  
>  # Search cached events
>  for event in self._events:
> +assert event['event'] != fail_on
>  if (event['event'] == name) and event_match(event, match):
>  self._events.remove(event)
>  return event
> @@ -437,6 +438,7 @@ class QEMUMachine(object):
>  # Poll for new events
>  while True:
>  event = self._qmp.pull_event(wait=timeout)
> +assert event['event'] != fail_on
>  if (event['event'] == name) and event_match(event, match):
>  return event
>  self._events.append(event)

I'd rather not put assertions directly in the QEMUMachine code, unless
it's actually an assertion and not a test failure, because this code is
not necessarily meant to be used exclusively for tests.

> diff --git a/tests/qemu-iotests/255 b/tests/qemu-iotests/255
> new file mode 100755
> index 00..36712689d3
> --- /dev/null
> +++ b/tests/qemu-iotests/255
> @@ -0,0 +1,81 @@
> +#!/usr/bin/env python
> +#
> +# Tests for temporary external snapshot when we have bitmaps.
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.
> +#
> +# 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 of the License, or
> +# (at your option) any later version.
> +#
> +# 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 .
> +#
> +
> +import iotests
> +from iotests import qemu_img_create, file_path, log
> +
> +iotests.verify_image_format(supported_fmts=['qcow2'])
> +
> +base, top = file_path('base', 'top')
> +size = 64 * 1024 * 3
> +
> +
> +def print_bitmap(msg, vm):
> +result = vm.qmp('query-block')['return'][0]
> +if 'dirty-bitmaps' in result:
> +bitmap = result['dirty-bitmaps'][0]
> +log('{}: name={} dirty-clusters={}'.format(msg, bitmap['name'],
> +bitmap['count'] // 64 // 1024))
> +else:
> +log(msg + ': not found')
> +
> +
> +def test(persistent, restart):
> +assert persistent or not restart
> +log("\nTestcase {}persistent {} restart\n".format(
> +'' if persistent else 'non-', 'with' if restart else 'without'))
> +
> +qemu_img_create('-f', iotests.imgfmt, base, str(size))
> +
> +vm = iotests.VM().add_drive(base)
> +vm.launch()
> +
> +vm.qmp_log('block-dirty-bitmap-add', node='drive0', name='bitmap0',
> +   persistent=persistent)
> +vm.hmp_qemu_io('drive0', 'write 0 64K')
> +print_bitmap('initial bitmap', vm)
> +
> +vm.qmp_log('blockdev-snapshot-sync', device='drive0', snapshot_file=top,
> +   format=iotests.imgfmt, filters=[iotests.filter_qmp_testfiles])
> +vm.hmp_qemu_io('drive0', 'write 64K 512')
> +print_bitmap('check, that no bitmaps in snapshot', vm)

'check that no bitmaps are in snapshot', probably.

> +
> +if restart:
> +log("... Restart ...")
> +vm.shutdown()
> +vm = iotests.VM().add_drive(top)
> +vm.launch()
> +
> +vm.qmp_log('block-commit', device='drive0', top=top,
> +   filters=[iotests.filter_qmp_testfiles])
> +log(vm.event_wait('BLOCK_JOB_READY', fail_on='BLOCK_JOB_COMPLETED'),
> +filters=[iotests.filter_qmp_event])

Looks like you probably saw some interesting things during development.
You shouldn't see COMPLETED before READY, should you? Is this necessary?

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Eduardo Habkost]

On Fri, May 24, 2019 at 10:32:36PM +0200, Aleksandar Markovic wrote:
> On May 24, 2019 9:40 PM, "Eduardo Habkost"  wrote:
> >
> > On Thu, May 23, 2019 at 07:27:35PM +0200, Philippe Mathieu-Daudé wrote:
> > > On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> > > > On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
> > > >>> From: "Aleksandar Markovic" 
> > > >>> On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> > > > From: "Eduardo Habkost" 
> > > >
> > > > Until we actually have a mechanism to exclude the test case on
> > > > travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > > > please don't merge patch 4/4 yet or it will break travis-ci.
> > > >
> > > > Cleber, Wainer, is it already possible to make "avocado run" skip
> > > > tests tagged with "slow"?
> > > >
> > > 
> > >  The mechanism exists, but we haven't tagged any test so far as
> slow.
> > > 
> > > >>>
> > > >>> Cleber,
> > > >>>
> > > >>> For the test from patch 4/4, there is no dilemma - it should be in
> the
> > > >>> “slow” group, as Philippe envisioned and said, so that it is not
> > > > humpered
> > > >>> with stricter requirements for “fast” (default) group. Could you
> > > > explain us
> > > >>> how to do it, so that we can hopefully finally proceed?
> > > >>>
> > > >>
> > > >> Hi Aleksandar,
> > > >>
> > > >> The point is that there's no "group" definition at this point.  This
> is
> > > > the
> > > >> core of the discussion.
> > > >>
> > > >> I think we're close to converging to something simple and effective.
> > > > Please
> > > >> let us know what you think of the proposals given.
> > > >>
> > > >> Thanks!
> > > >> - Cleber.
> > > >>
> > > >
> > > > Cleber, hi.
> > > >
> > > > Thanks for responding.
> > > >
> > > > My views are very similar to Philippe's, but I will provide you with
> more
> > > > details of our (mips) perspective.
> > > >
> > > > As far as black/whitelist issues that is a moot point for us - we
> only want
> > > > to be able to have a way to tag a test within the test itself (so,
> without
> > > > updating some common files, external lists,etc.)
> > > >
> > > > In general, we would like to have a test environment where we would
> be able
> > > > to test what WE deem suitable for us, without feeling that we bother
> you or
> > > > anybody else, or that we are bothered by others.
> > > >
> > > > Let me give you a little extreme example: Let's say we want a complex
> test
> > > > that downloads components from let's say fifty internet location,
> executes
> > > > zillion test cases, and last two days. I wouldn't like anybody to ask
> me
> > > > “Why would you that?” or tell me “You can't do this.” or say “No, we
> did
> > > > not anticipate such tests, patch rejected.” I (we, people from mips)
> should
> > > > be able to define what I (we) need.
> > >
> > > Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
> > > maintains tests/tcg/mips/). We should try to keep contribution upstream,
> > > so good idea/pattern can be reused by others.
> > >
> > > What I'd like to have with those tests is, at least:
> > >
> > > 1/ we don't need to run all the tests (but there is a set of 'quick'
> > > tests we can run on daily basis)
> > >
> > > 2/ maintainers can run their default tests easily (using a combination
> > > of Avocado tags)
> > >
> > > 3/ if a developer working on the PCI subsystem has to modify the MIPS
> > > subsystem (for example), he should be able to run the MIPS tests before
> > > sending his series.
> >
> > Keeping the test cases organized in subdirectories are a good
> > idea, but don't think this is going to help us when we need to
> > quickly enable/disable specific test cases on some CI systems.
> >
> 
> Well, Eduardo, nobody said that directory locations should be used for
> enabling/disabling or tagging/untagging tests in the first place. I think
> it was clear for everybody from the outset that these features should have
> their own mechanisms, which Cleber says already exist, but can't be used
> because the test group still can't figure out (in some hamletesque way)
> whether to blacklist or to whitelist, or how to name the tag for travis,
> and tag for not travis, and if such tags should even exist, etc. - that is
> my layman impression from recent discussions. And now when Philippe
> suggested (in my opinion logical and reasonable) subdirectory, an endless
> discussion begins: “To subdirectory or not to subdirectory? That is the
> question.” Meanwhile, 4.1 is inexorably getting closer and closer, and with
> each day, the value of any potential tests is decreasing.

I understand that seeing the discussions going on and the patches
taking too long to be included might be frustrating.

These discussions shouldn't get into the way of addressing other
problems.  We don't need to wait until all discussions have
finished before proposing new patches or before merging patches
that are considered good.


> 
> Directory structure should 

Re: [Qemu-devel] [PATCH v3] numa: improve cpu hotplug error message with a wrong node-id

[Eduardo Habkost]

On Fri, May 24, 2019 at 04:39:12PM +0200, Laurent Vivier wrote:
> On 24/05/2019 16:10, Igor Mammedov wrote:
> > On Fri, 24 May 2019 12:35:21 +0200
> > Laurent Vivier  wrote:
> > 
> > > On pseries, core-ids are strongly binded to a node-id by the command
> > > line option. If an user tries to add a CPU to the wrong node, he has
> > > an error but it is not really helpful:
> > > 
> > >qemu-system-ppc64 ... -smp 1,maxcpus=64,cores=1,threads=1,sockets=1 \
> > >  -numa node,nodeid=0 -numa node,nodeid=1 ...
> > > 
> > >(qemu) device_add power9_v2.0-spapr-cpu-core,core-id=30,node-id=1
> > >Error: node-id=1 must match numa node specified with -numa option
> > > 
> > > This patch improves this error message by giving to the user the good
> > > topology information (node-id, socket-id and thread-id if they are
> > > available) to use with the core-id he's providing:
> > > 
> > >Error: node-id=1 must match numa node specified with -numa option 
> > > 'node-id 0'
> > > 
> > > Signed-off-by: Laurent Vivier 
> > > ---
> > > 
> > > Notes:
> > >  v3: only add the topology to the existing message
> > >  As suggested by Igor replace
> > >Error: core-id 30 can only be plugged into node-id 0
> > >  by
> > >Error: node-id=1 must match numa node specified with -numa 
> > > option 'node-id 0'
> > >  v2: display full topology in the error message
> > > > > >   numa.c | 25 -
> > >   1 file changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/numa.c b/numa.c
> > > index 3875e1efda3a..7882ec294be4 100644
> > > --- a/numa.c
> > > +++ b/numa.c
> > > @@ -458,6 +458,27 @@ void qmp_set_numa_node(NumaOptions *cmd, Error 
> > > **errp)
> > >   set_numa_options(MACHINE(qdev_get_machine()), cmd, errp);
> > >   }
> > > +static char *cpu_topology_to_string(const CPUArchId *cpu)
> > > +{
> > > +GString *s = g_string_new(NULL);
> > > +if (cpu->props.has_socket_id) {
> > > +g_string_append_printf(s, "socket-id %"PRId64, 
> > > cpu->props.socket_id);
> > > +}
> > > +if (cpu->props.has_node_id) {
> > > +if (s->len) {
> > > +g_string_append_printf(s, ", ");
> > > +}
> > > +g_string_append_printf(s, "node-id %"PRId64, cpu->props.node_id);
> > > +}
> > > +if (cpu->props.has_thread_id) {
> > > +if (s->len) {
> > > +g_string_append_printf(s, ", ");
> > > +}
> > > +g_string_append_printf(s, "thread-id %"PRId64, 
> > > cpu->props.thread_id);
> > > +}
> > > +return g_string_free(s, false);
> > > +}
> > 
> > turns out we already have such helper: cpu_slot_to_string()
> 
> It doesn't display the node-id but the core-id. And node-id is what we need
> to know.

I'm confused about what you are trying to do here.

On v1, the message looked like:
  Error: core-id 30 can only be plugged into node-id 0

which is probably good for spapr.


Then I suggested you added the other cpu->props fields.  e.g. on
PC the message would look like:
  Error: socket-id 20, core-id 30, thread-id 40 can only be plugged into 
node-id 0


But you sent a v2 patch that would print this on PC:
  Error: core-id 30 can only be plugged into socket-id 20, node-id 0, thread-id 
40

which doesn't make sense to me.


Then in a reply to v2, Igor suggested:

 error_setg(errp, "node-id=%d must match numa node specified "
   "with -numa option '%s'", node_id, topology);


Igor suggest would address the problem above.  I expected it to become:
  node-id=0 must match numa node specified with -numa option core-id=30
and on PC:
  node-id=0 must match numa node specified with -numa option 
socket-id=20,core-id=30,thread-id=40

Or maybe it could include the input node-id too:
  node-id=0 must match numa node specified with -numa option 
node-id=1,core-id=30
and on PC:
  node-id=0 must match numa node specified with -numa option 
node-id=1,socket-id=20,core-id=30,thread-id=40

Both options would work.


But you implemented code that would print:
  Error: node-id=0 must match numa node specified with -numa option 'node-id 1'
and on PC it would print:
  Error: node-id=0 must match numa node specified with -numa option 'socket-id 
20 node-id 1 thread-id=40'

which doesn't make sense to me.


I was expecting something like:
  Error: CPU slot core-id=30 is bound to node-id 0, but node-id 1 was specified
and on PC:
  Error: CPU slot socket-id=20,core-id=30,thread-id=40 is bound to node-id 0, 
but node-id 1 was specified


-- 
Eduardo

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Aleksandar Markovic]

On May 24, 2019 9:40 PM, "Eduardo Habkost"  wrote:
>
> On Thu, May 23, 2019 at 07:27:35PM +0200, Philippe Mathieu-Daudé wrote:
> > On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> > > On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
> > >>> From: "Aleksandar Markovic" 
> > >>> On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> > > From: "Eduardo Habkost" 
> > >
> > > Until we actually have a mechanism to exclude the test case on
> > > travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > > please don't merge patch 4/4 yet or it will break travis-ci.
> > >
> > > Cleber, Wainer, is it already possible to make "avocado run" skip
> > > tests tagged with "slow"?
> > >
> > 
> >  The mechanism exists, but we haven't tagged any test so far as
slow.
> > 
> > >>>
> > >>> Cleber,
> > >>>
> > >>> For the test from patch 4/4, there is no dilemma - it should be in
the
> > >>> “slow” group, as Philippe envisioned and said, so that it is not
> > > humpered
> > >>> with stricter requirements for “fast” (default) group. Could you
> > > explain us
> > >>> how to do it, so that we can hopefully finally proceed?
> > >>>
> > >>
> > >> Hi Aleksandar,
> > >>
> > >> The point is that there's no "group" definition at this point.  This
is
> > > the
> > >> core of the discussion.
> > >>
> > >> I think we're close to converging to something simple and effective.
> > > Please
> > >> let us know what you think of the proposals given.
> > >>
> > >> Thanks!
> > >> - Cleber.
> > >>
> > >
> > > Cleber, hi.
> > >
> > > Thanks for responding.
> > >
> > > My views are very similar to Philippe's, but I will provide you with
more
> > > details of our (mips) perspective.
> > >
> > > As far as black/whitelist issues that is a moot point for us - we
only want
> > > to be able to have a way to tag a test within the test itself (so,
without
> > > updating some common files, external lists,etc.)
> > >
> > > In general, we would like to have a test environment where we would
be able
> > > to test what WE deem suitable for us, without feeling that we bother
you or
> > > anybody else, or that we are bothered by others.
> > >
> > > Let me give you a little extreme example: Let's say we want a complex
test
> > > that downloads components from let's say fifty internet location,
executes
> > > zillion test cases, and last two days. I wouldn't like anybody to ask
me
> > > “Why would you that?” or tell me “You can't do this.” or say “No, we
did
> > > not anticipate such tests, patch rejected.” I (we, people from mips)
should
> > > be able to define what I (we) need.
> >
> > Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
> > maintains tests/tcg/mips/). We should try to keep contribution upstream,
> > so good idea/pattern can be reused by others.
> >
> > What I'd like to have with those tests is, at least:
> >
> > 1/ we don't need to run all the tests (but there is a set of 'quick'
> > tests we can run on daily basis)
> >
> > 2/ maintainers can run their default tests easily (using a combination
> > of Avocado tags)
> >
> > 3/ if a developer working on the PCI subsystem has to modify the MIPS
> > subsystem (for example), he should be able to run the MIPS tests before
> > sending his series.
>
> Keeping the test cases organized in subdirectories are a good
> idea, but don't think this is going to help us when we need to
> quickly enable/disable specific test cases on some CI systems.
>

Well, Eduardo, nobody said that directory locations should be used for
enabling/disabling or tagging/untagging tests in the first place. I think
it was clear for everybody from the outset that these features should have
their own mechanisms, which Cleber says already exist, but can't be used
because the test group still can't figure out (in some hamletesque way)
whether to blacklist or to whitelist, or how to name the tag for travis,
and tag for not travis, and if such tags should even exist, etc. - that is
my layman impression from recent discussions. And now when Philippe
suggested (in my opinion logical and reasonable) subdirectory, an endless
discussion begins: “To subdirectory or not to subdirectory? That is the
question.” Meanwhile, 4.1 is inexorably getting closer and closer, and with
each day, the value of any potential tests is decreasing.

Directory structure should be used in its usual and basic way: for
clustering files of similar nature, purpose, or origin, and I do certainly
support any reasonable subdirectory organization for your directory - and
you should think about it, and probably while doing that consult a little
bit other people from all walks of QEMU. We are ready to comply with your
final decision.

The good thing is that nothing is set in stone, everything can be changed
and improved, moving files is easy in git.

All that said, many thanks for reviewing patch 4/4.

Aleksandar



> Disabling a test case (or an entire category of test cases) known
> to be failing on some CI 

Re: [Qemu-devel] Introducing GSoC project: API Documentation Generation

[Paolo Bonzini]

On 24/05/19 21:08, Eduardo Habkost wrote:
> On Fri, May 24, 2019 at 08:34:23PM +0200, Paolo Bonzini wrote:
>> On 23/05/19 14:20, John Snow wrote:
>>> OK, if that's where we're at! I just saw the RFC from Peter Maydell and
>>> assumed we were a little further along the decision making process, but
>>> maybe not. I'll stay tuned.
>>
>> For the decision making, yes; I think there's consensus to use
>> kerneldoc.  For the "debugging and seeing if anything has changed in 2.5
>> years", no.
>>
>> Testing the patch that Eduardo posted will help Gabriel, Eduardo and
>> everyone else decide whether to patch kerneldoc or rather change the API
>> doc comments style.  (Personally I am in favor of patching; the
>> different coding conventions make using vanilla kerneldoc awkward, and
>> there are several thousands of lines of existing doc comments which
>> would require a transition.)
> 
> I'd prefer to fix our doc comments instead of patching kerneldoc,
> whenever possible.  We don't even have a consistent doc comment
> style in QEMU.

I think we *mostly* do, at least as far as the @/#/% sigils are
concerned.  In particular, only "#" separates the QEMU doc comment style
from the kernel and it has 200+ instances vs. 6 for the kernel's
' foo' (all in accel/tcg/translate-all.c), so it's clear that our
standard is different from the kernel in this respect.

The rest of the patch is to handle typedefed structs, which again is
more or less a necessity.

Paolo

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[David Hildenbrand]

On 24.05.19 21:45, Christian Borntraeger wrote:
> 
> 
> On 24.05.19 21:00, David Hildenbrand wrote:
>> On 24.05.19 20:36, David Hildenbrand wrote:
>>> On 24.05.19 20:28, Christian Borntraeger wrote:


 On 24.05.19 20:04, David Hildenbrand wrote:
> On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
>> Hi Christian,
>>
>> I'm having hard time to understand why the S390_IPL object calls
>> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
>> being QOM'ified (it has a reset method).
>>
>> It doesn't seem to have a qdev children added explicitly to it.
>> I see it is used as a singleton, what else am I missing?
>>
>> Thanks,
>>
>> Phil.
>>
>
> Looks like I added it back then (~4 years ago) when converting it into a
> TYPE_DEVICE.
>
> I could imagine that - back then - this was needed because only
> TYPE_SYS_BUS_DEVICE would recursively get reset.

 Yes, back then singleton devices were not recursively resetted. Has that 
 changed?
>>>
>>> Hacking that call out, I don't see it getting called anymore. So it is
>>> still required. The question is if it can be reworked.
>>>
>>
>> Yes, as it is not a sysbus device, it won't get reset.
>> The owner (machine) has to take care of this. The following works:
>>
>>
>> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
>> index b93750c14e..91a31c2cd0 100644
>> --- a/hw/s390x/ipl.c
>> +++ b/hw/s390x/ipl.c
>> @@ -232,7 +232,6 @@ static void s390_ipl_realize(DeviceState *dev, Error 
>> **errp)
>>   */
>>  ipl->compat_start_addr = ipl->start_addr;
>>  ipl->compat_bios_start_addr = ipl->bios_start_addr;
>> -qemu_register_reset(qdev_reset_all_fn, dev);
>>  error:
>>  error_propagate(errp, err);
>>  }
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index bbc6e8fa0b..658ab529a1 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -338,6 +338,11 @@ static inline void s390_do_cpu_ipl(CPUState *cs, 
>> run_on_cpu_data arg)
>>  s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>>  }
>>  
>> +static void s390_ipl_reset(void)
>> +{
>> +qdev_reset_all(DEVICE(object_resolve_path_type("", TYPE_S390_IPL, 
>> NULL)));
>> +}
>> +
>>  static void s390_machine_reset(void)
>>  {
>>  enum s390_reset reset_type;
>> @@ -353,6 +358,7 @@ static void s390_machine_reset(void)
>>  case S390_RESET_EXTERNAL:
>>  case S390_RESET_REIPL:
>>  qemu_devices_reset();
>> +s390_ipl_reset();
>>  s390_crypto_reset();
>>  
>>  /* configure and start the ipl CPU only */
>>
> 
> While this patch is certainly ok, I find it disturbing that qdev devices are 
> being resetted,
> but qom devices not.

I guess the disturbing thing is "... after all these years" :)


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Eduardo Habkost]

On Fri, May 24, 2019 at 03:45:56PM +0200, Aleksandar Markovic wrote:
> On May 23, 2019 11:31 PM, "Eduardo Habkost"  wrote:
> >
> > On Thu, May 23, 2019 at 09:28:00AM -0400, Cleber Rosa wrote:
> > >
> > >
> > > - Original Message -
> > > > From: "Philippe Mathieu-Daudé" 
> > > > To: "Eduardo Habkost" , "Cleber Rosa" <
> cr...@redhat.com>
> > > > Cc: "Aleksandar Rikalo" , "Philippe
> Mathieu-Daudé" , "Wainer dos Santos
> > > > Moschetta" , qemu-devel@nongnu.org, "Aleksandar
> Markovic" ,
> > > > "Aleksandar Markovic" , "Aurelien Jarno" <
> aurel...@aurel32.net>
> > > > Sent: Thursday, May 23, 2019 5:38:34 AM
> > > > Subject: Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests
> > > >
> > > > On 5/23/19 1:07 AM, Eduardo Habkost wrote:
> > > > > On Wed, May 22, 2019 at 05:46:06PM -0400, Cleber Rosa wrote:
> > > > >> - Original Message -
> > > > >>> From: "Eduardo Habkost" 
> > > > >>> On Tue, May 21, 2019 at 01:19:06AM +0200, Philippe Mathieu-Daudé
> wrote:
> > > >  Hi,
> > > > 
> > > >  It was a rainy week-end here, so I invested it to automatize some
> > > >  of my MIPS tests.
> > > > 
> > > >  The BootLinuxSshTest is not Global warming friendly, it is not
> > > >  meant to run on a CI system but rather on a workstation previous
> > > >  to post a pull request.
> > > >  It can surely be improved, but it is a good starting point.
> > > > >>>
> > > > >>> Until we actually have a mechanism to exclude the test case on
> > > > >>> travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > > > >>> please don't merge patch 4/4 yet or it will break travis-ci.
> > > > >>>
> > > > >>> Cleber, Wainer, is it already possible to make "avocado run" skip
> > > > >>> tests tagged with "slow"?
> > > > >>>
> > > > >>
> > > > >> The mechanism exists, but we haven't tagged any test so far as
> slow.
> > > > >>
> > > > >> Should we define/document a criteria for a test to be slow?  Given
> > > > >> that this is highly subjective, we have to think of:
> > > > >>
> > > > >>  * Will we consider the average or maximum run time (the timeout
> > > > >>definition)?
> > > > >>
> > > > >>  * For a single test, what is "slow"? Some rough numbers from
> Travis
> > > > >>CI[1] to help us with guidelines:
> > > > >>- boot_linux_console.py:BootLinuxConsole.test_x86_64_pc:  PASS
> (6.04 s)
> > > > >>- boot_linux_console.py:BootLinuxConsole.test_arm_virt:  PASS
> (2.91 s)
> > > > >>-
> > > > >>
> linux_initrd.py:LinuxInitrd.test_with_2gib_file_should_work_with_linux_v4_16:
> > > > >>PASS (18.14 s)
> > > > >>- boot_linux.py:BootLinuxAarch64.test_virt:  PASS (396.88 s)
> > > > >
> > > > > I don't think we need to overthink this.  Whatever objective
> > > > > criteria we choose, I'm sure we'll have to adapt them later due
> > > > > to real world problems.
> > > > >
> > > > > e.g.: is 396 seconds too slow?  I don't know, it depends: does it
> > > > > break Travis and other CI systems often because of timeouts?  If
> > > > > yes, then we should probably tag it as slow.
> > > > >
> > > > > If having subjective criteria is really a problem (I don't think
> > > > > it is), then we can call the tag "skip_travis", and stop worrying
> > > > > about defining what exactly is "slow".
> > > >
> > > > I'd go with a simpler "tags:travis-ci" whitelisting any job expecting
> to
> > > > run smoothly there.
> > > >
> > >
> > > My concern is what becomes of "make check-acceptance".  Should we
> introduce
> > > another target, say, "make check-acceptance-ci" or just change its
> meaning
> > > and reuse it?
> >
> > What about "make check-acceptance TAG=travis-ci"?
> >
> > >
> > > > Then we can add "slow" tests without having to worry about
> blacklisting
> > > > for Travis CI.
> > > > Also, Other CI can set different timeouts.
> > > >
> > > > I'd like maintainers to add as many tests as they want to upstream, so
> > > > these tests can eventually run by anyone, then each maintainer is free
> > > > to select which particular set he wants to run as default.
> > > >
> > >
> > > OK, so this matches the idea of carefully curating a set of tests for
> > > CI.  WRT white or blacklisting, I favor the approach that requires the
> > > least effort from the developer to have its test enabled, so I'd go
> > > with blacklisting.  I fear that simple tests will just sit on the repo
> > > without being properly exercised if we need to whitelist them.
> > >
> >
> > I agree.  I'd prefer the default case to be simple and not
> > require extra tags.  (i.e. tests without any tags would be run in
> > Travis by default).
> >
> 
> Eduardo,
> 
> You are confusing me here.
> 
> You first suggest:
> 
> > What about "make check-acceptance TAG=travis-ci"?
> 

I was just trying to suggest using make variables as input to
check-acceptance, instead of creating separate makefile rules for
each set of test cases.  But you are right:

> ... and then say:
> 
> > ...tests without any tags would be run in 

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[Christian Borntraeger]

On 24.05.19 21:00, David Hildenbrand wrote:
> On 24.05.19 20:36, David Hildenbrand wrote:
>> On 24.05.19 20:28, Christian Borntraeger wrote:
>>>
>>>
>>> On 24.05.19 20:04, David Hildenbrand wrote:
 On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
> Hi Christian,
>
> I'm having hard time to understand why the S390_IPL object calls
> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
> being QOM'ified (it has a reset method).
>
> It doesn't seem to have a qdev children added explicitly to it.
> I see it is used as a singleton, what else am I missing?
>
> Thanks,
>
> Phil.
>

 Looks like I added it back then (~4 years ago) when converting it into a
 TYPE_DEVICE.

 I could imagine that - back then - this was needed because only
 TYPE_SYS_BUS_DEVICE would recursively get reset.
>>>
>>> Yes, back then singleton devices were not recursively resetted. Has that 
>>> changed?
>>
>> Hacking that call out, I don't see it getting called anymore. So it is
>> still required. The question is if it can be reworked.
>>
> 
> Yes, as it is not a sysbus device, it won't get reset.
> The owner (machine) has to take care of this. The following works:
> 
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index b93750c14e..91a31c2cd0 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -232,7 +232,6 @@ static void s390_ipl_realize(DeviceState *dev, Error 
> **errp)
>   */
>  ipl->compat_start_addr = ipl->start_addr;
>  ipl->compat_bios_start_addr = ipl->bios_start_addr;
> -qemu_register_reset(qdev_reset_all_fn, dev);
>  error:
>  error_propagate(errp, err);
>  }
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index bbc6e8fa0b..658ab529a1 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -338,6 +338,11 @@ static inline void s390_do_cpu_ipl(CPUState *cs, 
> run_on_cpu_data arg)
>  s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
>  }
>  
> +static void s390_ipl_reset(void)
> +{
> +qdev_reset_all(DEVICE(object_resolve_path_type("", TYPE_S390_IPL, 
> NULL)));
> +}
> +
>  static void s390_machine_reset(void)
>  {
>  enum s390_reset reset_type;
> @@ -353,6 +358,7 @@ static void s390_machine_reset(void)
>  case S390_RESET_EXTERNAL:
>  case S390_RESET_REIPL:
>  qemu_devices_reset();
> +s390_ipl_reset();
>  s390_crypto_reset();
>  
>  /* configure and start the ipl CPU only */
> 

While this patch is certainly ok, I find it disturbing that qdev devices are 
being resetted,
but qom devices not.

Re: [Qemu-devel] [PATCH 0/4] mips: Add more Avocado tests

[Eduardo Habkost]

On Thu, May 23, 2019 at 07:27:35PM +0200, Philippe Mathieu-Daudé wrote:
> On 5/23/19 7:11 PM, Aleksandar Markovic wrote:
> > On May 23, 2019 3:45 PM, "Cleber Rosa"  wrote:
> >>> From: "Aleksandar Markovic" 
> >>> On May 22, 2019 11:46 PM, "Cleber Rosa"  wrote:
> > From: "Eduardo Habkost" 
> >
> > Until we actually have a mechanism to exclude the test case on
> > travis-ci, I will remove patch 4/4 from the queue.  Aleksandar,
> > please don't merge patch 4/4 yet or it will break travis-ci.
> >
> > Cleber, Wainer, is it already possible to make "avocado run" skip
> > tests tagged with "slow"?
> >
> 
>  The mechanism exists, but we haven't tagged any test so far as slow.
> 
> >>>
> >>> Cleber,
> >>>
> >>> For the test from patch 4/4, there is no dilemma - it should be in the
> >>> “slow” group, as Philippe envisioned and said, so that it is not
> > humpered
> >>> with stricter requirements for “fast” (default) group. Could you
> > explain us
> >>> how to do it, so that we can hopefully finally proceed?
> >>>
> >>
> >> Hi Aleksandar,
> >>
> >> The point is that there's no "group" definition at this point.  This is
> > the
> >> core of the discussion.
> >>
> >> I think we're close to converging to something simple and effective.
> > Please
> >> let us know what you think of the proposals given.
> >>
> >> Thanks!
> >> - Cleber.
> >>
> > 
> > Cleber, hi.
> > 
> > Thanks for responding.
> > 
> > My views are very similar to Philippe's, but I will provide you with more
> > details of our (mips) perspective.
> > 
> > As far as black/whitelist issues that is a moot point for us - we only want
> > to be able to have a way to tag a test within the test itself (so, without
> > updating some common files, external lists,etc.)
> > 
> > In general, we would like to have a test environment where we would be able
> > to test what WE deem suitable for us, without feeling that we bother you or
> > anybody else, or that we are bothered by others.
> > 
> > Let me give you a little extreme example: Let's say we want a complex test
> > that downloads components from let's say fifty internet location, executes
> > zillion test cases, and last two days. I wouldn't like anybody to ask me
> > “Why would you that?” or tell me “You can't do this.” or say “No, we did
> > not anticipate such tests, patch rejected.” I (we, people from mips) should
> > be able to define what I (we) need.
> 
> Maybe we can use subdirectory like we do for the TCG tests (Aleksandar
> maintains tests/tcg/mips/). We should try to keep contribution upstream,
> so good idea/pattern can be reused by others.
> 
> What I'd like to have with those tests is, at least:
> 
> 1/ we don't need to run all the tests (but there is a set of 'quick'
> tests we can run on daily basis)
> 
> 2/ maintainers can run their default tests easily (using a combination
> of Avocado tags)
> 
> 3/ if a developer working on the PCI subsystem has to modify the MIPS
> subsystem (for example), he should be able to run the MIPS tests before
> sending his series.

Keeping the test cases organized in subdirectories are a good
idea, but don't think this is going to help us when we need to
quickly enable/disable specific test cases on some CI systems.

Disabling a test case (or an entire category of test cases) known
to be failing on some CI systems should require a one line patch,
not moving files to a separate directory.

> 
> > Having such test would be a big deal for me, not only that I could run it
> > manually or automatically every weekend, but I could ask submitters of
> > critical changes: “Did you run this test that we have in Avocado dir?”,
> > without specifying test details, procedures, etc. All this is a BIG deal
> > for me.
> > 
> > On the other hand, I agree that certain group of tests (envisioned for
> > daily or so Travis CI) should have some stricter limitations and structure.
> > But right now I feel humpered by it, and this is counterproductive.
> > 
> > So, we want freedom, responsibility and ownersheep of our tests. Please
> > give us the opportunity to get down on business and start writing tests and
> > start testing.
> > 
> > Yours,
> > Aleksandar

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions

[Eduardo Habkost]

On Thu, May 23, 2019 at 03:03:11PM +0200, Cédric Le Goater wrote:
> On 5/23/19 2:52 PM, Peter Maydell wrote:
> > On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé  
> > wrote:
> >>
> >> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
>  Peter,
> 
>  do you want me to resend with only the two first patches and include
>  Joel's in the same series ? I would leave out the part Philippe is
>  covering in his object_initialize_child() patchset.
> >>>
> >>> Nope, we can not do that, conflicts arise. I suppose the easier is wait
> >>> for Philippe's patchset to be merged and then rebase.
> >>
> >> Eduardo said he'll send a pull request during the week.
> > 
> > I am now completely lost about the status of these patches,
> > so I'm just dropping this series from my to-review queue.
> 
> yes. It's ok.
> 
> > Please send a fresh patchset once any dependencies have
> > got into master.
> 
> I plan to send a larger one once Eduardo's patchset is merged.

I've just submitted a pull request including the
object_initialize_child() patches from Philippe:
  [PULL 00/17] Machine Core queue, 2019-05-24

Sorry for the delay.

-- 
Eduardo

Re: [Qemu-devel] Introducing GSoC project: API Documentation Generation

[Eduardo Habkost]

On Fri, May 24, 2019 at 08:34:23PM +0200, Paolo Bonzini wrote:
> On 23/05/19 14:20, John Snow wrote:
> > OK, if that's where we're at! I just saw the RFC from Peter Maydell and
> > assumed we were a little further along the decision making process, but
> > maybe not. I'll stay tuned.
> 
> For the decision making, yes; I think there's consensus to use
> kerneldoc.  For the "debugging and seeing if anything has changed in 2.5
> years", no.
> 
> Testing the patch that Eduardo posted will help Gabriel, Eduardo and
> everyone else decide whether to patch kerneldoc or rather change the API
> doc comments style.  (Personally I am in favor of patching; the
> different coding conventions make using vanilla kerneldoc awkward, and
> there are several thousands of lines of existing doc comments which
> would require a transition.)

I'd prefer to fix our doc comments instead of patching kerneldoc,
whenever possible.  We don't even have a consistent doc comment
style in QEMU.

-- 
Eduardo

Re: [Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers

[Michael S. Tsirkin]

On Fri, May 24, 2019 at 08:47:18PM +0200, Paolo Bonzini wrote:
> On 24/05/19 20:36, Stefan Hajnoczi wrote:
> > v3:
> >  * Fix s/k->vmstate_change/vdc->vmstate_change/
> >  * Still RFC, waiting for customer to confirm this fixes the issue
> > v2:
> >  * Do it properly with a clean API instead of deferring to a BH!
> >Thanks for encouraging me to do this, Kevin.
> > 
> > These patches solve a deadlock when the 'cont' command is used and there are
> > failed requests on a virtio-scsi device with iothreads.  The deadlock 
> > itself is
> > actually not the thing we need to fix because we should never reach that 
> > case
> > anyway.  Instead we need to make sure DMA restart is only performed after 
> > the
> > virtio-scsi iothread is re-initialized.
> 
> custom_dma_restart is a bit ugly...  Do you think it would make sense to
> order the VMStateChange handlers using some kind of enum (with the order
> unspecified within the category)?
> 
> We could start with
> 
>   VMSTATECHANGE_PRIO_UNKNOWN  = 0  (if needed?)

Yes I think it's a good idea to explicitly say I don't care
about the order like this.

>   VMSTATECHANGE_PRIO_IOTHREAD = 100
> VMSTATECHANGE_PRIO_DEVICE   = 200
> 
> where higher priorities run first on stop and last on resume.
> 
> Paolo

Re: [Qemu-devel] [PATCH v6 05/10] esp: add pseudo-DMA as used by Macintosh

[Laurent Vivier]

On 25/01/2019 06:48, Thomas Huth wrote:

On 2018-11-02 16:22, Mark Cave-Ayland wrote:

From: Laurent Vivier 


I'd suggest to add a patch description that contains the text that
Laurent provided as a reply to this patch in v5:

 8< --
There is no DMA in Quadra 800, so the CPU reads/writes the data from the
PDMA register (offset 0x100, ESP_PDMA in hw/m68k/q800.c) and copies them
to/from the memory.

There is a nice assembly loop in the kernel to do that, see
linux/drivers/scsi/mac_esp.c:MAC_ESP_PDMA_LOOP().

The start of the transfer is triggered by the DREQ interrupt (see linux
mac_esp_send_pdma_cmd()), the CPU polls on the IRQ flag to start the
transfer after a SCSI command has been sent (in Quadra 800 it goes
through the VIA2, the via2-irq line and the vIFR register)

The Macintosh hardware includes hardware handshaking to prevent the CPU
from reading invalid data or writing data faster than the peripheral
device can accept it.

This is the "blind mode", and from the doc:
"Approximate maximum SCSI transfer rates within a blocks are 1.4 MB per
second for blind transfers in the Macintosh II"

Some references can be found in:
   Apple Macintosh Family Hardware Reference, ISBN 0-201-19255-1
   Guide to the Macintosh Family Hardware, ISBN-0-201-52405-8
 >8 --

?


Co-developed-by: Mark Cave-Ayland 
Signed-off-by: Mark Cave-Ayland 
Signed-off-by: Laurent Vivier 
---
  hw/scsi/esp.c | 291 +-
  include/hw/scsi/esp.h |   7 ++
  2 files changed, 269 insertions(+), 29 deletions(-)

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 630d923623..8e9e27e479 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c

[...]

@@ -356,8 +511,7 @@ static void handle_ti(ESPState *s)
  s->dma_left = minlen;
  s->rregs[ESP_RSTAT] &= ~STAT_TC;
  esp_do_dma(s);
-}
-if (s->do_cmd) {
+} else if (s->do_cmd) {


I'm not sure about this change... is it required? It could also change
the behavior of the other users of this device...?



The "else" is needed because this code has been duplicated inside 
esp_do_dma() to be executed only in the case of "real" dma and not for 
pseudo-dma.


Thanks,
Laurent

Re: [Qemu-devel] qapi/misc.json is too big, let's bite off a few chunks

[Eduardo Habkost]

On Thu, May 23, 2019 at 06:14:18PM +0200, Markus Armbruster wrote:
> It's nice when QAPI schema modules clearly belong to a single subsystem
> in addition to "QAPI Schema".  misc.json doesn't, and it's grown fat:
> 3000+ lines.  Let's move out some stuff.  Here are a few candidates:
> 
> * Dump (Marc-André)
> 
>   dump-guest-memory, query-dump, DUMP_COMPLETED,
>   query-dump-guest-memory-capability
> 
>   ~200 lines.
> 
> * Machine core (Eduardo, Marcel)
> 
>   query-machines, query-current-machine, 
> 
>   ~60 lines.  Hardly worthwhile from a "let's shrink misc.json" point of
>   view.  Might be worthwhile from a "let's make get_maintainers.pl
>   work".
> 
> * CPUs (Paolo, Richard)
> 
>   query-cpus, query-cpus-fast
> 
>   ~300 lines.  The commands are implemented in cpus.c, which MAINTAINERS
>   covers both under "Main loop" and under "Guest CPU cores (TCG) /
>   Overall".  Neither feels right to me for these QMP commands.

Should it include query-cpu-* (currently on target.json),
and query-hotpluggable-cpus?

> 
> * NUMA (Eduardo)
> 
>   query-memdev, set-numa-node
> 
>   ~200 lines.
> 
> Opinions?
> 
> Additional candidates?

QOM: qom-list, qom-get, qom-set, qom-list-properties, object-add
object-del.

-- 
Eduardo

Re: [Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers

[Paolo Bonzini]

On 24/05/19 20:36, Stefan Hajnoczi wrote:
> v3:
>  * Fix s/k->vmstate_change/vdc->vmstate_change/
>  * Still RFC, waiting for customer to confirm this fixes the issue
> v2:
>  * Do it properly with a clean API instead of deferring to a BH!
>Thanks for encouraging me to do this, Kevin.
> 
> These patches solve a deadlock when the 'cont' command is used and there are
> failed requests on a virtio-scsi device with iothreads.  The deadlock itself 
> is
> actually not the thing we need to fix because we should never reach that case
> anyway.  Instead we need to make sure DMA restart is only performed after the
> virtio-scsi iothread is re-initialized.

custom_dma_restart is a bit ugly...  Do you think it would make sense to
order the VMStateChange handlers using some kind of enum (with the order
unspecified within the category)?

We could start with

VMSTATECHANGE_PRIO_UNKNOWN  = 0  (if needed?)
VMSTATECHANGE_PRIO_IOTHREAD = 100
VMSTATECHANGE_PRIO_DEVICE   = 200

where higher priorities run first on stop and last on resume.

Paolo

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[David Hildenbrand]

On 24.05.19 20:36, David Hildenbrand wrote:
> On 24.05.19 20:28, Christian Borntraeger wrote:
>>
>>
>> On 24.05.19 20:04, David Hildenbrand wrote:
>>> On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
 Hi Christian,

 I'm having hard time to understand why the S390_IPL object calls
 qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
 being QOM'ified (it has a reset method).

 It doesn't seem to have a qdev children added explicitly to it.
 I see it is used as a singleton, what else am I missing?

 Thanks,

 Phil.

>>>
>>> Looks like I added it back then (~4 years ago) when converting it into a
>>> TYPE_DEVICE.
>>>
>>> I could imagine that - back then - this was needed because only
>>> TYPE_SYS_BUS_DEVICE would recursively get reset.
>>
>> Yes, back then singleton devices were not recursively resetted. Has that 
>> changed?
> 
> Hacking that call out, I don't see it getting called anymore. So it is
> still required. The question is if it can be reworked.
> 

Yes, as it is not a sysbus device, it won't get reset.
The owner (machine) has to take care of this. The following works:


diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index b93750c14e..91a31c2cd0 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -232,7 +232,6 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
  */
 ipl->compat_start_addr = ipl->start_addr;
 ipl->compat_bios_start_addr = ipl->bios_start_addr;
-qemu_register_reset(qdev_reset_all_fn, dev);
 error:
 error_propagate(errp, err);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index bbc6e8fa0b..658ab529a1 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -338,6 +338,11 @@ static inline void s390_do_cpu_ipl(CPUState *cs, 
run_on_cpu_data arg)
 s390_cpu_set_state(S390_CPU_STATE_OPERATING, cpu);
 }
 
+static void s390_ipl_reset(void)
+{
+qdev_reset_all(DEVICE(object_resolve_path_type("", TYPE_S390_IPL, NULL)));
+}
+
 static void s390_machine_reset(void)
 {
 enum s390_reset reset_type;
@@ -353,6 +358,7 @@ static void s390_machine_reset(void)
 case S390_RESET_EXTERNAL:
 case S390_RESET_REIPL:
 qemu_devices_reset();
+s390_ipl_reset();
 s390_crypto_reset();
 
 /* configure and start the ipl CPU only */



-- 

Thanks,

David / dhildenb

[Qemu-devel] [PULL 17/17] hw/intc/nvic: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 + child_size, child_type);

We let NVIC adopt the SysTick timer.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-17-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/intc/armv7m_nvic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 815e720cfa..dc2c206d9a 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -2595,9 +2595,9 @@ static void armv7m_nvic_realize(DeviceState *dev, Error 
**errp)
  * as we didn't know then if the CPU had the security extensions;
  * so we have to do it here.
  */
-object_initialize(>systick[M_REG_S], sizeof(s->systick[M_REG_S]),
-  TYPE_SYSTICK);
-qdev_set_parent_bus(DEVICE(>systick[M_REG_S]), 
sysbus_get_default());
+sysbus_init_child_obj(OBJECT(dev), "systick-reg-s",
+  >systick[M_REG_S],
+  sizeof(s->systick[M_REG_S]), TYPE_SYSTICK);
 
 object_property_set_bool(OBJECT(>systick[M_REG_S]), true,
  "realized", );
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] Running linux on qemu omap

[Aaro Koskinen]

Hi,

On Fri, May 24, 2019 at 06:00:18PM +0300, Aaro Koskinen wrote:
> Please don't delete OMAP boards quite yet :) In the mainline kernel
> they are not orphaned, they frequently get tested using actual hardware,
> and QEMU would help in additional testing. I'll try to get N8x0 boot to
> work with the minimal kernel I use on real HW.

So it was only a matter of attaching the serial console at the QEMU side
(a hackish patch at the end of the mail).

$ qemu-system-arm --version
QEMU emulator version 4.0.0 (v4.0.0-dirty)
Copyright (c) 2003-2019 Fabrice Bellard and the QEMU Project developers
$ qemu-system-arm -M n810 -kernel zImage -nographic
Uncompressing Linux... done, booting the kernel.
[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 5.1.0-n8x0_tiny-los_b1ac4+-7-g7435e73d8ac4 
(aaro@amd-fx-6350) (gcc version 8.3.0 (GCC)) #1 Fri May 24 20:43:02 EEST 2019
[0.00] CPU: ARMv6-compatible processor [4107b362] revision 2 
(ARMv6TEJ), cr=00c5387d
[0.00] CPU: VIPT aliasing data cache, unknown instruction cache
[0.00] OF: fdt: Machine model: Nokia N810
[0.00] printk: bootconsole [earlycon0] enabled
[0.00] Memory policy: Data cache writeback
[0.00] OMAP2420
[...]

However there are plenty of WARNs that are not present on real hardware.
Anyway, it's a start.

A.

...

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 906b7ca22d43..52ff83ec5147 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -792,6 +792,7 @@ static void n8x0_uart_setup(struct n800_s *s)
 qdev_connect_gpio_out(s->mpu->gpio, N8X0_BT_WKUP_GPIO,
 csrhci_pins_get(radio)[csrhci_pin_wakeup]);
 
+omap_uart_attach(s->mpu->uart[2], serial_hd(0));
 omap_uart_attach(s->mpu->uart[BT_UART], radio);
 }
 

[Qemu-devel] [PULL 14/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(then manually modified to use numbered IPI name)

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 + child_size, child_type);

We let the SoC adopt the IPI children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-14-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 20e973edf5..0948b1fd5f 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -71,9 +71,10 @@ static void xlnx_zynqmp_pmu_soc_init(Object *obj)
 
 /* Create the IPI device */
 for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-object_initialize(>ipi[i], sizeof(XlnxZynqMPIPI),
-  TYPE_XLNX_ZYNQMP_IPI);
-qdev_set_parent_bus(DEVICE(>ipi[i]), sysbus_get_default());
+char *name = g_strdup_printf("ipi%d", i);
+sysbus_init_child_obj(obj, name, >ipi[i],
+  sizeof(XlnxZynqMPIPI), TYPE_XLNX_ZYNQMP_IPI);
+g_free(name);
 }
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 15/17] hw/microblaze/zynqmp: Use object_initialize_child for correct ref. counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
?-   object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
?-   object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-15-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 0948b1fd5f..df6c0048aa 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -176,9 +176,9 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 pmu_ram);
 
 /* Create the PMU device */
-object_initialize(pmu, sizeof(XlnxZynqMPPMUSoCState), 
TYPE_XLNX_ZYNQMP_PMU_SOC);
-object_property_add_child(OBJECT(machine), "pmu", OBJECT(pmu),
-  _abort);
+object_initialize_child(OBJECT(machine), "pmu", pmu,
+sizeof(XlnxZynqMPPMUSoCState),
+TYPE_XLNX_ZYNQMP_PMU_SOC, _abort, NULL);
 object_property_set_bool(OBJECT(pmu), true, "realized", _fatal);
 
 /* Load the kernel */
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH] configure: remove tpm_passthrough & tpm_emulator

[Paolo Bonzini]

On 24/05/19 20:14, Marc-André Lureau wrote:
> This is a left-over from commit 7aaa6a16373 "tpm: express dependencies
> with Kconfig".
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/configure b/configure
> index 528b9ff705..4da99ee750 100755
> --- a/configure
> +++ b/configure
> @@ -6434,8 +6434,6 @@ echo "gcov  $gcov_tool"
>  echo "gcov enabled  $gcov"
>  echo "TPM support   $tpm"
>  echo "libssh2 support   $libssh2"
> -echo "TPM passthrough   $tpm_passthrough"
> -echo "TPM emulator  $tpm_emulator"
>  echo "QOM debugging $qom_cast_debug"
>  echo "Live block migration $live_block_migration"
>  echo "lzo support   $lzo"
> @@ -7133,14 +7131,6 @@ fi
>  
>  if test "$tpm" = "yes"; then
>echo 'CONFIG_TPM=$(CONFIG_SOFTMMU)' >> $config_host_mak
> -  # TPM passthrough support?
> -  if test "$tpm_passthrough" = "yes"; then
> -echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
> -  fi
> -  # TPM emulator support?
> -  if test "$tpm_emulator" = "yes"; then
> -echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak
> -  fi
>  fi
>  
>  echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak
> 

Queued, thanks.

Paolo

[Qemu-devel] [PULL 11/17] hw/mips: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 + child_size, child_type);

We let the Malta/Boston machines adopt the CPS child, and similarly
the CPS adopts the ITU/CPC/GIC/GCR children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-11-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 hw/mips/boston.c |  4 ++--
 hw/mips/cps.c| 20 
 hw/mips/mips_malta.c |  4 ++--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index cb3ea85fdc..1ffccc8da9 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -455,8 +455,8 @@ static void boston_mach_init(MachineState *machine)
 
 is_64b = cpu_supports_isa(machine->cpu_type, ISA_MIPS64);
 
-object_initialize(>cps, sizeof(s->cps), TYPE_MIPS_CPS);
-qdev_set_parent_bus(DEVICE(>cps), sysbus_get_default());
+sysbus_init_child_obj(OBJECT(machine), "cps", OBJECT(>cps),
+  sizeof(s->cps), TYPE_MIPS_CPS);
 object_property_set_str(OBJECT(>cps), machine->cpu_type, "cpu-type",
 );
 object_property_set_int(OBJECT(>cps), smp_cpus, "num-vp", );
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index fc97f59af4..649b35a76c 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -94,9 +94,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 
 /* Inter-Thread Communication Unit */
 if (itu_present) {
-object_initialize(>itu, sizeof(s->itu), TYPE_MIPS_ITU);
-qdev_set_parent_bus(DEVICE(>itu), sysbus_get_default());
-
+sysbus_init_child_obj(OBJECT(dev), "itu", >itu, sizeof(s->itu),
+  TYPE_MIPS_ITU);
 object_property_set_int(OBJECT(>itu), 16, "num-fifo", );
 object_property_set_int(OBJECT(>itu), 16, "num-semaphores", );
 object_property_set_bool(OBJECT(>itu), saar_present, "saar-present",
@@ -115,9 +114,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 }
 
 /* Cluster Power Controller */
-object_initialize(>cpc, sizeof(s->cpc), TYPE_MIPS_CPC);
-qdev_set_parent_bus(DEVICE(>cpc), sysbus_get_default());
-
+sysbus_init_child_obj(OBJECT(dev), "cpc", >cpc, sizeof(s->cpc),
+  TYPE_MIPS_CPC);
 object_property_set_int(OBJECT(>cpc), s->num_vp, "num-vp", );
 object_property_set_int(OBJECT(>cpc), 1, "vp-start-running", );
 object_property_set_bool(OBJECT(>cpc), true, "realized", );
@@ -130,9 +128,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 sysbus_mmio_get_region(SYS_BUS_DEVICE(>cpc), 
0));
 
 /* Global Interrupt Controller */
-object_initialize(>gic, sizeof(s->gic), TYPE_MIPS_GIC);
-qdev_set_parent_bus(DEVICE(>gic), sysbus_get_default());
-
+sysbus_init_child_obj(OBJECT(dev), "gic", >gic, sizeof(s->gic),
+  TYPE_MIPS_GIC);
 object_property_set_int(OBJECT(>gic), s->num_vp, "num-vp", );
 object_property_set_int(OBJECT(>gic), 128, "num-irq", );
 object_property_set_bool(OBJECT(>gic), true, "realized", );
@@ -147,9 +144,8 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 /* Global Configuration Registers */
 gcr_base = 

[Qemu-devel] [PULL 10/17] hw/mips: Use object_initialize() on MIPSCPSState

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

Initialize the MIPSCPSState with object_initialize() instead of
object_new(). This will allow us to add it as children of the
machine container.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-10-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 hw/mips/boston.c | 25 -
 hw/mips/mips_malta.c | 17 -
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index a8b29f62f5..cb3ea85fdc 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -49,7 +49,7 @@ typedef struct {
 SysBusDevice parent_obj;
 
 MachineState *mach;
-MIPSCPSState *cps;
+MIPSCPSState cps;
 SerialState *uart;
 
 CharBackend lcd_display;
@@ -188,7 +188,7 @@ static uint64_t boston_platreg_read(void *opaque, hwaddr 
addr,
 case PLAT_DDR3_STATUS:
 return PLAT_DDR3_STATUS_LOCKED | PLAT_DDR3_STATUS_CALIBRATED;
 case PLAT_MMCM_DIV:
-gic_freq = mips_gictimer_get_freq(s->cps->gic.gic_timer) / 100;
+gic_freq = mips_gictimer_get_freq(s->cps.gic.gic_timer) / 100;
 val = gic_freq << PLAT_MMCM_DIV_INPUT_SHIFT;
 val |= 1 << PLAT_MMCM_DIV_MUL_SHIFT;
 val |= 1 << PLAT_MMCM_DIV_CLK0DIV_SHIFT;
@@ -455,20 +455,19 @@ static void boston_mach_init(MachineState *machine)
 
 is_64b = cpu_supports_isa(machine->cpu_type, ISA_MIPS64);
 
-s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
-qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
-
-object_property_set_str(OBJECT(s->cps), machine->cpu_type, "cpu-type",
+object_initialize(>cps, sizeof(s->cps), TYPE_MIPS_CPS);
+qdev_set_parent_bus(DEVICE(>cps), sysbus_get_default());
+object_property_set_str(OBJECT(>cps), machine->cpu_type, "cpu-type",
 );
-object_property_set_int(OBJECT(s->cps), smp_cpus, "num-vp", );
-object_property_set_bool(OBJECT(s->cps), true, "realized", );
+object_property_set_int(OBJECT(>cps), smp_cpus, "num-vp", );
+object_property_set_bool(OBJECT(>cps), true, "realized", );
 
 if (err != NULL) {
 error_report("%s", error_get_pretty(err));
 exit(1);
 }
 
-sysbus_mmio_map_overlap(SYS_BUS_DEVICE(s->cps), 0, 0, 1);
+sysbus_mmio_map_overlap(SYS_BUS_DEVICE(>cps), 0, 0, 1);
 
 flash =  g_new(MemoryRegion, 1);
 memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB, );
@@ -487,17 +486,17 @@ static void boston_mach_init(MachineState *machine)
 xilinx_pcie_init(sys_mem, 0,
  0x1000, 32 * MiB,
  0x4000, 1 * GiB,
- get_cps_irq(s->cps, 2), false);
+ get_cps_irq(>cps, 2), false);
 
 xilinx_pcie_init(sys_mem, 1,
  0x1200, 32 * MiB,
  0x2000, 512 * MiB,
- get_cps_irq(s->cps, 1), false);
+ get_cps_irq(>cps, 1), false);
 
 pcie2 = xilinx_pcie_init(sys_mem, 2,
  0x1400, 32 * MiB,
  0x1600, 1 * MiB,
- get_cps_irq(s->cps, 0), true);
+ get_cps_irq(>cps, 0), true);
 
 platreg = g_new(MemoryRegion, 1);
 memory_region_init_io(platreg, NULL, _platreg_ops, s,
@@ -505,7 +504,7 @@ static void boston_mach_init(MachineState *machine)
 memory_region_add_subregion_overlap(sys_mem, 0x17ffd000, platreg, 0);
 
 s->uart = serial_mm_init(sys_mem, 0x17ffe000, 2,
- get_cps_irq(s->cps, 3), 1000,
+ get_cps_irq(>cps, 3), 1000,
  serial_hd(0), DEVICE_NATIVE_ENDIAN);
 
 lcd = g_new(MemoryRegion, 1);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 439665ab45..04f2117d71 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -94,7 +94,7 @@ typedef struct {
 typedef struct {
 SysBusDevice parent_obj;
 
-MIPSCPSState *cps;
+MIPSCPSState cps;
 qemu_irq *i8259;
 } MaltaState;
 
@@ -1151,20 +1151,19 @@ static void create_cps(MaltaState *s, const char 
*cpu_type,
 {
 Error *err = NULL;
 
-s->cps = MIPS_CPS(object_new(TYPE_MIPS_CPS));
-qdev_set_parent_bus(DEVICE(s->cps), sysbus_get_default());
-
-object_property_set_str(OBJECT(s->cps), cpu_type, "cpu-type", );
-object_property_set_int(OBJECT(s->cps), smp_cpus, "num-vp", );
-object_property_set_bool(OBJECT(s->cps), true, "realized", );
+object_initialize(>cps, sizeof(s->cps), TYPE_MIPS_CPS);
+qdev_set_parent_bus(DEVICE(>cps), sysbus_get_default());
+object_property_set_str(OBJECT(>cps), cpu_type, "cpu-type", );
+object_property_set_int(OBJECT(>cps), smp_cpus, "num-vp", );
+object_property_set_bool(OBJECT(>cps), true, "realized", );
 if (err != NULL) {
 error_report("%s", error_get_pretty(err));
 exit(1);

[Qemu-devel] [PULL 09/17] hw/arm: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-9-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/arm/digic.c   | 17 ++---
 hw/arm/imx25_pdk.c   |  5 ++---
 hw/arm/kzm.c |  5 ++---
 hw/arm/raspi.c   |  7 +++
 hw/arm/sabrelite.c   |  5 ++---
 hw/arm/xlnx-zcu102.c |  5 ++---
 hw/arm/xlnx-zynqmp.c |  8 
 7 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 726abb9b48..6ef26c6bac 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -32,27 +32,22 @@
 static void digic_init(Object *obj)
 {
 DigicState *s = DIGIC(obj);
-DeviceState *dev;
 int i;
 
-object_initialize(>cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
-object_property_add_child(obj, "cpu", OBJECT(>cpu), NULL);
+object_initialize_child(obj, "cpu", >cpu, sizeof(s->cpu),
+"arm946-" TYPE_ARM_CPU, _abort, NULL);
 
 for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
 #define DIGIC_TIMER_NAME_MLEN11
 char name[DIGIC_TIMER_NAME_MLEN];
 
-object_initialize(>timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
-dev = DEVICE(>timer[i]);
-qdev_set_parent_bus(dev, sysbus_get_default());
 snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
-object_property_add_child(obj, name, OBJECT(>timer[i]), NULL);
+sysbus_init_child_obj(obj, name, >timer[i], sizeof(s->timer[i]),
+  TYPE_DIGIC_TIMER);
 }
 
-object_initialize(>uart, sizeof(s->uart), TYPE_DIGIC_UART);
-dev = DEVICE(>uart);
-qdev_set_parent_bus(dev, sysbus_get_default());
-object_property_add_child(obj, "uart", OBJECT(>uart), NULL);
+sysbus_init_child_obj(obj, "uart", >uart, sizeof(s->uart),
+  

[Qemu-devel] [PULL 08/17] hw/arm/aspeed: Use object_initialize_child for correct ref. counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Signed-off-by: Cédric Le Goater 
Reviewed-by: Joel Stanley 
Message-Id: <20190507163416.24647-8-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 hw/arm/aspeed.c |  6 +++---
 hw/arm/aspeed_soc.c | 50 ++---
 2 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 415cff7a01..33070a6df8 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -160,9 +160,9 @@ static void aspeed_board_init(MachineState *machine,
 ram_addr_t max_ram_size;
 
 bmc = g_new0(AspeedBoardState, 1);
-object_initialize(>soc, (sizeof(bmc->soc)), cfg->soc_name);
-object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
-  _abort);
+object_initialize_child(OBJECT(machine), "soc", >soc,
+(sizeof(bmc->soc)), cfg->soc_name, _abort,
+NULL);
 
 sc = ASPEED_SOC_GET_CLASS(>soc);
 
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index a27233d487..faff42b84a 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -106,12 +106,11 @@ static void aspeed_soc_init(Object *obj)
 AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
 int i;
 
-object_initialize(>cpu, sizeof(s->cpu), sc->info->cpu_type);
-object_property_add_child(obj, "cpu", OBJECT(>cpu), NULL);
+object_initialize_child(obj, "cpu", OBJECT(>cpu), sizeof(s->cpu),
+sc->info->cpu_type, _abort, NULL);
 
-object_initialize(>scu, sizeof(s->scu), TYPE_ASPEED_SCU);
-object_property_add_child(obj, "scu", OBJECT(>scu), NULL);
-qdev_set_parent_bus(DEVICE(>scu), sysbus_get_default());
+sysbus_init_child_obj(obj, "scu", OBJECT(>scu), sizeof(s->scu),
+  

[Qemu-devel] [PULL 16/17] hw/arm/mps2: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_sysbus_init_child_obj_missing_parent@
 expression child_ptr;
 expression child_type;
 expression child_size;
 @@
 -   object_initialize(child_ptr, child_size, child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 ...
 ?-  object_unref(OBJECT(child_ptr));
 +   sysbus_init_child_obj(OBJECT(PARENT_OBJ), "CHILD_NAME", child_ptr,
 + child_size, child_type);

We let the MPS2 boards adopt the cpu core, the FPGA and the SCC children.

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-16-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Signed-off-by: Eduardo Habkost 
---
 hw/arm/mps2-tz.c | 8 
 hw/arm/mps2.c| 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index c167a5fa59..d85dc2c4bd 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -214,9 +214,9 @@ static MemoryRegion *make_scc(MPS2TZMachineState *mms, void 
*opaque,
 DeviceState *sccdev;
 MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
 
-object_initialize(scc, sizeof(mms->scc), TYPE_MPS2_SCC);
+sysbus_init_child_obj(OBJECT(mms), "scc", scc,
+  sizeof(mms->scc), TYPE_MPS2_SCC);
 sccdev = DEVICE(scc);
-qdev_set_parent_bus(sccdev, sysbus_get_default());
 qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
 qdev_prop_set_uint32(sccdev, "scc-aid", 0x0028);
 qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
@@ -229,8 +229,8 @@ static MemoryRegion *make_fpgaio(MPS2TZMachineState *mms, 
void *opaque,
 {
 MPS2FPGAIO *fpgaio = opaque;
 
-object_initialize(fpgaio, sizeof(mms->fpgaio), TYPE_MPS2_FPGAIO);
-qdev_set_parent_bus(DEVICE(fpgaio), sysbus_get_default());
+sysbus_init_child_obj(OBJECT(mms), "fpgaio", fpgaio,
+  sizeof(mms->fpgaio), TYPE_MPS2_FPGAIO);
 object_property_set_bool(OBJECT(fpgaio), true, "realized", _fatal);
 return sysbus_mmio_get_region(SYS_BUS_DEVICE(fpgaio), 0);
 }
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index b74f1378c9..10efff36b2 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -174,9 +174,9 @@ static void mps2_common_init(MachineState *machine)
 g_assert_not_reached();
 }
 
-object_initialize(>armv7m, sizeof(mms->armv7m), TYPE_ARMV7M);
+sysbus_init_child_obj(OBJECT(mms), "armv7m", >armv7m,
+  sizeof(mms->armv7m), TYPE_ARMV7M);
 armv7m = DEVICE(>armv7m);
-qdev_set_parent_bus(armv7m, sysbus_get_default());
 switch (mmc->fpga_type) {
 case FPGA_AN385:
 qdev_prop_set_uint32(armv7m, "num-irq", 32);
@@ -308,9 +308,9 @@ static void mps2_common_init(MachineState *machine)
qdev_get_gpio_in(armv7m, 10));
 sysbus_mmio_map(SYS_BUS_DEVICE(>dualtimer), 0, 0x40002000);
 
-object_initialize(>scc, sizeof(mms->scc), TYPE_MPS2_SCC);
+sysbus_init_child_obj(OBJECT(mms), "scc", >scc,
+  sizeof(mms->scc), TYPE_MPS2_SCC);
 sccdev = DEVICE(>scc);
-qdev_set_parent_bus(sccdev, sysbus_get_default());
 qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
 qdev_prop_set_uint32(sccdev, "scc-aid", 0x0028);
 qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 07/17] hw/arm/bcm2835: Use object_initialize_child for correct ref. counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 )

 @use_sysbus_init_child_obj@
 expression parent_obj;
 expression dev;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 ...
 -   qdev_set_parent_bus(DEVICE(child_ptr), sysbus_get_default());
 |
 -   object_initialize_child(parent_obj, child_name, child_ptr, child_size,
 -   child_type, errp, NULL);
 +   sysbus_init_child_obj(parent_obj, child_name, child_ptr, child_size,
 + child_type);
 -   dev = DEVICE(child_ptr);
 -   qdev_set_parent_bus(dev, sysbus_get_default());
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.
This choice also matches when using sysbus_init_child_obj(),
since its code is:

  void sysbus_init_child_obj(Object *parent,
 const char *childname, void *child,
 size_t childsize, const char *childtype)
  {
  object_initialize_child(parent, childname, child, childsize,
  childtype, _abort, NULL);

  qdev_set_parent_bus(DEVICE(child), sysbus_get_default());
  }

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-7-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/arm/bcm2835_peripherals.c | 53 ++--
 1 file changed, 20 insertions(+), 33 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 2931a82a25..0fb54c7964 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -41,44 +41,36 @@ static void bcm2835_peripherals_init(Object *obj)
MBOX_CHAN_COUNT << MBOX_AS_CHAN_SHIFT);
 
 /* Interrupt Controller */
-object_initialize(>ic, sizeof(s->ic), TYPE_BCM2835_IC);
-object_property_add_child(obj, "ic", OBJECT(>ic), NULL);
-qdev_set_parent_bus(DEVICE(>ic), sysbus_get_default());
+sysbus_init_child_obj(obj, "ic", >ic, sizeof(s->ic), TYPE_BCM2835_IC);
 
 /* UART0 */
-object_initialize(>uart0, sizeof(s->uart0), TYPE_PL011);
-object_property_add_child(obj, "uart0", OBJECT(>uart0), NULL);
-qdev_set_parent_bus(DEVICE(>uart0), sysbus_get_default());
+sysbus_init_child_obj(obj, "uart0", >uart0, sizeof(s->uart0),
+  TYPE_PL011);
 
 /* AUX / UART1 */
-object_initialize(>aux, sizeof(s->aux), TYPE_BCM2835_AUX);
-object_property_add_child(obj, "aux", OBJECT(>aux), NULL);
-qdev_set_parent_bus(DEVICE(>aux), sysbus_get_default());
+sysbus_init_child_obj(obj, "aux", >aux, sizeof(s->aux),
+  TYPE_BCM2835_AUX);
 
 /* Mailboxes */
-object_initialize(>mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
-object_property_add_child(obj, "mbox", OBJECT(>mboxes), NULL);
-qdev_set_parent_bus(DEVICE(>mboxes), sysbus_get_default());
+sysbus_init_child_obj(obj, "mbox", >mboxes, sizeof(s->mboxes),
+   

[Qemu-devel] [PULL 13/17] hw/microblaze/zynqmp: Let the SoC manage the IPI devices

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

The Inter Processor Interrupt is a block part of the SoC, not the
"machine" (See Zynq UltraScale+ Device TRM UG1085, "Platform
Management Unit", Power Domains and Islands).

Move the IPI management from the machine to the SoC.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-13-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 36 +++--
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index eba9945c19..20e973edf5 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -68,6 +68,13 @@ static void xlnx_zynqmp_pmu_soc_init(Object *obj)
 
 sysbus_init_child_obj(obj, "intc", >intc, sizeof(s->intc),
   TYPE_XLNX_PMU_IO_INTC);
+
+/* Create the IPI device */
+for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
+object_initialize(>ipi[i], sizeof(XlnxZynqMPIPI),
+  TYPE_XLNX_ZYNQMP_IPI);
+qdev_set_parent_bus(DEVICE(>ipi[i]), sysbus_get_default());
+}
 }
 
 static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, Error **errp)
@@ -113,6 +120,15 @@ static void xlnx_zynqmp_pmu_soc_realize(DeviceState *dev, 
Error **errp)
 sysbus_mmio_map(SYS_BUS_DEVICE(>intc), 0, XLNX_ZYNQMP_PMU_INTC_ADDR);
 sysbus_connect_irq(SYS_BUS_DEVICE(>intc), 0,
qdev_get_gpio_in(DEVICE(>cpu), MB_CPU_IRQ));
+
+/* Connect the IPI device */
+for (int i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
+object_property_set_bool(OBJECT(>ipi[i]), true, "realized",
+ _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>ipi[i]), 0, ipi_addr[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>ipi[i]), 0,
+   qdev_get_gpio_in(DEVICE(>intc), ipi_irq[i]));
+}
 }
 
 static void xlnx_zynqmp_pmu_soc_class_init(ObjectClass *oc, void *data)
@@ -145,8 +161,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *pmu_rom = g_new(MemoryRegion, 1);
 MemoryRegion *pmu_ram = g_new(MemoryRegion, 1);
-qemu_irq irq[32];
-int i;
 
 /* Create the ROM */
 memory_region_init_rom(pmu_rom, NULL, "xlnx-zynqmp-pmu.rom",
@@ -166,24 +180,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
   _abort);
 object_property_set_bool(OBJECT(pmu), true, "realized", _fatal);
 
-for (i = 0; i < 32; i++) {
-irq[i] = qdev_get_gpio_in(DEVICE(>intc), i);
-}
-
-/* Create and connect the IPI device */
-for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-object_initialize(>ipi[i], sizeof(XlnxZynqMPIPI),
-  TYPE_XLNX_ZYNQMP_IPI);
-qdev_set_parent_bus(DEVICE(>ipi[i]), sysbus_get_default());
-}
-
-for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-object_property_set_bool(OBJECT(>ipi[i]), true, "realized",
- _abort);
-sysbus_mmio_map(SYS_BUS_DEVICE(>ipi[i]), 0, ipi_addr[i]);
-sysbus_connect_irq(SYS_BUS_DEVICE(>ipi[i]), 0, irq[ipi_irq[i]]);
-}
-
 /* Load the kernel */
 microblaze_load_kernel(>cpu, XLNX_ZYNQMP_PMU_RAM_ADDR,
machine->ram_size,
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 06/17] hw/arm/bcm2835: Use object_initialize() on PL011State

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

To be coherent with the other peripherals contained in the
BCM2835PeripheralState structure, directly allocate the PL011State
(instead of using the pl011 uart as a pointer to a SysBusDevice).

Initialize the PL011State with object_initialize() instead of
object_new().

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-6-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 include/hw/arm/bcm2835_peripherals.h |  2 +-
 hw/arm/bcm2835_peripherals.c | 14 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/bcm2835_peripherals.h 
b/include/hw/arm/bcm2835_peripherals.h
index 959508d57d..e79c21771f 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -38,7 +38,7 @@ typedef struct BCM2835PeripheralState {
 MemoryRegion ram_alias[4];
 qemu_irq irq, fiq;
 
-SysBusDevice *uart0;
+PL011State uart0;
 BCM2835AuxState aux;
 BCM2835FBState fb;
 BCM2835DMAState dma;
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 7ffb51b692..2931a82a25 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -46,9 +46,9 @@ static void bcm2835_peripherals_init(Object *obj)
 qdev_set_parent_bus(DEVICE(>ic), sysbus_get_default());
 
 /* UART0 */
-s->uart0 = SYS_BUS_DEVICE(object_new(TYPE_PL011));
-object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
-qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
+object_initialize(>uart0, sizeof(s->uart0), TYPE_PL011);
+object_property_add_child(obj, "uart0", OBJECT(>uart0), NULL);
+qdev_set_parent_bus(DEVICE(>uart0), sysbus_get_default());
 
 /* AUX / UART1 */
 object_initialize(>aux, sizeof(s->aux), TYPE_BCM2835_AUX);
@@ -166,16 +166,16 @@ static void bcm2835_peripherals_realize(DeviceState *dev, 
Error **errp)
 sysbus_pass_irq(SYS_BUS_DEVICE(s), SYS_BUS_DEVICE(>ic));
 
 /* UART0 */
-qdev_prop_set_chr(DEVICE(s->uart0), "chardev", serial_hd(0));
-object_property_set_bool(OBJECT(s->uart0), true, "realized", );
+qdev_prop_set_chr(DEVICE(>uart0), "chardev", serial_hd(0));
+object_property_set_bool(OBJECT(>uart0), true, "realized", );
 if (err) {
 error_propagate(errp, err);
 return;
 }
 
 memory_region_add_subregion(>peri_mr, UART0_OFFSET,
-sysbus_mmio_get_region(s->uart0, 0));
-sysbus_connect_irq(s->uart0, 0,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(>uart0), 0));
+sysbus_connect_irq(SYS_BUS_DEVICE(>uart0), 0,
 qdev_get_gpio_in_named(DEVICE(>ic), BCM2835_IC_GPU_IRQ,
INTERRUPT_UART));
 /* AUX / UART1 */
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 04/17] hw/virtio: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script:

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-4-phi...@redhat.com>
Signed-off-by: Eduardo Habkost 
---
 hw/virtio/virtio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4805727b53..07f4a64b48 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2312,9 +2312,8 @@ void virtio_instance_init_common(Object *proxy_obj, void 
*data,
 {
 DeviceState *vdev = data;
 
-object_initialize(vdev, vdev_size, vdev_name);
-object_property_add_child(proxy_obj, "virtio-backend", OBJECT(vdev), NULL);
-object_unref(OBJECT(vdev));
+object_initialize_child(proxy_obj, "virtio-backend", vdev, vdev_size,
+vdev_name, _abort, NULL);
 qdev_alias_all_properties(vdev, proxy_obj);
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 05/17] hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-5-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 include/hw/arm/bcm2835_peripherals.h | 1 +
 hw/arm/bcm2835_peripherals.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/bcm2835_peripherals.h 
b/include/hw/arm/bcm2835_peripherals.h
index f5b193f670..959508d57d 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -13,6 +13,7 @@
 
 #include "qemu-common.h"
 #include "hw/sysbus.h"
+#include "hw/char/pl011.h"
 #include "hw/char/bcm2835_aux.h"
 #include "hw/display/bcm2835_fb.h"
 #include "hw/dma/bcm2835_dma.h"
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 6be7660e8c..7ffb51b692 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -46,7 +46,7 @@ static void bcm2835_peripherals_init(Object *obj)
 qdev_set_parent_bus(DEVICE(>ic), sysbus_get_default());
 
 /* UART0 */
-s->uart0 = SYS_BUS_DEVICE(object_new("pl011"));
+s->uart0 = SYS_BUS_DEVICE(object_new(TYPE_PL011));
 object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
 qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 03/17] hw/misc/macio: Use object_initialize_child for correct ref. counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
 ?-  object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-3-phi...@redhat.com>
Acked-by: David Gibson 
Signed-off-by: Eduardo Habkost 
---
 hw/misc/macio/macio.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 94da85c8d7..b726c73022 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -346,12 +346,12 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 object_property_set_bool(OBJECT(>gpio), true, "realized", );
 
 /* PMU */
-object_initialize(>pmu, sizeof(s->pmu), TYPE_VIA_PMU);
+object_initialize_child(OBJECT(s), "pmu", >pmu, sizeof(s->pmu),
+TYPE_VIA_PMU, _abort, NULL);
 object_property_set_link(OBJECT(>pmu), OBJECT(sysbus_dev), "gpio",
  _abort);
 qdev_prop_set_bit(DEVICE(>pmu), "has-adb", ns->has_adb);
 qdev_set_parent_bus(DEVICE(>pmu), BUS(>macio_bus));
-object_property_add_child(OBJECT(s), "pmu", OBJECT(>pmu), NULL);
 
 object_property_set_bool(OBJECT(>pmu), true, "realized", );
 if (err) {
@@ -365,9 +365,9 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 sysbus_mmio_get_region(sysbus_dev, 0));
 } else {
 /* CUDA */
-object_initialize(>cuda, sizeof(s->cuda), TYPE_CUDA);
+object_initialize_child(OBJECT(s), "cuda", >cuda, sizeof(s->cuda),
+TYPE_CUDA, _abort, NULL);
 qdev_set_parent_bus(DEVICE(>cuda), BUS(>macio_bus));
-object_property_add_child(OBJECT(s), "cuda", OBJECT(>cuda), NULL);
 qdev_prop_set_uint64(DEVICE(>cuda), "timebase-frequency",
  s->frequency);
 
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 12/17] hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

The Inter Processor Interrupt is a block part of the SoC, not the
"machine" (talking about machine is borderline with the PMU, since
it is embedded into the ZynqMP SoC, but currentl QEMU doesn't
support multi-arch cores).

Move the IPI state to the SoC state, this will simplify the review
of the next patch.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-12-phi...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Alistair Francis 
Signed-off-by: Eduardo Habkost 
---
 hw/microblaze/xlnx-zynqmp-pmu.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/microblaze/xlnx-zynqmp-pmu.c b/hw/microblaze/xlnx-zynqmp-pmu.c
index 57dc1ccd42..eba9945c19 100644
--- a/hw/microblaze/xlnx-zynqmp-pmu.c
+++ b/hw/microblaze/xlnx-zynqmp-pmu.c
@@ -55,6 +55,7 @@ typedef struct XlnxZynqMPPMUSoCState {
 /*< public >*/
 MicroBlazeCPU cpu;
 XlnxPMUIOIntc intc;
+XlnxZynqMPIPI ipi[XLNX_ZYNQMP_PMU_NUM_IPIS];
 }  XlnxZynqMPPMUSoCState;
 
 
@@ -144,7 +145,6 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 MemoryRegion *address_space_mem = get_system_memory();
 MemoryRegion *pmu_rom = g_new(MemoryRegion, 1);
 MemoryRegion *pmu_ram = g_new(MemoryRegion, 1);
-XlnxZynqMPIPI *ipi[XLNX_ZYNQMP_PMU_NUM_IPIS];
 qemu_irq irq[32];
 int i;
 
@@ -172,16 +172,16 @@ static void xlnx_zynqmp_pmu_init(MachineState *machine)
 
 /* Create and connect the IPI device */
 for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-ipi[i] = g_new0(XlnxZynqMPIPI, 1);
-object_initialize(ipi[i], sizeof(XlnxZynqMPIPI), TYPE_XLNX_ZYNQMP_IPI);
-qdev_set_parent_bus(DEVICE(ipi[i]), sysbus_get_default());
+object_initialize(>ipi[i], sizeof(XlnxZynqMPIPI),
+  TYPE_XLNX_ZYNQMP_IPI);
+qdev_set_parent_bus(DEVICE(>ipi[i]), sysbus_get_default());
 }
 
 for (i = 0; i < XLNX_ZYNQMP_PMU_NUM_IPIS; i++) {
-object_property_set_bool(OBJECT(ipi[i]), true, "realized",
+object_property_set_bool(OBJECT(>ipi[i]), true, "realized",
  _abort);
-sysbus_mmio_map(SYS_BUS_DEVICE(ipi[i]), 0, ipi_addr[i]);
-sysbus_connect_irq(SYS_BUS_DEVICE(ipi[i]), 0, irq[ipi_irq[i]]);
+sysbus_mmio_map(SYS_BUS_DEVICE(>ipi[i]), 0, ipi_addr[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>ipi[i]), 0, irq[ipi_irq[i]]);
 }
 
 /* Load the kernel */
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 02/17] hw/ppc/pnv: Use object_initialize_child for correct reference counting

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

As explained in commit aff39be0ed97:

  Both functions, object_initialize() and object_property_add_child()
  increase the reference counter of the new object, so one of the
  references has to be dropped afterwards to get the reference
  counting right. Otherwise the child object will not be properly
  cleaned up when the parent gets destroyed.
  Thus let's use now object_initialize_child() instead to get the
  reference counting here right.

This patch was generated using the following Coccinelle script
(with a bit of manual fix-up for overly long lines):

 @use_object_initialize_child@
 expression parent_obj;
 expression child_ptr;
 expression child_name;
 expression child_type;
 expression child_size;
 expression errp;
 @@
 (
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +   child_type, _abort, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), NULL);
 ...
?-   object_unref(OBJECT(child_ptr));
 |
 -   object_initialize(child_ptr, child_size, child_type);
 +   object_initialize_child(parent_obj, child_name,  child_ptr, child_size,
 +child_type, errp, NULL);
 ... when != parent_obj
 -   object_property_add_child(parent_obj, child_name, OBJECT(child_ptr), errp);
 ...
?-   object_unref(OBJECT(child_ptr));
 )

While the object_initialize() function doesn't take an
'Error *errp' argument, the object_initialize_child() does.
Since this code is used when a machine is created (and is not
yet running), we deliberately choose to use the _abort
argument instead of ignoring errors if an object creation failed.

Suggested-by: Eduardo Habkost 
Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190507163416.24647-2-phi...@redhat.com>
Acked-by: David Gibson 
Signed-off-by: Eduardo Habkost 
---
 hw/ppc/pnv.c | 12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index dfb4ea5742..31aa20ee25 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -994,14 +994,12 @@ static void pnv_chip_quad_realize(Pnv9Chip *chip9, Error 
**errp)
 PnvCore *pnv_core = PNV_CORE(chip->cores + (i * 4) * typesize);
 int core_id = CPU_CORE(pnv_core)->core_id;
 
-object_initialize(eq, sizeof(*eq), TYPE_PNV_QUAD);
 snprintf(eq_name, sizeof(eq_name), "eq[%d]", core_id);
+object_initialize_child(OBJECT(chip), eq_name, eq, sizeof(*eq),
+TYPE_PNV_QUAD, _fatal, NULL);
 
-object_property_add_child(OBJECT(chip), eq_name, OBJECT(eq),
-  _fatal);
 object_property_set_int(OBJECT(eq), core_id, "id", _fatal);
 object_property_set_bool(OBJECT(eq), true, "realized", _fatal);
-object_unref(OBJECT(eq));
 
 pnv_xscom_add_subregion(chip, PNV9_XSCOM_EQ_BASE(eq->id),
 >xscom_regs);
@@ -1165,10 +1163,9 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
**errp)
 continue;
 }
 
-object_initialize(pnv_core, typesize, typename);
 snprintf(core_name, sizeof(core_name), "core[%d]", core_hwid);
-object_property_add_child(OBJECT(chip), core_name, OBJECT(pnv_core),
-  _fatal);
+object_initialize_child(OBJECT(chip), core_name, pnv_core, typesize,
+typename, _fatal, NULL);
 object_property_set_int(OBJECT(pnv_core), smp_threads, "nr-threads",
 _fatal);
 object_property_set_int(OBJECT(pnv_core), core_hwid,
@@ -1180,7 +1177,6 @@ static void pnv_chip_core_realize(PnvChip *chip, Error 
**errp)
OBJECT(chip), _fatal);
 object_property_set_bool(OBJECT(pnv_core), true, "realized",
  _fatal);
-object_unref(OBJECT(pnv_core));
 
 /* Each core has an XSCOM MMIO region */
 if (!pnv_chip_is_power9(chip)) {
-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 00/17] Machine Core queue, 2019-05-24

[Eduardo Habkost]

The following changes since commit a7b21f6762a2d6ec08106d8a7ccb11829914523f:

  Merge remote-tracking branch 
'remotes/vivier2/tags/linux-user-for-4.1-pull-request' into staging (2019-05-24 
12:47:49 +0100)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-next-pull-request

for you to fetch changes up to 23d1f360f3de1d968d98ba605bd3b718f5309e6f:

  hw/intc/nvic: Use object_initialize_child for correct reference counting 
(2019-05-24 15:29:02 -0300)


Machine Core queue, 2019-05-24

* Display more helpful message when an object type is missing
  (Philippe Mathieu-Daudé)
* Use object_initialize_child for correct reference counting
  (Philippe Mathieu-Daudé)



Queue for Machine Core patches


Philippe Mathieu-Daudé (17):
  qom/object: Display more helpful message when an object type is
missing
  hw/ppc/pnv: Use object_initialize_child for correct reference counting
  hw/misc/macio: Use object_initialize_child for correct ref. counting
  hw/virtio: Use object_initialize_child for correct reference counting
  hw/arm/bcm2835: Use TYPE_PL011 instead of hardcoded string
  hw/arm/bcm2835: Use object_initialize() on PL011State
  hw/arm/bcm2835: Use object_initialize_child for correct ref. counting
  hw/arm/aspeed: Use object_initialize_child for correct ref. counting
  hw/arm: Use object_initialize_child for correct reference counting
  hw/mips: Use object_initialize() on MIPSCPSState
  hw/mips: Use object_initialize_child for correct reference counting
  hw/microblaze/zynqmp: Move the IPI state into the PMUSoC state
  hw/microblaze/zynqmp: Let the SoC manage the IPI devices
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
counting
  hw/microblaze/zynqmp: Use object_initialize_child for correct ref.
counting
  hw/arm/mps2: Use object_initialize_child for correct reference
counting
  hw/intc/nvic: Use object_initialize_child for correct reference
counting

 include/hw/arm/bcm2835_peripherals.h |  3 +-
 hw/arm/aspeed.c  |  6 +--
 hw/arm/aspeed_soc.c  | 50 +--
 hw/arm/bcm2835_peripherals.c | 61 +++-
 hw/arm/digic.c   | 17 +++-
 hw/arm/imx25_pdk.c   |  5 +--
 hw/arm/kzm.c |  5 +--
 hw/arm/mps2-tz.c |  8 ++--
 hw/arm/mps2.c|  8 ++--
 hw/arm/raspi.c   |  7 ++--
 hw/arm/sabrelite.c   |  5 +--
 hw/arm/xlnx-zcu102.c |  5 +--
 hw/arm/xlnx-zynqmp.c |  8 ++--
 hw/intc/armv7m_nvic.c|  6 +--
 hw/microblaze/xlnx-zynqmp-pmu.c  | 45 ++--
 hw/mips/boston.c | 25 ++--
 hw/mips/cps.c| 20 -
 hw/mips/mips_malta.c | 17 
 hw/misc/macio/macio.c|  8 ++--
 hw/ppc/pnv.c | 12 ++
 hw/virtio/virtio.c   |  5 +--
 qom/object.c |  7 +++-
 22 files changed, 146 insertions(+), 187 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140

[Qemu-devel] [PULL 01/17] qom/object: Display more helpful message when an object type is missing

[Eduardo Habkost]

From: Philippe Mathieu-Daudé 

When writing a new board, adding device which uses other devices
(container) or simply refactoring, one can discover the hard way
his machine misses some devices. In the case of containers, the
error is not obvious:

  $ qemu-system-microblaze -M xlnx-zynqmp-pmu
  **
  ERROR:/source/qemu/qom/object.c:454:object_initialize_with_type: assertion 
failed: (type != NULL)
  Aborted (core dumped)

And we have to look at the coredump to figure the error:

  (gdb) bt
  #1  0x7f84773cf895 in abort () at /lib64/libc.so.6
  #2  0x7f847961fb53 in  () at /lib64/libglib-2.0.so.0
  #3  0x7f847967a4de in g_assertion_message_expr () at 
/lib64/libglib-2.0.so.0
  #4  0x55c4bcac6c11 in object_initialize_with_type 
(data=data@entry=0x55c4bdf239e0, size=size@entry=2464, type=) at 
/source/qemu/qom/object.c:454
  #5  0x55c4bcac6e6d in object_initialize (data=data@entry=0x55c4bdf239e0, 
size=size@entry=2464, typename=typename@entry=0x55c4bcc7c643 "xlnx.zynqmp_ipi") 
at /source/qemu/qom/object.c:474
  #6  0x55c4bc9ea474 in xlnx_zynqmp_pmu_init (machine=0x55c4bdd46000) at 
/source/qemu/hw/microblaze/xlnx-zynqmp-pmu.c:176
  #7  0x55c4bca3b6cb in machine_run_board_init (machine=0x55c4bdd46000) at 
/source/qemu/hw/core/machine.c:1030
  #8  0x55c4bc95f6d2 in main (argc=, argv=, 
envp=) at /source/qemu/vl.c:4479

Since the caller knows the type name requested, we can simply display it
to ease development.

With this patch applied we get:

  $ qemu-system-microblaze -M xlnx-zynqmp-pmu
  qemu-system-microblaze: missing object type 'xlnx.zynqmp_ipi'
  Aborted (core dumped)

Since the assert(type) check in object_initialize_with_type() is
now impossible, remove it.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20190427135642.16464-1-phi...@redhat.com>
Reviewed-by: Cornelia Huck 
Reviewed-by: Stefano Garzarella 
Reviewed-by: Daniel P. Berrangé 
Signed-off-by: Eduardo Habkost 
---
 qom/object.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 99c4fa707e..3966a3d461 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -28,6 +28,7 @@
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qnum.h"
 #include "qapi/qmp/qstring.h"
+#include "qemu/error-report.h"
 
 #define MAX_INTERFACES 32
 
@@ -448,7 +449,6 @@ static void object_initialize_with_type(void *data, size_t 
size, TypeImpl *type)
 {
 Object *obj = data;
 
-g_assert(type != NULL);
 type_initialize(type);
 
 g_assert(type->instance_size >= sizeof(Object));
@@ -468,6 +468,11 @@ void object_initialize(void *data, size_t size, const char 
*typename)
 {
 TypeImpl *type = type_get_by_name(typename);
 
+if (!type) {
+error_report("missing object type '%s'", typename);
+abort();
+}
+
 object_initialize_with_type(data, size, type);
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140

Re: [Qemu-devel] [PATCH v2 4/4] BootLinuxSshTest: Test some userspace commands on Malta

[Eduardo Habkost]

On Thu, May 23, 2019 at 06:18:32PM +0200, Philippe Mathieu-Daudé wrote:
> This tests boot a full VM and check the serial console until
> the SSH daemon is running, then start a SSH session and run
> some commands.
> 
> This test can be run using:
> 
>   $ avocado --show=ssh run -t arch:mips 
> tests/acceptance/linux_ssh_mips_malta.py
>   ssh: Entering interactive session.
>   ssh: # uname -a
>   ssh: Linux debian-mips 3.2.0-4-4kc-malta #1 Debian 3.2.51-1 mips GNU/Linux
>   ssh: # lspci -d 11ab:4620
>   ssh: 00:00.0 Host bridge: Marvell Technology Group Ltd. 
> GT-64120/64120A/64121A System Controller (rev 10)
>   ssh: # cat /sys/bus/i2c/devices/i2c-0/name
>   ssh: SMBus PIIX4 adapter at 1100
>   ssh: # cat /proc/mtd
>   ssh: dev:size   erasesize  name
>   ssh: mtd0: 0010 0001 "YAMON"
>   ssh: mtd1: 002e 0001 "User FS"
>   ssh: mtd2: 0002 0001 "Board Config"
>   ssh: # md5sum /dev/mtd2ro
>   ssh: 0dfbe8aa4c20b52e1b8bf3cb6cbdf193  /dev/mtd2ro
>   ssh: # poweroff
> 
> Acked-by: Aleksandar Markovic 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> v2: Add skipIf(getenv(CONTINUOUS_INTEGRATION))

This will make the behavior of "make check-acceptance" different
inside and outside Travis, in a way that is not documented
anywhere.  I thought we would be using Avocado tags to indicate
which tests we want to skip.

But if it solves our immediate problems we're having, I won't
NACK this.  Can we at least add TODO comments indicating we
should eventually get rid of the hack?

Reviewed-by: Eduardo Habkost 

> ---
>  MAINTAINERS  |   1 +
>  tests/acceptance/linux_ssh_mips_malta.py | 230 +++
>  tests/requirements.txt   |   1 +
>  3 files changed, 232 insertions(+)
>  create mode 100644 tests/acceptance/linux_ssh_mips_malta.py
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3cacd751bf..8c34d5c34b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -934,6 +934,7 @@ M: Aurelien Jarno 
>  R: Aleksandar Rikalo 
>  S: Maintained
>  F: hw/mips/mips_malta.c
> +F: tests/acceptance/linux_ssh_mips_malta.py
>  
>  Mipssim
>  M: Aleksandar Markovic 
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
> b/tests/acceptance/linux_ssh_mips_malta.py
> new file mode 100644
> index 00..aafb0c39f6
> --- /dev/null
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -0,0 +1,230 @@
> +# Functional test that boots a VM and run commands via a SSH session
> +#
> +# Copyright (c) Philippe Mathieu-Daudé 
> +#
> +# 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 re
> +import base64
> +import logging
> +import paramiko
> +import time
> +
> +from avocado import skipIf
> +from avocado_qemu import Test
> +from avocado.utils import process
> +from avocado.utils import archive
> +
> +
> +class LinuxSSH(Test):
> +
> +timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
> +
> +KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> +VM_IP = '127.0.0.1'
> +
> +IMAGE_INFO = {
> +'be': {
> +'image_url': 'https://people.debian.org/~aurel32/qemu/mips/'
> + 'debian_wheezy_mips_standard.qcow2',
> +'image_hash': '8987a63270df67345b2135a6b7a4885a35e392d5',
> +'rsa_hostkey': b'B3NzaC1yc2EDAQABAAABAQCca1VitiyLAdQOld'
> +   b'zT43IOEVJZ0wHD78GJi8wDAjMiYWUzNSSn0rXGQsINHuH5'
> +   b'IlF+kBZsHinb/FtKCAyS9a8uCHhQI4SuB4QhAb0+39MlUw'
> +   b'Mm0CLkctgM2eUUZ6MQMQvDlqnue6CCkxN62EZYbaxmby7j'
> +   b'CQa1125o1HRKBvdGm2zrJWxXAfA+f1v6jHLyE8Jnu83eQ+'
> +   b'BFY25G+Vzx1PVc3zQBwJ8r0NGTRqy2//oWQP0h+bMsgeFe'
> +   b'KH/J3RJM22vg6+I4JAdBFcxnK+l781h1FuRxOn4O/Xslbg'
> +   b'go6WtB4V4TOsw2E/KfxI5IZ/icxF+swVcnvF46Hf3uQc/0'
> +   b'BBqb',
> +},
> +'le': {
> +'image_url': 'https://people.debian.org/~aurel32/qemu/mipsel/'
> + 'debian_wheezy_mipsel_standard.qcow2',
> +'image_hash': '7866764d9de3ef536ffca24c9fb9f04ffdb45802',
> +'rsa_hostkey': b'B3NzaC1yc2EDAQABAAABAQClXJlBT71HL5yKvv'
> +   b'gfC7jmxSWx5zSBCzET6CLZczwAafSIs7YKfNOy/dQTxhuk'
> +   b'yIGFUugZFoF3E9PzdhunuyvyTd56MPoNIqFbb5rGokwU5I'
> +   b'TOx3dBHZR0mClypL6MVrwe0bsiIb8GhF1zioNwcsaAZnAi'
> +   b'KfXStVDtXvn/kLLq+xLABYt48CC5KYWoFaCoICskLAY+qo'
> +   b'L+LWyAnQisj4jAH8VSaSKIImFpfkHWEXPhHcC4ZBlDKtnH'
> +   b'po9vhfCHgnfW3Pzrqmk8BI4HysqPFVmJWkJGlGUL+sGeg3'
> +   b'ZZolAYuDXGuBrw8ooPJq2v2dOH+z6dyD2q/ypmAbyPqj5C'
> +   b'rc8H',
> +},
> +}
> +
> +def 

[Qemu-devel] [RFC v3 3/3] virtio-scsi: fix iothread deadlock on 'cont'

[Stefan Hajnoczi]

When the 'cont' command resumes guest execution the vm change state
handlers are invoked.  Unfortunately there is no explicit ordering
between vm change state handlers.  When two layers of code both use vm
change state handlers, we don't control which handler runs first.

virtio-scsi with iothreads hits a deadlock when a failed SCSI command is
restarted and completes before the iothread is re-initialized.

This patch makes sure that DMA restart happens after the iothread has
been started again.

Signed-off-by: Stefan Hajnoczi 
---
 hw/scsi/virtio-scsi.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 839f120256..236a0ee873 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -846,12 +846,28 @@ static void virtio_scsi_hotunplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 qdev_simple_device_unplug_cb(hotplug_dev, dev, errp);
 }
 
+static void virtio_scsi_vmstate_change(VirtIODevice *vdev, bool running)
+{
+VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+
+if (running) {
+scsi_bus_dma_restart(>bus);
+}
+}
+
 static struct SCSIBusInfo virtio_scsi_scsi_info = {
 .tcq = true,
 .max_channel = VIRTIO_SCSI_MAX_CHANNEL,
 .max_target = VIRTIO_SCSI_MAX_TARGET,
 .max_lun = VIRTIO_SCSI_MAX_LUN,
 
+/* We call scsi_bus_dma_restart() ourselves to control the ordering between
+ * ->start_ioeventfd() and DMA restart.  Do it in
+ * virtio_scsi_vmstate_change(), which is called by the core virtio code
+ * after ->start_ioeventfd().
+ */
+.custom_dma_restart = true,
+
 .complete = virtio_scsi_command_complete,
 .cancel = virtio_scsi_request_cancelled,
 .change = virtio_scsi_change,
@@ -986,6 +1002,7 @@ static void virtio_scsi_class_init(ObjectClass *klass, 
void *data)
 vdc->reset = virtio_scsi_reset;
 vdc->start_ioeventfd = virtio_scsi_dataplane_start;
 vdc->stop_ioeventfd = virtio_scsi_dataplane_stop;
+vdc->vmstate_change = virtio_scsi_vmstate_change;
 hc->plug = virtio_scsi_hotplug;
 hc->unplug = virtio_scsi_hotunplug;
 }
-- 
2.21.0

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[David Hildenbrand]

On 24.05.19 20:28, Christian Borntraeger wrote:
> 
> 
> On 24.05.19 20:04, David Hildenbrand wrote:
>> On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
>>> Hi Christian,
>>>
>>> I'm having hard time to understand why the S390_IPL object calls
>>> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
>>> being QOM'ified (it has a reset method).
>>>
>>> It doesn't seem to have a qdev children added explicitly to it.
>>> I see it is used as a singleton, what else am I missing?
>>>
>>> Thanks,
>>>
>>> Phil.
>>>
>>
>> Looks like I added it back then (~4 years ago) when converting it into a
>> TYPE_DEVICE.
>>
>> I could imagine that - back then - this was needed because only
>> TYPE_SYS_BUS_DEVICE would recursively get reset.
> 
> Yes, back then singleton devices were not recursively resetted. Has that 
> changed?

Hacking that call out, I don't see it getting called anymore. So it is
still required. The question is if it can be reworked.

-- 

Thanks,

David / dhildenb

[Qemu-devel] [RFC v3 2/3] scsi: add scsi_bus_dma_restart()

[Stefan Hajnoczi]

By default scsi-bus.c registers vm change state handlers for SCSIDevice
instances and restarts DMA when guest execution is resumed.

Unfortunately virtio-scsi with iothreads has a special ordering
requirement that the core virtio code's vm change state handler runs
before scsi-bus.c's vm change state handler.

This patch allows SCSI busses to disable the default vm change state
handler for DMA restart.  The new scsi_bus_dma_restart() API allows them
to manually restart DMA at a time of their choice.

Signed-off-by: Stefan Hajnoczi 
---
 include/hw/scsi/scsi.h |  5 +
 hw/scsi/scsi-bus.c | 37 ++---
 2 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index acef25faa4..e9cc563daa 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -120,6 +120,10 @@ struct SCSIReqOps {
 struct SCSIBusInfo {
 int tcq;
 int max_channel, max_target, max_lun;
+
+/* Will the bus call scsi_bus_dma_restart() itself? */
+bool custom_dma_restart;
+
 int (*parse_cdb)(SCSIDevice *dev, SCSICommand *cmd, uint8_t *buf,
  void *hba_private);
 void (*transfer_data)(SCSIRequest *req, uint32_t arg);
@@ -160,6 +164,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
BlockBackend *blk,
   const char *serial, Error **errp);
 void scsi_bus_legacy_handle_cmdline(SCSIBus *bus);
 void scsi_legacy_handle_cmdline(void);
+void scsi_bus_dma_restart(SCSIBus *bus);
 
 SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d,
 uint32_t tag, uint32_t lun, void *hba_private);
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index c480553083..d2c5a1652b 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -134,6 +134,27 @@ void scsi_req_retry(SCSIRequest *req)
 req->retry = true;
 }
 
+static void scsi_device_dma_restart(SCSIDevice *dev)
+{
+if (!dev->bh) {
+AioContext *ctx = blk_get_aio_context(dev->conf.blk);
+dev->bh = aio_bh_new(ctx, scsi_dma_restart_bh, dev);
+qemu_bh_schedule(dev->bh);
+}
+}
+
+void scsi_bus_dma_restart(SCSIBus *bus)
+{
+BusChild *kid;
+
+QTAILQ_FOREACH(kid, >qbus.children, sibling) {
+DeviceState *qdev = kid->child;
+SCSIDevice *dev = SCSI_DEVICE(qdev);
+
+scsi_device_dma_restart(dev);
+}
+}
+
 static void scsi_dma_restart_cb(void *opaque, int running, RunState state)
 {
 SCSIDevice *s = opaque;
@@ -141,11 +162,8 @@ static void scsi_dma_restart_cb(void *opaque, int running, 
RunState state)
 if (!running) {
 return;
 }
-if (!s->bh) {
-AioContext *ctx = blk_get_aio_context(s->conf.blk);
-s->bh = aio_bh_new(ctx, scsi_dma_restart_bh, s);
-qemu_bh_schedule(s->bh);
-}
+
+scsi_device_dma_restart(s);
 }
 
 static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
@@ -206,8 +224,13 @@ static void scsi_qdev_realize(DeviceState *qdev, Error 
**errp)
 error_propagate(errp, local_err);
 return;
 }
-dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
- dev);
+
+if (bus->info->custom_dma_restart) {
+dev->vmsentry = NULL;
+} else {
+dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
+ dev);
+}
 }
 
 static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp)
-- 
2.21.0

[Qemu-devel] [RFC v3 1/3] virtio: add vdc->vmchange_state() callback

[Stefan Hajnoczi]

The core virtio code invokes ->set_status() followed by
->ioeventfd_start() when the guest resumes execution.  Both of these
functions are also called in other cases unrelated to vm change state.

This patch introduces ->vmstate_change() so that devices can act on
guest pause/resume.  The existing qemu_add_vm_change_state_handler() API
isn't usable by virtio devices since the ordering between vm change
state handlers is undefined.  The new ->vmstate_change() callback is
always invoked after ->set_status() and ->ioeventfd_start() when
resuming a guest.

A later patch makes use of this new callback.

Signed-off-by: Stefan Hajnoczi 
---
 include/hw/virtio/virtio.h | 7 +++
 hw/virtio/virtio.c | 9 +
 2 files changed, 16 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27c0efc3d0..5742efa1d7 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -158,6 +158,13 @@ typedef struct VirtioDeviceClass {
 void (*save)(VirtIODevice *vdev, QEMUFile *f);
 int (*load)(VirtIODevice *vdev, QEMUFile *f, int version_id);
 const VMStateDescription *vmsd;
+
+/* Called when the device should start/stop running because the guest was
+ * resumed/paused.  Note that this takes VIRTIO_CONFIG_S_DRIVER_OK into
+ * account so running is true iff the guest is resumed and the guest driver
+ * has already indicated it is ready.
+ */
+void (*vmstate_change)(VirtIODevice *vdev, bool running);
 } VirtioDeviceClass;
 
 void virtio_instance_init_common(Object *proxy_obj, void *data,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4805727b53..cdf869456b 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2291,6 +2291,7 @@ static void virtio_vmstate_change(void *opaque, int 
running, RunState state)
 VirtIODevice *vdev = opaque;
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 bool backend_run = running && vdev->started;
 vdev->vm_running = running;
 
@@ -2298,10 +2299,18 @@ static void virtio_vmstate_change(void *opaque, int 
running, RunState state)
 virtio_set_status(vdev, vdev->status);
 }
 
+if (!backend_run && vdc->vmstate_change) {
+vdc->vmstate_change(vdev, backend_run);
+}
+
 if (k->vmstate_change) {
 k->vmstate_change(qbus->parent, backend_run);
 }
 
+if (backend_run && vdc->vmstate_change) {
+vdc->vmstate_change(vdev, backend_run);
+}
+
 if (!backend_run) {
 virtio_set_status(vdev, vdev->status);
 }
-- 
2.21.0

[Qemu-devel] [RFC v3 0/3] scsi: restart dma after vm change state handlers

[Stefan Hajnoczi]

v3:
 * Fix s/k->vmstate_change/vdc->vmstate_change/
 * Still RFC, waiting for customer to confirm this fixes the issue
v2:
 * Do it properly with a clean API instead of deferring to a BH!
   Thanks for encouraging me to do this, Kevin.

These patches solve a deadlock when the 'cont' command is used and there are
failed requests on a virtio-scsi device with iothreads.  The deadlock itself is
actually not the thing we need to fix because we should never reach that case
anyway.  Instead we need to make sure DMA restart is only performed after the
virtio-scsi iothread is re-initialized.

Stefan Hajnoczi (3):
  virtio: add vdc->vmchange_state() callback
  scsi: add scsi_bus_dma_restart()
  virtio-scsi: fix iothread deadlock on 'cont'

 include/hw/scsi/scsi.h |  5 +
 include/hw/virtio/virtio.h |  7 +++
 hw/scsi/scsi-bus.c | 37 ++---
 hw/scsi/virtio-scsi.c  | 17 +
 hw/virtio/virtio.c |  9 +
 5 files changed, 68 insertions(+), 7 deletions(-)

-- 
2.21.0

Re: [Qemu-devel] Introducing GSoC project: API Documentation Generation

[Paolo Bonzini]

On 23/05/19 14:20, John Snow wrote:
> OK, if that's where we're at! I just saw the RFC from Peter Maydell and
> assumed we were a little further along the decision making process, but
> maybe not. I'll stay tuned.

For the decision making, yes; I think there's consensus to use
kerneldoc.  For the "debugging and seeing if anything has changed in 2.5
years", no.

Testing the patch that Eduardo posted will help Gabriel, Eduardo and
everyone else decide whether to patch kerneldoc or rather change the API
doc comments style.  (Personally I am in favor of patching; the
different coding conventions make using vanilla kerneldoc awkward, and
there are several thousands of lines of existing doc comments which
would require a transition.)

Paolo

Re: [Qemu-devel] qapi/misc.json is too big, let's bite off a few chunks

[Paolo Bonzini]

On 23/05/19 18:14, Markus Armbruster wrote:
> * Machine core (Eduardo, Marcel)
> 
>   query-machines, query-current-machine, 
> 
>   ~60 lines.  Hardly worthwhile from a "let's shrink misc.json" point of
>   view.  Might be worthwhile from a "let's make get_maintainers.pl
>   work".
> 
> * CPUs (Paolo, Richard)
> 
>   query-cpus, query-cpus-fast
> 
>   ~300 lines.  The commands are implemented in cpus.c, which MAINTAINERS
>   covers both under "Main loop" and under "Guest CPU cores (TCG) /
>   Overall".  Neither feels right to me for these QMP commands.
> 
> * NUMA (Eduardo)
> 
>   query-memdev, set-numa-node
> 
>   ~200 lines.

I would move all three of these and add a new entry to MAINTAINERS.

Paoo

[Qemu-devel] [PATCH] configure: remove tpm_passthrough & tpm_emulator

[Marc-André Lureau]

This is a left-over from commit 7aaa6a16373 "tpm: express dependencies
with Kconfig".

Signed-off-by: Marc-André Lureau 
---
 configure | 10 --
 1 file changed, 10 deletions(-)

diff --git a/configure b/configure
index 528b9ff705..4da99ee750 100755
--- a/configure
+++ b/configure
@@ -6434,8 +6434,6 @@ echo "gcov  $gcov_tool"
 echo "gcov enabled  $gcov"
 echo "TPM support   $tpm"
 echo "libssh2 support   $libssh2"
-echo "TPM passthrough   $tpm_passthrough"
-echo "TPM emulator  $tpm_emulator"
 echo "QOM debugging $qom_cast_debug"
 echo "Live block migration $live_block_migration"
 echo "lzo support   $lzo"
@@ -7133,14 +7131,6 @@ fi
 
 if test "$tpm" = "yes"; then
   echo 'CONFIG_TPM=$(CONFIG_SOFTMMU)' >> $config_host_mak
-  # TPM passthrough support?
-  if test "$tpm_passthrough" = "yes"; then
-echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
-  fi
-  # TPM emulator support?
-  if test "$tpm_emulator" = "yes"; then
-echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak
-  fi
 fi
 
 echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak
-- 
2.22.0.rc1.1.g079e7d2849.dirty

Re: [Qemu-devel] [RFC v2 1/3] virtio: add vdc->vmchange_state() callback

[Stefan Hajnoczi]

On Thu, May 23, 2019 at 2:55 PM Stefan Hajnoczi  wrote:
> @@ -2253,10 +2254,18 @@ static void virtio_vmstate_change(void *opaque, int 
> running, RunState state)
>  virtio_set_status(vdev, vdev->status);
>  }
>
> +if (!backend_run && k->vmstate_change) {
> +vdc->vmstate_change(vdev, backend_run);
> +}
> +
>  if (k->vmstate_change) {
>  k->vmstate_change(qbus->parent, backend_run);
>  }
>
> +if (backend_run && k->vmstate_change) {
> +vdc->vmstate_change(vdev, backend_run);
> +}

Oops, k->vmstate_change should be vdc->vmstate_change.  I'll send a
new revision.

Stefan

Re: [Qemu-devel] How do we do user input bitmap properties?

[Eduardo Habkost]

On Thu, May 23, 2019 at 10:35:24AM +0200, Andrea Bolognani wrote:
> On Wed, 2019-05-15 at 13:54 +0200, Andrew Jones wrote:
> > On Wed, May 15, 2019 at 12:52:29PM +0200, Igor Mammedov wrote:
> > > since using magic numbers is not very descriptive
> > > (but if there is some spec where they come from that we could point users 
> > > to
> > > it might be acceptable too, but I'd reserve number approach for values 
> > > only).
> > 
> > The numbers aren't magic, they're part of the name. '1' in the above
> > 'sve1' means one quadword. It would probably have been better to use bits
> > instead in the example, i.e.
> > 
> >   -cpu host,sve128=on,sve256=on,sve384=off,sve512=on
> > 
> > where it's now clear that "sve512" has an analogy with x86's "avx512".
> > 
> [...]
> > 
> > So I set off to convince Igor of the wide word idea (he sits next to me,
> > so I didn't have go far), but he has convinced me of the above property
> > idea. He used the magic phrase: "less code would be needed". If we use
> > the properties like above then we get introspection for free (cpu property
> > listing which libvirt already knows how to do) - so no QMP query needed.
> > The cost is adding several properties (16 to handle the current 2048-bit
> > limit), but I guess that's cheap enough. The command line is verbose, but
> > also much easier for a human to construct and read. I'm pretty sold on
> > this path, but adding Andrea and Eduardo for their input as well.
> 
> Sorry for taking a while to respond. Anyway, the above looks good to
> me as a general direction, but note that you'll have to implement at
> the very least the query-cpu-model-expansion QMP command for the
> introspection to work.

Why is query-cpu-model-expansion needed?  Isn't
device-list-properties enough?

> 
> query-cpu-model-baseline and query-cpu-model-comparison are two more
> QMP command which, while perhaps not immediately applicabile, we will
> want to implement at some point; more in general, what s390x is doing
> with respect to CPU models is a good blueprint, according to the
> libvirt developer who's the most involved with that specific area of
> the project.

Agreed.  Even if not necessary right now, query-cpu-model-* will
probably be needed eventually.

-- 
Eduardo

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[Philippe Mathieu-Daudé]

On 5/24/19 8:04 PM, David Hildenbrand wrote:
> On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
>> Hi Christian,
>>
>> I'm having hard time to understand why the S390_IPL object calls
>> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
>> being QOM'ified (it has a reset method).
>>
>> It doesn't seem to have a qdev children added explicitly to it.
>> I see it is used as a singleton, what else am I missing?
>>
>> Thanks,
>>
>> Phil.
>>
> 
> Looks like I added it back then (~4 years ago) when converting it into a
> TYPE_DEVICE.
> 
> I could imagine that - back then - this was needed because only
> TYPE_SYS_BUS_DEVICE would recursively get reset.

Thanks for the quick response :)

> Did you try removing it, to see if anything breaks?

>From build POV it is OK, but I have now idea of the effects with KVM.

I don't know how to test on s390x systems, but luckily Patchew/s390x is
back up so I'll try my luck with a RFC patch, but I'm not sure the
default tests cover this specific device uses.

Regards,

Phil.

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[Christian Borntraeger]

On 24.05.19 20:04, David Hildenbrand wrote:
> On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
>> Hi Christian,
>>
>> I'm having hard time to understand why the S390_IPL object calls
>> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
>> being QOM'ified (it has a reset method).
>>
>> It doesn't seem to have a qdev children added explicitly to it.
>> I see it is used as a singleton, what else am I missing?
>>
>> Thanks,
>>
>> Phil.
>>
> 
> Looks like I added it back then (~4 years ago) when converting it into a
> TYPE_DEVICE.
> 
> I could imagine that - back then - this was needed because only
> TYPE_SYS_BUS_DEVICE would recursively get reset.

Yes, back then singleton devices were not recursively resetted. Has that 
changed?
> 
> Did you try removing it, to see if anything breaks?
> 

Re: [Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[David Hildenbrand]

On 24.05.19 19:54, Philippe Mathieu-Daudé wrote:
> Hi Christian,
> 
> I'm having hard time to understand why the S390_IPL object calls
> qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
> being QOM'ified (it has a reset method).
> 
> It doesn't seem to have a qdev children added explicitly to it.
> I see it is used as a singleton, what else am I missing?
> 
> Thanks,
> 
> Phil.
> 

Looks like I added it back then (~4 years ago) when converting it into a
TYPE_DEVICE.

I could imagine that - back then - this was needed because only
TYPE_SYS_BUS_DEVICE would recursively get reset.

Did you try removing it, to see if anything breaks?

-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH] event_match: always match on None value

[John Snow]

Before, event_match didn't always recurse if the event value was not a
dictionary, and would instead check for equality immediately.

By delaying equality checking to post-recursion, we can allow leaf
values like "5" to match "None" and take advantage of the generic
None-returns-True clause.

This makes the matching a little more obviously consistent at the
expense of being able to check for explicit None values, which is
probably not that important given what this function is used for.

Signed-off-by: John Snow 
---
 python/qemu/__init__.py | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
index 98ed8a2e28..77d45f88fe 100644
--- a/python/qemu/__init__.py
+++ b/python/qemu/__init__.py
@@ -409,27 +409,30 @@ class QEMUMachine(object):
 
 The match criteria takes the form of a matching subdict. The event is
 checked to be a superset of the subdict, recursively, with matching
-values whenever those values are not None.
+values whenever the subdict values are not None.
+
+This has a limitation that you cannot explicitly check for None values.
 
 Examples, with the subdict queries on the left:
  - None matches any object.
  - {"foo": None} matches {"foo": {"bar": 1}}
- - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
- - {"foo": {"baz": 2}} matches {"foo": {"bar": 1, "baz": 2}}
+ - {"foo": None} matches {"foo": 5}
+ - {"foo": {"abc": None}} does not match {"foo": {"bar": 1}}
+ - {"foo": {"rab": 2}} matches {"foo": {"bar": 1, "rab": 2}}
 """
 if match is None:
 return True
 
-for key in match:
-if key in event:
-if isinstance(event[key], dict):
-if not QEMUMachine.event_match(event[key], match[key]):
-return False
-elif event[key] != match[key]:
+try:
+for key in match:
+if key in event:
+return QEMUMachine.event_match(event[key], match[key])
+else:
 return False
-else:
-return False
-return True
+return True
+except TypeError:
+# either match or event wasn't iterable (not a dict)
+return match == event
 
 def event_wait(self, name, timeout=60.0, match=None):
 """
-- 
2.20.1

Re: [Qemu-devel] [RFC 0/3] block: Inquire images’ rotational info

[Eric Blake]

On 5/24/19 12:28 PM, Max Reitz wrote:
> Hi,
> 
> http://lists.nongnu.org/archive/html/qemu-block/2019-05/msg00569.html
> shows that optimizations affect HDDs and SSDs differently.  It would be
> nice if we could differentiate between the two and then choose to adjust
> our behavior depending on whether a given image is stored on an HDD or
> not.
> 
> Or maybe it isn’t so nice.  That’s one reason this is an RFC.

Not an entirely bad idea. The NBD spec advertises whether an image is
rotational, precisely because it CAN make a difference for optimizing
access patterns. Likewise, commit 3b19f450 added support for ide
emulation to report rotation or non-rotational through to the guest.  So
plumbing it up in more places makes sense.

> 
> The other is that I implemented recognition of the rotational status by
> querying sysfs.  That looks stupid, but I didn’t find a better way
> (there is a BLKROTATIONAL ioctl, but that only works on device files).
> 
> But, hey, if you look through block/file-posix.c, you’ll find that I’m
> not the first to query sysfs.  hdev_get_max_segments() does so, too.  So
> maybe it isn’t that bad of an idea.
> 
> 
> What do you think?  Is the whole idea stupid?  Just the implementation?
> Or does it make sense?
> 

I'm in favor of the idea, whether or not review of the patches points
out tweaks to make before dropping RFC.

> 
> Max Reitz (3):
>   block: Add ImageRotationalInfo
>   file-posix: Inquire rotational status
>   qcow2: Evaluate rotational info
> 
>  qapi/block-core.json  | 19 ++-
>  block/qcow2.h |  3 ++
>  include/block/block.h |  7 +
>  block.c   | 20 +++-
>  block/file-posix.c| 73 +++
>  block/qapi.c  |  3 ++
>  block/qcow2.c | 34 +---
>  7 files changed, 153 insertions(+), 6 deletions(-)

Given my comments about NBD, I'd expect a patch to block/nbd{,-client}.c
to expose this as well.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 3/5] QEMUMachine: add events_wait method

[John Snow]

On 5/24/19 8:38 AM, Max Reitz wrote:
> On 23.05.19 20:03, John Snow wrote:
>>
>>
>> On 5/23/19 1:49 PM, Max Reitz wrote:
>>> On 23.05.19 19:06, John Snow wrote:
 Instead of event_wait which looks for a single event, add an events_wait
 which can look for any number of events simultaneously. However, it
 will still only return one at a time, whichever happens first.

 Signed-off-by: John Snow 
 ---
  python/qemu/__init__.py | 69 +
  1 file changed, 49 insertions(+), 20 deletions(-)

 diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
 index 81d9657ec0..98ed8a2e28 100644
 --- a/python/qemu/__init__.py
 +++ b/python/qemu/__init__.py
 @@ -402,42 +402,71 @@ class QEMUMachine(object):
  self._qmp.clear_events()
  return events
  
 -def event_wait(self, name, timeout=60.0, match=None):
 +@staticmethod
 +def event_match(event, match=None):
  """
 -Wait for specified timeout on named event in QMP; optionally 
 filter
 -results by match.
 +Check if an event matches optional match criteria.
  
 -The 'match' is checked to be a recursive subset of the 'event'; 
 skips
 -branch processing on match's value None
 -   {"foo": {"bar": 1}} matches {"foo": None}
 -   {"foo": {"bar": 1}} does not matches {"foo": {"baz": None}}
 +The match criteria takes the form of a matching subdict. The 
 event is
 +checked to be a superset of the subdict, recursively, with 
 matching
 +values whenever those values are not None.
 +
 +Examples, with the subdict queries on the left:
 + - None matches any object.
 + - {"foo": None} matches {"foo": {"bar": 1}}
 + - {"foo": {"baz": None}} does not match {"foo": {"bar": 1}}
>>>
>>> Pre-existing, but the difference between “bar” and “baz” confused me
>>> quite a bit.
>>>
>>> Also, I wonder...  {"foo": None} would not match {"foo": 1}, right?
>>> Does that make sense?  Shouldn’t None be the wildcard here in general?
>>> (Also pre-existing of course.)
>>>
>>> But this patch doesn’t make things worse, so:
>>>
>>> Reviewed-by: Max Reitz 
>>>
>>> (I’d still like your opinion.)
>>>
>>
>> I knew I was inviting trouble by trying to re-document this.
>>
>> The intention I had when writing the docs, which I think are wrong now,
>> was for {"foo": None} to match {"foo": 1}, but I think you're right that
>> it won't because '1' isn't a dict, so it tests for equality instead.
>>
>> So I need to fix this one up a little bit, but I'll take the review as a
>> sign that this approach seems workable.
> 
> I think the comment is technically completely correct.  It’s just that
> (1) it’s hard to discern “bar” from “baz”, and (2) if {"foo": None}
> intentionally does not match {"foo": 1}, we may want to document that,
> because it isn’t intuitively clear from the description.  If it’s a bug,
> the code should be fixed (and it should still be documented).
> 
> Max
> 

Yeah, I see. OK; I have prepared a patch that can be applied
independently after this series, if you'd like to stage this as-is. The
new patch changes behavior of this function a little, but is backwards
compatible with no changes because nobody was using these edge cases.

--js

[Qemu-devel] hw/s390x/ipl: Dubious use of qdev_reset_all_fn

[Philippe Mathieu-Daudé]

Hi Christian,

I'm having hard time to understand why the S390_IPL object calls
qemu_register_reset(qdev_reset_all_fn) in its realize() method, while
being QOM'ified (it has a reset method).

It doesn't seem to have a qdev children added explicitly to it.
I see it is used as a singleton, what else am I missing?

Thanks,

Phil.

[Qemu-devel] [Bug 1830415] [NEW] linux-user elf loader issue

[antonio barbalace]

Public bug reported:

all versions up to 4.0 (I didn't test others)
file affected linux-user/elfload.c
function load_elf_image

if (phdr[i].p_type == PT_LOAD) {
   
-abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset; 
+abi_ulong a = phdr[i].p_vaddr ; // - phdr[i].p_offset; 
if (a < loaddr) {
loaddr = a;

To the best of my understanding of the elf format p_offset is not a
virtual offset. In fact, when I load statically compiled applications,
the load fails because the libc before main is trying to access phdr in
the executable image but that memory is not mapped -- this is caused by
the wrong loaddr above.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  linux-user elf loader issue

Status in QEMU:
  New

Bug description:
  all versions up to 4.0 (I didn't test others)
  file affected linux-user/elfload.c
  function load_elf_image

  if (phdr[i].p_type == PT_LOAD) {
 
  -abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset; 
  +abi_ulong a = phdr[i].p_vaddr ; // - phdr[i].p_offset; 
  if (a < loaddr) {
  loaddr = a;

  To the best of my understanding of the elf format p_offset is not a
  virtual offset. In fact, when I load statically compiled applications,
  the load fails because the libc before main is trying to access phdr
  in the executable image but that memory is not mapped -- this is
  caused by the wrong loaddr above.

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

Re: [Qemu-devel] [PATCH v2] hw/core/bus.c: Only the main system bus can have no parent

[Philippe Mathieu-Daudé]

On 5/23/19 5:05 PM, Peter Maydell wrote:
> In commit 80376c3fc2c38fdd453 in 2010 we added a workaround for
> some qbus buses not being connected to qdev devices -- if the
> bus has no parent object then we register a reset function which
> resets the bus on system reset (and unregister it when the
> bus is unparented).
> 
> Nearly a decade later, we have now no buses in the tree which
> are created with non-NULL parents, so we can remove the
> workaround and instead just assert that if the bus has a NULL
> parent then it is the main system bus.
> 
> (The absence of other parentless buses was confirmed by
> code inspection of all the callsites of qbus_create() and
> qbus_create_inplace() and cross-checked by 'make check'.)
> 
> Signed-off-by: Peter Maydell 
> Reviewed-by: Markus Armbruster 
> ---
> v1->v2: clean up also the bus_unparent() code
> ---
>  hw/core/bus.c | 21 +
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/core/bus.c b/hw/core/bus.c
> index e09843f6abe..b8839c7268d 100644
> --- a/hw/core/bus.c
> +++ b/hw/core/bus.c
> @@ -96,10 +96,9 @@ static void qbus_realize(BusState *bus, DeviceState 
> *parent, const char *name)
>  bus->parent->num_child_bus++;
>  object_property_add_child(OBJECT(bus->parent), bus->name, 
> OBJECT(bus), NULL);
>  object_unref(OBJECT(bus));
> -} else if (bus != sysbus_get_default()) {
> -/* TODO: once all bus devices are qdevified,
> -   only reset handler for main_system_bus should be registered here. 
> */
> -qemu_register_reset(qbus_reset_all_fn, bus);
> +} else {
> +/* The only bus without a parent is the main system bus */
> +assert(bus == sysbus_get_default());
>  }
>  }
>  
> @@ -108,18 +107,16 @@ static void bus_unparent(Object *obj)
>  BusState *bus = BUS(obj);
>  BusChild *kid;
>  
> +/* Only the main system bus has no parent, and that bus is never freed */
> +assert(bus->parent);
> +
>  while ((kid = QTAILQ_FIRST(>children)) != NULL) {
>  DeviceState *dev = kid->child;
>  object_unparent(OBJECT(dev));
>  }
> -if (bus->parent) {
> -QLIST_REMOVE(bus, sibling);
> -bus->parent->num_child_bus--;
> -bus->parent = NULL;
> -} else {
> -assert(bus != sysbus_get_default()); /* main_system_bus is never 
> freed */
> -qemu_unregister_reset(qbus_reset_all_fn, bus);
> -}
> +QLIST_REMOVE(bus, sibling);
> +bus->parent->num_child_bus--;
> +bus->parent = NULL;
>  }
>  
>  void qbus_create_inplace(void *bus, size_t size, const char *typename,
> 

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

[Qemu-devel] [Bug 1574327] Re: qemu-system-x86_64 -net nic, model=help outputs to stderr instead of std

[Thomas Huth]

Finally fixed here:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=7b71e03af98fd7b5e1

** Changed in: qemu
   Status: New => Fix Committed

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

Title:
  qemu-system-x86_64 -net nic,model=help outputs to stderr instead of
  std

Status in QEMU:
  Fix Committed

Bug description:
  qemu-system-x86_64 -net nic,model=help

  output comes to stderr instead of std

  
  qemu-system-x86_64 -net nic,model=help  -> stdout
  qemu-system-x86_64 -machine help -> stdout
  qemu-system-x86_64 -cpu help -> stdout

  as of
  
https://github.com/qemu/qemu/blob/044d65525f6ac2093042ae18dbf8c1300b5c1c18/net/net.c#L831

  I run qemu 2.5 on x86_64

  kind regards

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

[Qemu-devel] [Bug 1585971] Re: Host system crashes on qemu with DMA remapping

[Thomas Huth]

Triaging old bug tickets ... Can you still reproduce the issue with the
latest version of QEMU and the kernel, or could we close this bug
nowadays?

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

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

Title:
  Host system crashes on qemu with DMA remapping

Status in QEMU:
  Incomplete

Bug description:
  Hy,

  the host system crashes completely, when i try to pass an physical
  device without boot option intel_iommu=on set. In older kernel
  versions you didn't have to pass that option.

  I wonder if this can be easily checked by asking iommu state, avoiding
  a crash of the complete system.

  My data:
  cpu model: Intel(R) Core(TM) i7 CPU
  qemu version: 2.4.1-r2
  kernel version: 4.1.2 x86_64
  command line: 
  qemu-system-x86_64 -enable-kvm -drive 
file=/vms/prod/fw/fw.iso,if=virtio,format=raw -drive 
file=/vms/prod/fw/swap,if=virtio,format=raw -drive 
file=/vms/prod/fw/fwdata.iso,if=virtio,format=raw -m 512 -nographic -kernel 
/data/kernels/vmlinuz-2.6.36-gentoo-r8 -append "root=/dev/vda console=ttyS0 
earlyprintk=serial" -net nic,model=virtio,macaddr=DE:AD:BE:EF:2D:AD -net 
tap,ifname=tapfw0,script=/etc/qemu/qemu-ifup -device pci-assign,host=03:00.0

  There are also more detailed informations (if needed) here:
  https://forums.gentoo.org/viewtopic-p-7923976.html

  Thanks,
  Antonios.

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

[Qemu-devel] [RFC 0/3] block: Inquire images’ rotational info

[Max Reitz]

Hi,

http://lists.nongnu.org/archive/html/qemu-block/2019-05/msg00569.html
shows that optimizations affect HDDs and SSDs differently.  It would be
nice if we could differentiate between the two and then choose to adjust
our behavior depending on whether a given image is stored on an HDD or
not.

Or maybe it isn’t so nice.  That’s one reason this is an RFC.

The other is that I implemented recognition of the rotational status by
querying sysfs.  That looks stupid, but I didn’t find a better way
(there is a BLKROTATIONAL ioctl, but that only works on device files).

But, hey, if you look through block/file-posix.c, you’ll find that I’m
not the first to query sysfs.  hdev_get_max_segments() does so, too.  So
maybe it isn’t that bad of an idea.


What do you think?  Is the whole idea stupid?  Just the implementation?
Or does it make sense?


Max Reitz (3):
  block: Add ImageRotationalInfo
  file-posix: Inquire rotational status
  qcow2: Evaluate rotational info

 qapi/block-core.json  | 19 ++-
 block/qcow2.h |  3 ++
 include/block/block.h |  7 +
 block.c   | 20 +++-
 block/file-posix.c| 73 +++
 block/qapi.c  |  3 ++
 block/qcow2.c | 34 +---
 7 files changed, 153 insertions(+), 6 deletions(-)

-- 
2.21.0

[Qemu-devel] [Bug 1586229] Re: seabios hell

[Thomas Huth]

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

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

Title:
  seabios hell

Status in QEMU:
  Invalid

Bug description:
  getting weird annoying seabios hell and not sure how to fix it.

  ok.

  there IS a SEA-BIOS. There IS a way in.

  -I found it by mistake.(and yall need to move the BIOS key...its in
  the wrong place)

  I was tryng to boot Yosemite to re-install. I mashed the key too early
  and it wanted to boot the hard drive.

  Apparently the bios loads AFTER the hard drive wants to boot, not
  BEFORE it.And it will ONLY load when booting a hard disk.

  ..Booting hard disk...[mash F8 here but let go and wait]
  eventually will want to load the OS and clear the screen[mash F8 again]

  --Youre in!

  Its tiny, like a mini award bios but youre in! 
  -Change anything HERE, though...and kiss booting a cd goodbye!

  Im trying to diagnose a black screen, seems related to seabios, not
  the vga driver.

  -mayhaps wants to boot hard disk but in fact its not bootable as the
  installer hung(and often unices install bootloader late in process)?

  I cant boot the disc to reinstall to tell. But I have a few dos iso
  lying around...hmmm.

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

Re: [Qemu-devel] ?==?utf-8?q? ?==?utf-8?q? [PATCH 0/5]?==?utf-8?q? linux-user: Support signal passing for targets having more signals than host

[Milos Stojanovic]

Hello, Peter.
For this implemenation rt_sigqueueinfo()/rt_tgsigqueueinfo() were used to 
multiplex signals that are out of the host range. This enabled the use of a 
wider signal range with rt_sigqueueinfo(), rt_tgsigqueueinfo(), as well as 
kill() (for pid > 0) and tgkill(). A process can now use these system calls to 
send a range of signals which weren't supporeted before to itself or others 
threads or processes.

The tkill() system call and kill() with the argument pid <= 0 couldn't be 
implemented simply using this method because it requires acquiring information 
about, and sending simultaneous signals to multiple threads or processes and 
these functionalities are out of the scope of 
rt_sigqueueinfo()/rt_tgsigqueueinfo().

This patch set was primarily focused on expanding the range of real-time 
signals and making them usable but some limitations still remain. For example, 
the priority of those real-time signals, as defined by POSIX, doesn't depend on 
the actual signal number but on the host signal nuber which is used for 
multiplexing.
As it now is, the functionaly is only enabled for signals that are 
higher-numbered then the host signals but I don't see a conceptual problem in 
expanding and testing the implenentation to include other signals (e.g. the 
signals that NPTL uses).

Regards,
Miloš

[Qemu-devel] [RFC 1/3] block: Add ImageRotationalInfo

[Max Reitz]

This enum indicates whether a file is stored on a rotating disk or a
solid-state drive.  Drivers report it via the .bdrv_get_info() callback,
and if they do not, the global bdrv_get_info() implementation
automatically takes it from bs->file or bs->backing, if available.

On the QAPI side, we report the value as part of the ImageInfo
structure.

Signed-off-by: Max Reitz 
---
 qapi/block-core.json  | 19 ++-
 include/block/block.h |  7 +++
 block.c   | 20 +++-
 block/qapi.c  |  3 +++
 4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 3e4042be7f..80b9f2a69c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -130,6 +130,20 @@
   'luks': 'QCryptoBlockInfoLUKS'
   } }
 
+##
+# @ImageRotationalInfo:
+#
+# Indicates whether an image is stored on a rotating disk or not.
+#
+# @solid-state: Image is stored on a solid-state drive
+#
+# @rotating:Image is stored on a rotating disk
+#
+# Since: 4.1
+##
+{ 'enum': 'ImageRotationalInfo',
+  'data': [ 'solid-state', 'rotating' ] }
+
 ##
 # @ImageInfo:
 #
@@ -161,6 +175,9 @@
 #
 # @backing-image: info of the backing image (since 1.6)
 #
+# @rotational: indicates whether the image is stored on a rotating
+#  disk or not (since 4.1)
+#
 # @format-specific: structure supplying additional format-specific
 # information (since 1.7)
 #
@@ -173,7 +190,7 @@
'*cluster-size': 'int', '*encrypted': 'bool', '*compressed': 'bool',
'*backing-filename': 'str', '*full-backing-filename': 'str',
'*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
-   '*backing-image': 'ImageInfo',
+   '*backing-image': 'ImageInfo', '*rotational': 'ImageRotationalInfo',
'*format-specific': 'ImageInfoSpecific' } }
 
 ##
diff --git a/include/block/block.h b/include/block/block.h
index 531cf595cf..dc0f8a4671 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -31,6 +31,13 @@ typedef struct BlockDriverInfo {
  * True if this block driver only supports compressed writes
  */
 bool needs_compressed_writes;
+
+/*
+ * Indicates whether this image file is stored on a rotating disk
+ * or a solid-state drive.
+ */
+bool has_rotational_info;
+ImageRotationalInfo rotational_info;
 } BlockDriverInfo;
 
 typedef struct BlockFragInfo {
diff --git a/block.c b/block.c
index 4c3902508d..e6b2d6d23b 100644
--- a/block.c
+++ b/block.c
@@ -4950,6 +4950,8 @@ void bdrv_get_backing_filename(BlockDriverState *bs,
 int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
 {
 BlockDriver *drv = bs->drv;
+int ret;
+
 /* if bs->drv == NULL, bs is closed, so there's nothing to do here */
 if (!drv) {
 return -ENOMEDIUM;
@@ -4960,8 +4962,24 @@ int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo 
*bdi)
 }
 return -ENOTSUP;
 }
+
 memset(bdi, 0, sizeof(*bdi));
-return drv->bdrv_get_info(bs, bdi);
+ret = drv->bdrv_get_info(bs, bdi);
+if (ret < 0) {
+return ret;
+}
+
+if (!bdi->has_rotational_info && (bs->file || bs->backing)) {
+BlockDriverState *child = bs->file ? bs->file->bs : bs->backing->bs;
+BlockDriverInfo child_bdi;
+
+if (bdrv_get_info(child, _bdi) >= 0) {
+bdi->rotational_info = child_bdi.rotational_info;
+bdi->has_rotational_info = child_bdi.has_rotational_info;
+}
+}
+
+return 0;
 }
 
 ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
diff --git a/block/qapi.c b/block/qapi.c
index 0c13c86f4e..48ebfbdcc1 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -286,6 +286,9 @@ void bdrv_query_image_info(BlockDriverState *bs,
 }
 info->dirty_flag = bdi.is_dirty;
 info->has_dirty_flag = true;
+
+info->rotational = bdi.rotational_info;
+info->has_rotational = bdi.has_rotational_info;
 }
 info->format_specific = bdrv_get_specific_info(bs, );
 if (err) {
-- 
2.21.0

[Qemu-devel] [RFC 2/3] file-posix: Inquire rotational status

[Max Reitz]

On Linux, we can inquire whether the file is stored on a rotating disk
or on a solid-state drive through the sysfs.  (The BLKROTATIONAL ioctl
only works on device files themselves.)

Signed-off-by: Max Reitz 
---
 block/file-posix.c | 73 ++
 1 file changed, 73 insertions(+)

diff --git a/block/file-posix.c b/block/file-posix.c
index d018429672..f84179c0dc 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -161,6 +161,9 @@ typedef struct BDRVRawState {
 bool check_cache_dropped;
 
 PRManager *pr_mgr;
+
+bool has_rotational_info;
+ImageRotationalInfo rotational_info;
 } BDRVRawState;
 
 typedef struct BDRVRawReopenState {
@@ -378,6 +381,68 @@ static void raw_probe_alignment(BlockDriverState *bs, int 
fd, Error **errp)
 }
 }
 
+/**
+ * Tries to inquire whether the file is stored on a rotating disk or a
+ * solid-state drive.
+ */
+static void raw_update_rotational_info(BDRVRawState *s)
+{
+#ifdef CONFIG_LINUX
+struct stat st;
+char *part_fname, *rot_fname;
+char rot_info[3];
+dev_t dev;
+int rot_fd;
+int ret;
+
+s->has_rotational_info = false;
+
+if (fstat(s->fd, ) < 0) {
+return;
+}
+
+if (st.st_rdev) {
+dev = st.st_rdev;
+} else {
+dev = st.st_dev;
+}
+
+part_fname = g_strdup_printf("/sys/dev/block/%u:%u/partition",
+ major(dev), minor(dev));
+if (access(part_fname, F_OK) == 0) {
+rot_fname = g_strdup_printf("/sys/dev/block/%u:%u/../queue/rotational",
+major(dev), minor(dev));
+} else {
+rot_fname = g_strdup_printf("/sys/dev/block/%u:%u/queue/rotational",
+major(dev), minor(dev));
+}
+g_free(part_fname);
+
+rot_fd = open(rot_fname, O_RDONLY);
+g_free(rot_fname);
+if (rot_fd < 0) {
+return;
+}
+
+ret = read(rot_fd, rot_info, 2);
+close(rot_fd);
+if (ret < 2) {
+return;
+}
+
+rot_info[2] = '\0';
+if (!strcmp(rot_info, "0\n")) {
+s->rotational_info = IMAGE_ROTATIONAL_INFO_SOLID_STATE;
+s->has_rotational_info = true;
+} else if (!strcmp(rot_info, "1\n")) {
+s->rotational_info = IMAGE_ROTATIONAL_INFO_ROTATING;
+s->has_rotational_info = true;
+}
+#else /* CONFIG_LINUX */
+s->has_rotational_info = false;
+#endif
+}
+
 static void raw_parse_flags(int bdrv_flags, int *open_flags, bool has_writers)
 {
 bool read_write = false;
@@ -652,6 +717,8 @@ static int raw_open_common(BlockDriverState *bs, QDict 
*options,
 }
 #endif
 
+raw_update_rotational_info(s);
+
 bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK;
 ret = 0;
 fail:
@@ -1007,6 +1074,8 @@ static void raw_reopen_commit(BDRVReopenState *state)
 qemu_close(s->fd);
 s->fd = rs->fd;
 
+raw_update_rotational_info(s);
+
 g_free(state->opaque);
 state->opaque = NULL;
 
@@ -2731,6 +2800,10 @@ static int raw_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 BDRVRawState *s = bs->opaque;
 
 bdi->unallocated_blocks_are_zero = s->discard_zeroes;
+
+bdi->rotational_info = s->rotational_info;
+bdi->has_rotational_info = s->has_rotational_info;
+
 return 0;
 }
 
-- 
2.21.0