[Bug 1649040] Re: Ubuntu 16.04.1 Grub Splash Doesn't Appear

[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/1649040

Title:
  Ubuntu 16.04.1 Grub Splash Doesn't Appear

Status in QEMU:
  Expired

Bug description:
  My Specs:

  Slackware 14.2 x86_64 > Host
  QEMU 2.7.0

  Ubuntu 16.04.1 x86_64 > Guest

  Start options for Ubuntu:

  qemu-system-x86_64 -drive format=raw,file=ubuntu.img \
  -cpu host \
  --enable-kvm \
  -smp 2 \
  -m 4096 \
  -vga vmware \
  -soundhw ac97 \
  -usbdevice tablet \
  -rtc base=localtime \
  -usbdevice host:0781:5575

  I've started Ubuntu around 6-8 times, and I have only see the Grub
  Boot Splash appear twice, so pretty much without fail it typically
  boots past the grub splash and automatically boots...

  These are the /etc/default/grub settings; (I only changed these
  options GRUB_TIMEOUT=15 and GRUB_GFXMODE=1440x900)

  # If you change this file, run 'update-grub' afterwards to update
  # /boot/grub/grub.cfg.
  # For full documentation of the options in this file, see:
  #   info -f grub -n 'Simple configuration'

  GRUB_DEFAULT=0
  GRUB_HIDDEN_TIMEOUT=0
  GRUB_HIDDEN_TIMEOUT_QUIET=true
  GRUB_TIMEOUT=15
  GRUB_DISTRIBUTOR=`lsb_release -i -s 2> /dev/null || echo Debian`
  GRUB_CMDLINE_LINUX_DEFAULT="quiet splash"
  GRUB_CMDLINE_LINUX=""

  # Uncomment to enable BadRAM filtering, modify to suit your needs
  # This works with Linux (no patch required) and with any kernel that obtains
  # the memory map information from GRUB (GNU Mach, kernel of FreeBSD ...)
  #GRUB_BADRAM="0x01234567,0xfefefefe,0x89abcdef,0xefefefef"

  # Uncomment to disable graphical terminal (grub-pc only)
  #GRUB_TERMINAL=console

  # The resolution used on graphical terminal
  # note that you can use only modes which your graphic card supports via VBE
  # you can see them in real GRUB with the command `vbeinfo'
  GRUB_GFXMODE=1440x900

  # Uncomment if you don't want GRUB to pass "root=UUID=xxx" parameter to Linux
  #GRUB_DISABLE_LINUX_UUID=true

  # Uncomment to disable generation of recovery mode menu entries
  #GRUB_DISABLE_RECOVERY="true"

  # Uncomment to get a beep at grub start
  #GRUB_INIT_TUNE="480 440 1"

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

[Bug 1665389] Re: Nested kvm guest fails to start on a emulated Westmere CPU guest under a Broadwell CPU host

[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/1665389

Title:
  Nested kvm guest fails to start on a emulated Westmere CPU guest under
  a Broadwell CPU host

Status in QEMU:
  Expired

Bug description:
  Using latest master(5dae13), qemu fails to start any nested guest in a
  Westmere emulated guest(layer 1), under a Broadwell host(layer 0),
  with the error:

  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs:
  Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.

  The qemu command used(though other CPUs didn't work either):
  /usr/bin/qemu-custom -name guest=12ed9230-vm-el73,debug-threads=on -S -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/master-key.aes
 -machine pc-i440fx-2.9,accel=kvm,usb=off -cpu Westmere,+vmx -m 512 -realtime 
mlock=off -smp 2,sockets=2,cores=1,threads=1 -object iothread,id=iothread1 
-uuid f4ce4eba-985f-42a3-94c4-6e4a8a530347 -nographic -no-user-config 
-nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device 
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -drive 
file=/root/lago/.lago/default/images/vm-el73_root.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,serial=1,discard=unmap
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:c0:a7:c8:02,bus=pci.0,addr=0x2 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-chardev 
socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-5-12ed9230-vm-el73/org.qemu.guest_agent.0,server,nowait
 -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
 -object rng-random,id=objrng0,filename=/dev/random -device 
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0x9 -msg timestamp=on
  2017-02-16T15:14:45.840412Z qemu-custom: -chardev pty,id=charserial0: char 
device redirected to /dev/pts/2 (label charserial0)
  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs: Assertion `ret 
== cpu->kvm_msr_buf->nmsrs' failed.

  
  The CPU flags in the Westmere guest:
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush mmx fxsr sse sse2 syscall nx lm constant_tsc rep_good nopl 
pni pclmulqdq vmx ssse3 cx16 sse4_1 sse4_2 x2apic popcnt aes hypervisor lahf_lm 
arat tpr_shadow vnmi flexpriority ept vpid

  The guest kernel is 3.10.0-514.2.2.el7.x86_64.

  The CPU flags of the host(Broadwell): 
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb 
rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est 
tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch epb 
intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle 
avx2 smep bmi2 erms invpcid rtm mpx rdseed adx smap clflushopt xsaveopt xsavec 
xgetbv1 xsaves dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp

  qemu command on the host - Broadwell(which works):
  /usr/bin/qemu-kvm -name 4ffcd448-vm-el73,debug-threads=on -S -machine 
pc-i440fx-2.6,accel=kvm,usb=off -cpu Westmere,+x2apic,+vmx,+vme -m 4096 
-realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -object 
iothread,id=iothread1 -uuid 8cc0a2cf-d25a-4014-acdb-f159c376a532 -nographic 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-4ffcd448-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -drive 
file=/home/ngoldin/src/nvgoldin.github.com/lago-init-files/.lago/flags-tests/default/images/vm-el73_root.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,serial=1,discard=unmap
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -drive 
file=/home/ngoldin/src/nvgoldin.github.com/lago-init-files/.lago/flags-tests/default/images/vm-el73_additonal.qcow2,format=qcow2,if=none,id=drive-scsi0-0-0-0,serial=2,discard=unmap
 -device 
scsi-hd,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0,bootindex=2
 -netdev 

[Bug 1663079] Re: socket network not working

[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/1663079

Title:
  socket network not working

Status in QEMU:
  Expired

Bug description:
  The socket network type is no longer working in 2.8.0.

  When trying to establish a network between 2 qemu instances:

  The listening host:
  qemu-system-x86_64 -netdev socket,id=in0,listen=127.0.0.1: -device 
virtio-net-pci,netdev=in0

  works fine, but for the second one:

  qemu-system-x86_64 -netdev socket,id=in0,connect=127.0.0.1:
  -device virtio-net-pci,netdev=in0

  It fails with:

  qemu-system-x86_64: -device virtio-net-pci,netdev=in0: Property
  'virtio-net-device.netdev' can't find value 'in0'

  netstat shows a new connection to port  in time_wait state every
  time.

  host: kernel 4.4.38, 64bits.

  It was working fine with previous version.

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

[PATCH] linux-user/riscv: fix up struct target_ucontext definition

[LIU Zhiwei]

As struct target_ucontext will be transfered to signal handler, it
must keep pace with struct ucontext_t defined in Linux kernel.

Signed-off-by: LIU Zhiwei 
---
 linux-user/riscv/signal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 83ecc6f799..67a95dbc7b 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -40,8 +40,9 @@ struct target_ucontext {
 unsigned long uc_flags;
 struct target_ucontext *uc_link;
 target_stack_t uc_stack;
-struct target_sigcontext uc_mcontext;
 target_sigset_t uc_sigmask;
+uint8_t   __unused[1024 / 8 - sizeof(target_sigset_t)];
+struct target_sigcontext uc_mcontext QEMU_ALIGNED(16);
 };
 
 struct target_rt_sigframe {
-- 
2.23.0

[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2

[Russell Morris]

Hi,

I built against the latest library I could (Windows Insider Preview,
SDK) - same failure.

Thoughts?

Thanks!

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

Title:
  Failed to emulate MMIO access with EmulatorReturnStatus: 2

Status in QEMU:
  New

Bug description:
  I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in 
Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the 
following problem: 
  qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with 
EmulatorReturnStatus: 2
  qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor

  
  configuration directives:

  ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\
   --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\
   --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\
   --disable-werror --disable-stack-protector --prefix="../../QEMU-bin"

  
  Qemu command line:
  qemu-system-x86_64.exe -m 1024 -cdrom 
"C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso"
 -display sdl -machine q35 -accel whpx

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

[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2

[Russell Morris]

Should say - I rebuilt (today). Still no joy.

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

Title:
  Failed to emulate MMIO access with EmulatorReturnStatus: 2

Status in QEMU:
  New

Bug description:
  I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in 
Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the 
following problem: 
  qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with 
EmulatorReturnStatus: 2
  qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor

  
  configuration directives:

  ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\
   --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\
   --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\
   --disable-werror --disable-stack-protector --prefix="../../QEMU-bin"

  
  Qemu command line:
  qemu-system-x86_64.exe -m 1024 -cdrom 
"C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso"
 -display sdl -machine q35 -accel whpx

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

Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels

[Nathan Chancellor]

On Sat, Apr 11, 2020 at 11:57:23PM +1000, Nicholas Piggin wrote:
> Nicholas Piggin's on April 11, 2020 7:32 pm:
> > Nathan Chancellor's on April 11, 2020 10:53 am:
> >> The tt.config values are needed to reproduce but I did not verify that
> >> ONLY tt.config was needed. Other than that, no, we are just building
> >> either pseries_defconfig or powernv_defconfig with those configs and
> >> letting it boot up with a simple initramfs, which prints the version
> >> string then shuts the machine down.
> >> 
> >> Let me know if you need any more information, cheers!
> > 
> > Okay I can reproduce it. Sometimes it eventually recovers after a long
> > pause, and some keyboard input often helps it along. So that seems like 
> > it might be a lost interrupt.
> > 
> > POWER8 vs POWER9 might just be a timing thing if P9 is still hanging
> > sometimes. I wasn't able to reproduce it with defconfig+tt.config, I
> > needed your other config with various other debug options.
> > 
> > Thanks for the very good report. I'll let you know what I find.
> 
> It looks like a qemu bug. Booting with '-d int' shows the decrementer 
> simply stops firing at the point of the hang, even though MSR[EE]=1 and 
> the DEC register is wrapping. Linux appears to be doing the right thing 
> as far as I can tell (not losing interrupts).
> 
> This qemu patch fixes the boot hang for me. I don't know that qemu 
> really has the right idea of "context synchronizing" as defined in the
> powerpc architecture -- mtmsrd L=1 is not context synchronizing but that
> does not mean it can avoid looking at exceptions until the next such
> event. It looks like the decrementer exception goes high but the
> execution of mtmsrd L=1 is ignoring it.
> 
> Prior to the Linux patch 3282a3da25b you bisected to, interrupt replay
> code would return with an 'rfi' instruction as part of interrupt return,
> which probably helped to get things moving along a bit. However it would
> not be foolproof, and Cedric did say he encountered some mysterious
> lockups under load with qemu powernv before that patch was merged, so
> maybe it's the same issue?
> 
> Thanks,
> Nick
> 
> The patch is a bit of a hack, but if you can run it and verify it fixes
> your boot hang would be good.

Yes, with this patch applied on top of 5.0.0-rc2 and using the
pseries-3.1 and powernv8 machines, I do not see any hangs with a clang
built kernel at b032227c62939b5481bcd45442b36dfa263f4a7c across 100
boots.

If you happen to send it upstream:

Tested-by: Nathan Chancellor 

Thanks for looking into it!

> ---
> 
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b207fb5386..1d997f5c32 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4364,12 +4364,21 @@ static void gen_mtmsrd(DisasContext *ctx)
>  if (ctx->opcode & 0x0001) {
>  /* Special form that does not need any synchronisation */
>  TCGv t0 = tcg_temp_new();
> +TCGv t1 = tcg_temp_new();
>  tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
>  (1 << MSR_RI) | (1 << MSR_EE));
> -tcg_gen_andi_tl(cpu_msr, cpu_msr,
> +tcg_gen_andi_tl(t1, cpu_msr,
>  ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
> -tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
> +tcg_gen_or_tl(t1, t1, t0);
> +
> +gen_update_nip(ctx, ctx->base.pc_next);
> +gen_helper_store_msr(cpu_env, t1);
>  tcg_temp_free(t0);
> +tcg_temp_free(t1);
> +/* Must stop the translation as machine state (may have) changed */
> +/* Note that mtmsr is not always defined as context-synchronizing */
> +gen_stop_exception(ctx);
> +
>  } else {
>  /*
>   * XXX: we need to update nip before the store if we enter

Cheers,
Nathan

Re: [PATCH-for-5.0 1/2] hw/display/sm501: Avoid heap overflow in sm501_2d_operation()

[Peter Maydell]

On Sat, 11 Apr 2020 at 20:45, Philippe Mathieu-Daudé  wrote:
> Buffer overflows are security issues because they allow attacker to
> arbitrarily write data in the process memory, and eventually take
> control of it. When attacker takes control, it can access underlying
> private data.

Note that for QEMU our security boundary is "VMs using KVM"; so
buffer overflows are a security issue in code and devices that
you can use in a KVM setup (including pluggable devices like
PCI devices) but not devices you can only use in a TCG setup
(where they're just bugs, though obviously ones we should
fix sooner rather than later).

thanks
-- PMM

[Bug 1872237] Re: SysTick reload behavior emulated incorrectly

[Peter Maydell]

Yeah, our systick implementation is broken; I've known about this for
ages but never got round to trying to work through what the right way to
implement the behaviour is. I do have some more time to work on
M-profile stuff coming up at some point so I might get round to this if
nobody else does first.

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

Title:
  SysTick reload behavior emulated incorrectly

Status in QEMU:
  New

Bug description:
  QEMU's emuation of SysTick on ARM is incorrect with respect to reload
  behavior.  This issue is described here, and also in a repository
  dedicated to the issue:

https://github.com/oxidecomputer/qemu-systick-bug

  
  (What follows is in Markdown, which I understand that Launchpad does
  not support; see the repository linked above for a rendering of it.)

  Take this Rust program:

  ```rust
  #![no_std]
  #![no_main]

  extern crate panic_semihosting;

  use cortex_m_rt::entry;
  use cortex_m_semihosting::hprintln;
  use cortex_m::peripheral::syst::SystClkSource;
  use cortex_m::peripheral::SYST;

  fn delay(syst:  cortex_m::peripheral::SYST, ms: u32)
  {
  /*
   * Configured for the LM3S6965, which has a default CPU clock of 12 Mhz
   */
  let reload = 12_000 * ms;

  syst.set_reload(reload);
  syst.clear_current();
  syst.enable_counter();

  hprintln!("waiting for {} ms (SYST_CVR={}) ...",
  ms, SYST::get_current()
  ).unwrap();

  while !syst.has_wrapped() {}

  hprintln!("  ... done (SYST_CVR={})\n",
  SYST::get_current()).unwrap();

  syst.disable_counter();
  }

  #[entry]
  fn main() -> ! {
  let p = cortex_m::Peripherals::take().unwrap();
  let mut syst = p.SYST;

  syst.set_clock_source(SystClkSource::Core);

  loop {
  delay( syst, 1000);
  delay( syst, 100);
  }
  }
  ```

  This program should oscillate between waiting for one second and waiting
  for 100 milliseconds.  Under hardware, this is more or less what it does
  (depending on core clock frequency); e.g., from an STM32F4107 (connected via
  OCD and with semi-hosting enabled):

  ```
  waiting for 1000 ms (SYST_CVR=1149) ...
... done (SYST_CVR=1102)

  waiting for 100 ms (SYST_CVR=1199949) ...
... done (SYST_CVR=1199897)

  waiting for 1000 ms (SYST_CVR=1149) ...
... done (SYST_CVR=11999885)

  waiting for 100 ms (SYST_CVR=1199949) ...
... done (SYST_CVR=1199897)

  waiting for 1000 ms (SYST_CVR=1149) ...
... done (SYST_CVR=11999885)

  ```

  Under QEMU, however, its behavior is quite different:

  ```
  $ cargo run
  Finished dev [unoptimized + debuginfo] target(s) in 0.03s
   Running `qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic 
-semihosting-config enable=on,target=native -kernel 
target/thumbv7m-none-eabi/debug/qemu-systick-bug`
  waiting for 1000 ms (SYST_CVR=11999658) ...
... done (SYST_CVR=11986226)

  waiting for 100 ms (SYST_CVR=0) ...
... done (SYST_CVR=1186560)

  waiting for 1000 ms (SYST_CVR=1185996) ...
... done (SYST_CVR=11997350)

  waiting for 100 ms (SYST_CVR=0) ...
... done (SYST_CVR=1186581)
  ```

  In addition to the values being strangely wrong, the behavior is wrong:
  the first wait correctly waits for 1000 ms -- but the subsequent wait
  (which should be for 100 ms) is in fact 1000 ms, and the next wait (which
  should be for 1000 ms) is in fact 100 ms.  (That is, it appears as if
  the periods of the two delays have been switched.)

  The problems is that the QEMU ARM emulation code does not reload SYST_CVR from
  SYST_RVR if SYST_CSR.ENABLE is not set -- and moreover, that SYST_CVR is
  not reloaded from SYST_RVR even when SYST_CSR.ENABLE becomes set.  This is
  very explicit; from
  https://github.com/qemu/qemu/blob/8bac3ba57eecc466b7e73dabf7d19328a59f684e/hw/timer/armv7m_systick.c#L42-L60;>hw/timer/armv7m_systick.c:

  ```c
  static void systick_reload(SysTickState *s, int reset)
  {
  /* The Cortex-M3 Devices Generic User Guide says that "When the
   * ENABLE bit is set to 1, the counter loads the RELOAD value from the
   * SYST RVR register and then counts down". So, we need to check the
   * ENABLE bit before reloading the value.
   */
  trace_systick_reload();

  if ((s->control & SYSTICK_ENABLE) == 0) {
  return;
  }

  if (reset) {
  s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
  }
  s->tick += (s->reload + 1) * systick_scale(s);
  timer_mod(s->timer, s->tick);
  }
  ```

  The statement in the code is stronger than the statement in the
  https://static.docs.arm.com/ddi0403/eb/DDI0403E_B_armv7m_arm.pdf;>ARMv7-M 
Architecture Reference Manual (sec B3.3.1):

  > Writing to SYST_CVR clears both the register and the COUNTFLAG status
  > bit to zero. This causes the SysTick logic to reload SYST_CVR from SYST_RVR
  > on 

Re: [PATCH for-5.0? 1/3] configure: Honour --disable-werror for Sphinx

[Richard Henderson]

On 4/11/20 11:29 AM, Peter Maydell wrote:
> If we are not making warnings fatal for compilation, make them
> non-fatal when building the Sphinx documentation also.  (For instance
> Sphinx 3.0 warns about some constructs that older versions were happy
> with, which is a build failure if we use the warnings-as-errors
> flag.)
> 
> This provides a workaround at least for LP:1872113.
> 
> Signed-off-by: Peter Maydell 
> ---
>  configure | 9 -
>  Makefile  | 2 +-
>  2 files changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [PATCH for-5.0? 2/3] scripts/kernel-doc: Add missing close-paren in c:function directives

[Richard Henderson]

On 4/11/20 11:29 AM, Peter Maydell wrote:
> When kernel-doc generates a 'c:function' directive for a function
> one of whose arguments is a function pointer, it fails to print
> the close-paren after the argument list of the function pointer
> argument, for instance:
>   .. c:function:: void memory_region_init_resizeable_ram (MemoryRegion * mr, 
> struct Object * owner, const char * name, uint64_t size, uint64_t max_size, 
> void (*resized) (const char*, uint64_t length, void *host, Error ** errp)
> 
> which should have a ')' after the 'void *host' which is the
> last argument to 'resized'.
> 
> Older versions of Sphinx don't try to parse the argumnet
> to c:function, but Sphinx 3.0 does do this and will complain:
> 
>   
> /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:834:
>  WARNING: Error in declarator or parameters
>   Invalid C declaration: Expecting "," or ")" in parameters, got "EOF". 
> [error at 208]
> void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object 
> * owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) 
> (const char*, uint64_t length, void *host, Error ** errp)
> 
> ^
> 
> Add the missing close-paren.
> 
> Signed-off-by: Peter Maydell 
> ---
>  scripts/kernel-doc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

[Bug 1872237] [NEW] SysTick reload behavior emulated incorrectly

[Bryan Cantrill]

Public bug reported:

QEMU's emuation of SysTick on ARM is incorrect with respect to reload
behavior.  This issue is described here, and also in a repository
dedicated to the issue:

  https://github.com/oxidecomputer/qemu-systick-bug


(What follows is in Markdown, which I understand that Launchpad does
not support; see the repository linked above for a rendering of it.)

Take this Rust program:

```rust
#![no_std]
#![no_main]

extern crate panic_semihosting;

use cortex_m_rt::entry;
use cortex_m_semihosting::hprintln;
use cortex_m::peripheral::syst::SystClkSource;
use cortex_m::peripheral::SYST;

fn delay(syst:  cortex_m::peripheral::SYST, ms: u32)
{
/*
 * Configured for the LM3S6965, which has a default CPU clock of 12 Mhz
 */
let reload = 12_000 * ms;

syst.set_reload(reload);
syst.clear_current();
syst.enable_counter();

hprintln!("waiting for {} ms (SYST_CVR={}) ...",
ms, SYST::get_current()
).unwrap();

while !syst.has_wrapped() {}

hprintln!("  ... done (SYST_CVR={})\n",
SYST::get_current()).unwrap();

syst.disable_counter();
}

#[entry]
fn main() -> ! {
let p = cortex_m::Peripherals::take().unwrap();
let mut syst = p.SYST;

syst.set_clock_source(SystClkSource::Core);

loop {
delay( syst, 1000);
delay( syst, 100);
}
}
```

This program should oscillate between waiting for one second and waiting
for 100 milliseconds.  Under hardware, this is more or less what it does
(depending on core clock frequency); e.g., from an STM32F4107 (connected via
OCD and with semi-hosting enabled):

```
waiting for 1000 ms (SYST_CVR=1149) ...
  ... done (SYST_CVR=1102)

waiting for 100 ms (SYST_CVR=1199949) ...
  ... done (SYST_CVR=1199897)

waiting for 1000 ms (SYST_CVR=1149) ...
  ... done (SYST_CVR=11999885)

waiting for 100 ms (SYST_CVR=1199949) ...
  ... done (SYST_CVR=1199897)

waiting for 1000 ms (SYST_CVR=1149) ...
  ... done (SYST_CVR=11999885)

```

Under QEMU, however, its behavior is quite different:

```
$ cargo run
Finished dev [unoptimized + debuginfo] target(s) in 0.03s
 Running `qemu-system-arm -cpu cortex-m3 -machine lm3s6965evb -nographic 
-semihosting-config enable=on,target=native -kernel 
target/thumbv7m-none-eabi/debug/qemu-systick-bug`
waiting for 1000 ms (SYST_CVR=11999658) ...
  ... done (SYST_CVR=11986226)

waiting for 100 ms (SYST_CVR=0) ...
  ... done (SYST_CVR=1186560)

waiting for 1000 ms (SYST_CVR=1185996) ...
  ... done (SYST_CVR=11997350)

waiting for 100 ms (SYST_CVR=0) ...
  ... done (SYST_CVR=1186581)
```

In addition to the values being strangely wrong, the behavior is wrong:
the first wait correctly waits for 1000 ms -- but the subsequent wait
(which should be for 100 ms) is in fact 1000 ms, and the next wait (which
should be for 1000 ms) is in fact 100 ms.  (That is, it appears as if
the periods of the two delays have been switched.)

The problems is that the QEMU ARM emulation code does not reload SYST_CVR from
SYST_RVR if SYST_CSR.ENABLE is not set -- and moreover, that SYST_CVR is
not reloaded from SYST_RVR even when SYST_CSR.ENABLE becomes set.  This is
very explicit; from
https://github.com/qemu/qemu/blob/8bac3ba57eecc466b7e73dabf7d19328a59f684e/hw/timer/armv7m_systick.c#L42-L60;>hw/timer/armv7m_systick.c:

```c
static void systick_reload(SysTickState *s, int reset)
{
/* The Cortex-M3 Devices Generic User Guide says that "When the
 * ENABLE bit is set to 1, the counter loads the RELOAD value from the
 * SYST RVR register and then counts down". So, we need to check the
 * ENABLE bit before reloading the value.
 */
trace_systick_reload();

if ((s->control & SYSTICK_ENABLE) == 0) {
return;
}

if (reset) {
s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
}
s->tick += (s->reload + 1) * systick_scale(s);
timer_mod(s->timer, s->tick);
}
```

The statement in the code is stronger than the statement in the
https://static.docs.arm.com/ddi0403/eb/DDI0403E_B_armv7m_arm.pdf;>ARMv7-M 
Architecture Reference Manual (sec B3.3.1):

> Writing to SYST_CVR clears both the register and the COUNTFLAG status
> bit to zero. This causes the SysTick logic to reload SYST_CVR from SYST_RVR
> on the next timer clock. A write to SYST_CVR does not trigger the
> SysTick exception logic.

Note that this does not mention the behavior on a write to SYST_CVR when
SYST_CSR.ENABLE is not set -- and in particular, does *not* say that writes to
SYST_CVR will be ignored if SYST_CSR.ENABLE is not set.

Section 3.3.1 does go on to say:

> The SYST_CVR value is UNKNOWN on reset. Before enabling the SysTick counter,u
> software must write the required counter value to SYST_RVR, and then write
> to SYST_CVR. This clears SYST_CVR to zero. When enabled, the counter 
> reloads the value from SYST_RVR, and counts down from that value, rather]
> than from an arbitrary value.

(This is more or less what has been quoted in the implementation of

Re: [PATCH-for-5.0 1/2] hw/display/sm501: Avoid heap overflow in sm501_2d_operation()

[Philippe Mathieu-Daudé]

[Cc'ing Sebastian Bauer & Aurelien Jarno because I'm not sure the
problem was introduced by commit debc7e7dad1 or 07d8a50cb0e).

On 4/11/20 8:05 PM, BALATON Zoltan wrote:
> On Sat, 11 Apr 2020, Philippe Mathieu-Daudé wrote:
>> Zhang Zi Ming reported a heap overflow in the Drawing Engine of
>> the SM501 companion chip model, in particular in the COPY_AREA()
>> macro in sm501_2d_operation().
>>
>> As I have no idea what this code is supposed to do, add a simple
>> check to avoid the heap overflow. This fixes:
> 
> As the function name says it performs a 2D blitter operation. The code
> had no bounds checking at all and there are easier ways to crash it by
> writing any unimplemented register for which it has abort() calls in the
> device implementation. I'm not sure this patch fixes all possible
> overflows but at least plugs this particular one so why not.

Buffer overflows are security issues because they allow attacker to
arbitrarily write data in the process memory, and eventually take
control of it. When attacker takes control, it can access underlying
private data.
While aborts are pointless to take control, they can still lead to
denial of service (i.e. keeping cloud provider hw busy while rebooting VMs).

I am not sure why 'security researchers' care about the R2D-PLUS and
aCube Sam460ex machines... I suppose they get money each time they
report a such issue.

> 
> Acked-by: BALATON Zoltan 
> 
> Otherwise this device emulation should be rewritten sometimes but I had
> not time for that.

It is pointless to implement features we'll never use, so if you are
happy with this fix as it, I'm happy too :) No need to rewrite a device
that works for our use cases.

> 
> Regards,
> BALATON Zoltan
> 
>>  =
>>  ==20518==ERROR: AddressSanitizer: heap-buffer-overflow on address
>> 0x7f6f4c3f at pc 0x55b1e1d358f0 bp 0x7ffce464dfb0 sp 0x7ffce464dfa8
>>  READ of size 1 at 0x7f6f4c3f thread T0
>>  #0 0x55b1e1d358ef in sm501_2d_operation hw/display/sm501.c:788:13
>>  #1 0x55b1e1d32c38 in sm501_2d_engine_write
>> hw/display/sm501.c:1466:13
>>  #2 0x55b1e0cd19d8 in memory_region_write_accessor memory.c:483:5
>>  #3 0x55b1e0cd1404 in access_with_adjusted_size memory.c:544:18
>>  #4 0x55b1e0ccfb9d in memory_region_dispatch_write memory.c:1476:16
>>  #5 0x55b1e0ae55a8 in flatview_write_continue exec.c:3125:23
>>  #6 0x55b1e0ad3e87 in flatview_write exec.c:3165:14
>>  #7 0x55b1e0ad3a24 in address_space_write exec.c:3256:18
>>
>>  0x7f6f4c3f is located 4194303 bytes to the right of 4194304-byte
>> region [0x7f6f4bc0,0x7f6f4c00)
>>  allocated by thread T0 here:
>>  #0 0x55b1e0a6e715 in __interceptor_posix_memalign
>> (ppc64-softmmu/qemu-system-ppc64+0x19c0715)
>>  #1 0x55b1e31c1482 in qemu_try_memalign util/oslib-posix.c:189:11
>>  #2 0x55b1e31c168c in qemu_memalign util/oslib-posix.c:205:27
>>  #3 0x55b1e11a00b3 in spapr_reallocate_hpt hw/ppc/spapr.c:1560:23
>>  #4 0x55b1e11a0ce4 in spapr_setup_hpt hw/ppc/spapr.c:1593:5
>>  #5 0x55b1e11c2fba in spapr_machine_reset hw/ppc/spapr.c:1644:9
>>  #6 0x55b1e1368b01 in qemu_system_reset softmmu/vl.c:1391:9
>>  #7 0x55b1e1375af3 in qemu_init softmmu/vl.c:4436:5
>>  #8 0x55b1e2fc8a59 in main softmmu/main.c:48:5
>>  #9 0x7f6f8150bf42 in __libc_start_main (/lib64/libc.so.6+0x23f42)
>>
>>  SUMMARY: AddressSanitizer: heap-buffer-overflow
>> hw/display/sm501.c:788:13 in sm501_2d_operation
>>  Shadow bytes around the buggy address:
>>    0x0fee69877fa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>    0x0fee69877fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>    0x0fee69877fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>    0x0fee69877fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>    0x0fee69877fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>>  =>0x0fee69877ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
>>    0x0fee69878000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x0fee69878010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x0fee69878020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x0fee69878030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x0fee69878040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>  Shadow byte legend (one shadow byte represents 8 application bytes):
>>    Addressable:   00
>>    Partially addressable: 01 02 03 04 05 06 07
>>    Heap left redzone:   fa
>>    Freed heap region:   fd
>>    Poisoned by user:    f7
>>    ASan internal:   fe
>>  ==20518==ABORTING
>>

I forgot here:

Cc: qemu-sta...@nongnu.org

>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786026
>> Reported-by: Zhang Zi Ming <1015138...@qq.com>
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>> Per the links on
>> https://bugzilla.redhat.com/show_bug.cgi?id=1808510 there is probably
>> a CVE assigned to this, but I don't have 

[Bug 1872113] Re: qemu docs fails to build with Sphinx 3.0.x

[Peter Maydell]

I've sent a proposed fix to the list:
https://patchew.org/QEMU/20200411182934.28678-1-peter.mayd...@linaro.org/


** Changed in: qemu
   Status: New => In Progress

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

Title:
  qemu docs fails to build with Sphinx 3.0.x

Status in QEMU:
  In Progress

Bug description:
  We've just updated Sphinx to version 3.0.1 and qemu fails to build the
  docs with this version.

  Here's the relevant section in the build log.

  CONFDIR="/etc/qemu" /usr/bin/sphinx-build-3  -W -b html -D version=4.2.92 -D 
release="4.2.92 (qemu-5.0.0-0.rc2.0.1.mga8)" -d .doctrees/devel-html 
/home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/devel docs/devel
  Running Sphinx v3.0.1
  making output directory... done
  building [mo]: targets for 0 po files that are out of date
  building [html]: targets for 14 source files that are out of date
  updating environment: [new config] 14 added, 0 changed, 0 removed
  reading sources... [  7%] bitops
  reading sources... [ 14%] decodetree
  reading sources... [ 21%] index
  reading sources... [ 28%] kconfig
  reading sources... [ 35%] loads-stores
  reading sources... [ 42%] memory
  reading sources... [ 50%] migration
  reading sources... [ 57%] reset
  reading sources... [ 64%] s390-dasd-ipl
  reading sources... [ 71%] secure-coding-practices
  reading sources... [ 78%] stable-process
  reading sources... [ 85%] tcg
  reading sources... [ 92%] tcg-plugins
  reading sources... [100%] testing

  Warning, treated as error:
  /home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/../include/exec/memory.h:3:Type 
must be either just a name or a typedef-like declaration.
  If just a name:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name, got keyword: 
struct [error at 6]
  struct MemoryListener
  --^
  If typedef-like declaration:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name. [error at 21]
  struct MemoryListener
  -^

  make: *** [Makefile:1095: docs/devel/index.html] Error 2
  make: *** Waiting for unfinished jobs

  I found this commit for memory.h that includes the section that faults.
  
https://github.com/qemu/qemu/commit/5d248213180749e674fbccbacc6ee9c38499abb3#diff-d892cbf314945b44699534cc1de4ebbd

  You can see the whole build log here.
  
https://pkgsubmit.mageia.org/uploads/failure/cauldron/core/release/20200410161120.tv.duvel.699/log/qemu-5.0.0-0.rc2.0.1.mga8/build.0.20200410161338.log

  System: Mageia Cauldron

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

Re: [PATCH 01/31] target/arm: Add ID_AA64ZFR0 fields and isar_feature_aa64_sve2

[Alex Bennée]

Richard Henderson  writes:

> Will be used for SVE2 isa subset enablement.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH] tcg/mips: mips sync* encode error

[Aleksandar Markovic]

20:08 Sub, 11.04.2020. Richard Henderson  је
написао/ла:
>
> On 4/11/20 5:46 AM, lixinyu wrote:
> > OPC_SYNC_WMB, OPC_SYNC_MB, OPC_SYNC_ACQUIRE, OPC_SYNC_RELEASE and
> > OPC_SYNC_RMB have wrong encode. According to the mips manual,
> > their encode should be 'OPC_SYNC | 0x?? << 6' rather than
> > 'OPC_SYNC | 0x?? << 5'. Wrong encode can lead illegal instruction
> > errors. These instructions often appear with multi-threaded
> > simulation.
>
> Good catch.
>
> Reviewed-by: Richard Henderson 
>
> Queued to tcg-for-5.0.
>

Reviewed-by: Aleksandar Markovic 

Thanks to Lixinyu, in the first place, for reporting the problem, and also
thanks to Richard for reviewing and incorporating this patch into his queue
for 5.0. I also think, as Richard does, that this should go into 5.0, even
in this final stages.

Happy Easter, or hapy and sunny weekend, whatever you choose.

Aleksandar

>
> r~
>

[PATCH for-5.0? 3/3] kernel-doc: Use c:struct for Sphinx 3.0 and later

[Peter Maydell]

The kernel-doc Sphinx plugin and associated script currently emit
'c:type' directives for "struct foo" documentation.

Sphinx 3.0 warns about this:
  /home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:3: 
WARNING: Type must be either just a name or a typedef-like declaration.
  If just a name:
Error in declarator or parameters
Invalid C declaration: Expected identifier in nested name, got keyword: 
struct [error at 6]
  struct MemoryListener
  --^
  If typedef-like declaration:
Error in declarator or parameters
Invalid C declaration: Expected identifier in nested name. [error at 21]
  struct MemoryListener
  -^

because it wants us to use the new-in-3.0 'c:struct' instead.

Plumb the Sphinx version through to the kernel-doc script
and use it to select 'c:struct' for newer versions than 3.0.

Fixes: LP:1872113
Signed-off-by: Peter Maydell 
---
 docs/sphinx/kerneldoc.py |  1 +
 scripts/kernel-doc   | 16 +++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/docs/sphinx/kerneldoc.py b/docs/sphinx/kerneldoc.py
index 1159405cb92..3e879402064 100644
--- a/docs/sphinx/kerneldoc.py
+++ b/docs/sphinx/kerneldoc.py
@@ -99,6 +99,7 @@ class KernelDocDirective(Directive):
 env.note_dependency(os.path.abspath(f))
 cmd += ['-export-file', f]
 
+cmd += ['-sphinx-version', sphinx.__version__]
 cmd += [filename]
 
 try:
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 8dc30e01e58..030b5c8691f 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -71,6 +71,8 @@ Output selection (mutually exclusive):
DOC: sections. May be specified multiple times.
 
 Output selection modifiers:
+  -sphinx-version VER   Generate rST syntax for the specified Sphinx version.
+Only works with reStructuredTextFormat.
   -no-doc-sections Do not output DOC: sections.
   -enable-linenoEnable output of #define LINENO lines. Only works with
 reStructuredText format.
@@ -286,6 +288,7 @@ use constant {
 };
 my $output_selection = OUTPUT_ALL;
 my $show_not_found = 0;# No longer used
+my $sphinx_version = "0.0"; # if not specified, assume old
 
 my @export_file_list;
 
@@ -436,6 +439,8 @@ while ($ARGV[0] =~ m/^--?(.*)/) {
$enable_lineno = 1;
 } elsif ($cmd eq 'show-not-found') {
$show_not_found = 1;  # A no-op but don't fail
+} elsif ($cmd eq 'sphinx-version') {
+$sphinx_version = shift @ARGV;
 } else {
# Unknown argument
 usage();
@@ -963,7 +968,16 @@ sub output_struct_rst(%) {
 my $oldprefix = $lineprefix;
 my $name = $args{'type'} . " " . $args{'struct'};
 
-print "\n\n.. c:type:: " . $name . "\n\n";
+# Sphinx 3.0 and up will emit warnings for "c:type:: struct Foo".
+# It wants to see "c:struct:: Foo" (and will add the word 'struct' in
+# the rendered output).
+if ((split(/\./, $sphinx_version))[0] >= 3) {
+my $sname = $name;
+$sname =~ s/^struct //;
+print "\n\n.. c:struct:: " . $sname . "\n\n";
+} else {
+print "\n\n.. c:type:: " . $name . "\n\n";
+}
 print_lineno($declaration_start_line);
 $lineprefix = "   ";
 output_highlight_rst($args{'purpose'});
-- 
2.20.1

[PATCH for-5.0? 1/3] configure: Honour --disable-werror for Sphinx

[Peter Maydell]

If we are not making warnings fatal for compilation, make them
non-fatal when building the Sphinx documentation also.  (For instance
Sphinx 3.0 warns about some constructs that older versions were happy
with, which is a build failure if we use the warnings-as-errors
flag.)

This provides a workaround at least for LP:1872113.

Signed-off-by: Peter Maydell 
---
 configure | 9 -
 Makefile  | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 22870f38672..7b9ae0418d0 100755
--- a/configure
+++ b/configure
@@ -4928,6 +4928,12 @@ if check_include sys/kcov.h ; then
 kcov=yes
 fi
 
+# If we're making warnings fatal, apply this to Sphinx runs as well
+sphinx_werror=""
+if test "$werror" = "yes"; then
+sphinx_werror="-W"
+fi
+
 # Check we have a new enough version of sphinx-build
 has_sphinx_build() {
 # This is a bit awkward but works: create a trivial document and
@@ -4936,7 +4942,7 @@ has_sphinx_build() {
 # sphinx-build doesn't exist at all or if it is too old.
 mkdir -p "$TMPDIR1/sphinx"
 touch "$TMPDIR1/sphinx/index.rst"
-"$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx" 
"$TMPDIR1/sphinx/out" >/dev/null 2>&1
+"$sphinx_build" $sphinx_werror -c "$source_path/docs" -b html 
"$TMPDIR1/sphinx" "$TMPDIR1/sphinx/out" >/dev/null 2>&1
 }
 
 # Check if tools are available to build documentation.
@@ -7631,6 +7637,7 @@ echo "INSTALL_PROG=$install -c -m 0755" >> 
$config_host_mak
 echo "INSTALL_LIB=$install -c -m 0644" >> $config_host_mak
 echo "PYTHON=$python" >> $config_host_mak
 echo "SPHINX_BUILD=$sphinx_build" >> $config_host_mak
+echo "SPHINX_WERROR=$sphinx_werror" >> $config_host_mak
 echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
 if $iasl -h > /dev/null 2>&1; then
diff --git a/Makefile b/Makefile
index 84ef8816000..8a9113e6663 100644
--- a/Makefile
+++ b/Makefile
@@ -1076,7 +1076,7 @@ sphinxdocs: $(MANUAL_BUILDDIR)/devel/index.html \
 # Note the use of different doctree for each (manual, builder) tuple;
 # this works around Sphinx not handling parallel invocation on
 # a single doctree: https://github.com/sphinx-doc/sphinx/issues/2946
-build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" $(SPHINX_BUILD) 
$(if $(V),,-q) -W -b $2 -D version=$(VERSION) -D release="$(FULL_VERSION)" -d 
.doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 
,"SPHINX","$(MANUAL_BUILDDIR)/$1")
+build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" $(SPHINX_BUILD) 
$(if $(V),,-q) $(SPHINX_WERROR) -b $2 -D version=$(VERSION) -D 
release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 
$(MANUAL_BUILDDIR)/$1 ,"SPHINX","$(MANUAL_BUILDDIR)/$1")
 # We assume all RST files in the manual's directory are used in it
 manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst 
$(SRC_PATH)/docs/$1/*/*.rst) \
   $(SRC_PATH)/docs/defs.rst.inc \
-- 
2.20.1

[PATCH for-5.0? 0/3] Make docs build work with Sphinx 3

[Peter Maydell]

Our current docs don't build with Sphinx 3, as noted in
https://bugs.launchpad.net/bugs/1872113 -- this is a combination of:
 (1) we are using the sphinx-build -W option so warnings are treated
 as errors
 (3) a kernel-doc script bug meant it was omitting a close-paren
 when a function parameter was a function pointer; older Sphinx
 ignored this but Sphinx 3 parses the function declaration and
 warns about it; and because of (1) this is fatal to the QEMU build
 (2) Sphinx 3 makes a breaking change in how it wants C structs
 to be marked up (moving from 'c:type:: struct Foo' to
 'c:struct:: Foo'); our use of the old syntax provokes a
 warning, which again because of point (1) is fatal

Patch 1 extends configure's --disable-werror to cover Sphinx as
well as the C compiler, so that at least there is a workaround
(which will be automatic for release builds).

Patch 2 fixes the trivial kernel-doc bug.

Patch 3 adds and uses a new --sphinx-version option to kernel-doc,
so that our Sphinx plugin can pass the Sphinx version down and
the script can then choose the right syntax.

I've marked this up as 'for-5.0?' because I think it would be
nice if at least patch 1 went in. Patch 2 seems uncontroversial
(though I guess we should forward it up to the kernel folks
since kernel-doc is from them originally). Patch 3 is the
expedient change, but you could argue about whether this is
the best way to tell kernel-doc what to do.

thanks
-- PMM

Peter Maydell (3):
  configure: Honour --disable-werror for Sphinx
  scripts/kernel-doc: Add missing close-paren in c:function directives
  kernel-doc: Use c:struct for Sphinx 3.0 and later

 configure|  9 -
 Makefile |  2 +-
 docs/sphinx/kerneldoc.py |  1 +
 scripts/kernel-doc   | 18 --
 4 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH for-5.0? 2/3] scripts/kernel-doc: Add missing close-paren in c:function directives

[Peter Maydell]

When kernel-doc generates a 'c:function' directive for a function
one of whose arguments is a function pointer, it fails to print
the close-paren after the argument list of the function pointer
argument, for instance:
  .. c:function:: void memory_region_init_resizeable_ram (MemoryRegion * mr, 
struct Object * owner, const char * name, uint64_t size, uint64_t max_size, 
void (*resized) (const char*, uint64_t length, void *host, Error ** errp)

which should have a ')' after the 'void *host' which is the
last argument to 'resized'.

Older versions of Sphinx don't try to parse the argumnet
to c:function, but Sphinx 3.0 does do this and will complain:

  
/home/petmay01/linaro/qemu-from-laptop/qemu/docs/../include/exec/memory.h:834: 
WARNING: Error in declarator or parameters
  Invalid C declaration: Expecting "," or ")" in parameters, got "EOF". [error 
at 208]
void memory_region_init_resizeable_ram (MemoryRegion * mr, struct Object * 
owner, const char * name, uint64_t size, uint64_t max_size, void (*resized) 
(const char*, uint64_t length, void *host, Error ** errp)

^

Add the missing close-paren.

Signed-off-by: Peter Maydell 
---
 scripts/kernel-doc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index af470eb3211..8dc30e01e58 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -853,7 +853,7 @@ sub output_function_rst(%) {
 
if ($type =~ m/([^\(]*\(\*)\s*\)\s*\(([^\)]*)\)/) {
# pointer-to-function
-   print $1 . $parameter . ") (" . $2;
+   print $1 . $parameter . ") (" . $2 . ")";
} else {
print $type . " " . $parameter;
}
-- 
2.20.1

Re: [PATCH] tcg/mips: mips sync* encode error

[Richard Henderson]

On 4/11/20 5:46 AM, lixinyu wrote:
> OPC_SYNC_WMB, OPC_SYNC_MB, OPC_SYNC_ACQUIRE, OPC_SYNC_RELEASE and
> OPC_SYNC_RMB have wrong encode. According to the mips manual,
> their encode should be 'OPC_SYNC | 0x?? << 6' rather than
> 'OPC_SYNC | 0x?? << 5'. Wrong encode can lead illegal instruction
> errors. These instructions often appear with multi-threaded
> simulation.

Good catch.

Reviewed-by: Richard Henderson 

Queued to tcg-for-5.0.


r~

Re: [PATCH-for-5.0 1/2] hw/display/sm501: Avoid heap overflow in sm501_2d_operation()

[BALATON Zoltan]

On Sat, 11 Apr 2020, Philippe Mathieu-Daudé wrote:

Zhang Zi Ming reported a heap overflow in the Drawing Engine of
the SM501 companion chip model, in particular in the COPY_AREA()
macro in sm501_2d_operation().

As I have no idea what this code is supposed to do, add a simple
check to avoid the heap overflow. This fixes:


As the function name says it performs a 2D blitter operation. The code had 
no bounds checking at all and there are easier ways to crash it by writing 
any unimplemented register for which it has abort() calls in the device 
implementation. I'm not sure this patch fixes all possible overflows but 
at least plugs this particular one so why not.


Acked-by: BALATON Zoltan 

Otherwise this device emulation should be rewritten sometimes but I had 
not time for that.


Regards,
BALATON Zoltan


 =
 ==20518==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x7f6f4c3f at pc 0x55b1e1d358f0 bp 0x7ffce464dfb0 sp 0x7ffce464dfa8
 READ of size 1 at 0x7f6f4c3f thread T0
 #0 0x55b1e1d358ef in sm501_2d_operation hw/display/sm501.c:788:13
 #1 0x55b1e1d32c38 in sm501_2d_engine_write hw/display/sm501.c:1466:13
 #2 0x55b1e0cd19d8 in memory_region_write_accessor memory.c:483:5
 #3 0x55b1e0cd1404 in access_with_adjusted_size memory.c:544:18
 #4 0x55b1e0ccfb9d in memory_region_dispatch_write memory.c:1476:16
 #5 0x55b1e0ae55a8 in flatview_write_continue exec.c:3125:23
 #6 0x55b1e0ad3e87 in flatview_write exec.c:3165:14
 #7 0x55b1e0ad3a24 in address_space_write exec.c:3256:18

 0x7f6f4c3f is located 4194303 bytes to the right of 4194304-byte region 
[0x7f6f4bc0,0x7f6f4c00)
 allocated by thread T0 here:
 #0 0x55b1e0a6e715 in __interceptor_posix_memalign 
(ppc64-softmmu/qemu-system-ppc64+0x19c0715)
 #1 0x55b1e31c1482 in qemu_try_memalign util/oslib-posix.c:189:11
 #2 0x55b1e31c168c in qemu_memalign util/oslib-posix.c:205:27
 #3 0x55b1e11a00b3 in spapr_reallocate_hpt hw/ppc/spapr.c:1560:23
 #4 0x55b1e11a0ce4 in spapr_setup_hpt hw/ppc/spapr.c:1593:5
 #5 0x55b1e11c2fba in spapr_machine_reset hw/ppc/spapr.c:1644:9
 #6 0x55b1e1368b01 in qemu_system_reset softmmu/vl.c:1391:9
 #7 0x55b1e1375af3 in qemu_init softmmu/vl.c:4436:5
 #8 0x55b1e2fc8a59 in main softmmu/main.c:48:5
 #9 0x7f6f8150bf42 in __libc_start_main (/lib64/libc.so.6+0x23f42)

 SUMMARY: AddressSanitizer: heap-buffer-overflow hw/display/sm501.c:788:13 in 
sm501_2d_operation
 Shadow bytes around the buggy address:
   0x0fee69877fa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0fee69877fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0fee69877fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0fee69877fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
   0x0fee69877fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
 =>0x0fee69877ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
   0x0fee69878000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   0x0fee69878010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   0x0fee69878020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   0x0fee69878030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
   0x0fee69878040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 Shadow byte legend (one shadow byte represents 8 application bytes):
   Addressable:   00
   Partially addressable: 01 02 03 04 05 06 07
   Heap left redzone:   fa
   Freed heap region:   fd
   Poisoned by user:f7
   ASan internal:   fe
 ==20518==ABORTING

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786026
Reported-by: Zhang Zi Ming <1015138...@qq.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
Per the links on
https://bugzilla.redhat.com/show_bug.cgi?id=1808510 there is probably
a CVE assigned to this, but I don't have access to the information,
https://bugzilla.redhat.com/show_bug.cgi?id=1786593 only show:

 You are not authorized to access bug #1786593.
 Most likely the bug has been restricted for internal development processes and 
we cannot grant access.
---
hw/display/sm501.c | 6 ++
1 file changed, 6 insertions(+)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index de0ab9d977..902acb3875 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -726,6 +726,12 @@ static void sm501_2d_operation(SM501State *s)
int crt = (s->dc_crt_control & SM501_DC_CRT_CONTROL_SEL) ? 1 : 0;
int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt);

+if (rtl && (src_x < operation_width || src_y < operation_height)) {
+qemu_log_mask(LOG_GUEST_ERROR, "sm501: Illegal RTL address (%i, %i)\n",
+  src_x, src_y);
+return;
+}
+
if (addressing != 0x0) {
printf("%s: only XY addressing is supported.\n", __func__);
abort();

Re: [PATCH 0/7] hw/sparc/leon3: Few fixes and disable HelenOS test

[Philippe Mathieu-Daudé]

On 3/31/20 12:50 PM, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (7):
>   hw/misc/grlib_ahb_apb_pnp: Avoid crash when writing to AHB PnP
> registers
>   hw/misc/grlib_ahb_apb_pnp: Fix AHB PnP 8-bit accesses

Ping ^^^ for 5.0?

>   hw/misc/grlib_ahb_apb_pnp: Add trace events on read accesses
>   hw/timer/grlib_gptimer: Display frequency in decimal
>   target/sparc/int32_helper: Remove DEBUG_PCALL definition
>   target/sparc/int32_helper: Extract and use excp_name_str()
> 
>  hw/misc/grlib_ahb_apb_pnp.c | 24 ++--
>  target/sparc/int32_helper.c | 23 ---
>  hw/misc/trace-events|  4 
>  hw/timer/trace-events   |  2 +-
>  tests/acceptance/machine_sparc_leon3.py |  4 
>  5 files changed, 43 insertions(+), 14 deletions(-)
> 

colo: qemu 4.2.0 vs. qemu 5.0.0-rc2 performance regression

[Lukas Straub]

Hello Everyone,
I did some Benchmarking with iperf3 and memtester (to dirty some guest memory)
of colo performance in qemu 4.2.0 and in qemu 5.0.0-rc2
with my bugfixes on top.( 
https://lists.nongnu.org/archive/html/qemu-devel/2020-04/msg01432.html )

I have taken the average over 4 runs.
Client-to-server tcp bandwidth rose slightly from ~83.98 Mbit/s to ~89.40 Mbits.
Server-to-client tcp bandwidth fell from ~9.73 Mbit/s to ~1.79 Mbit/s.
Client-to-server udp bandwidth stayed the same at 1.05 Mbit/s
and jitter rose from ~5.12 ms to ~10.77 ms.
Server-to-client udp bandwidth fell from ~380.5 Kbit/s to ~33.6 Kbit/s
and jitter rose from ~41.74 ms to ~83976.15 ms (!).

I haven't looked closely into it, but i think
0393031a16735835a441b6d6e0495a1bd14adb90 "COLO: Optimize memory back-up process"
is the culprint as it reduces vm downtime for the checkpoints but increases
the overall checkpoint time and we can only release miscompared primary packets
after the checkpoint is completely finished.

Another thing that I noticed: With 4.2.0, the secondary qemu uses thrice
the amount of gest memory. With 5.0.0-rc2 it's just double the amount of
guest memory. So maybe the ram cache isn't working properly?

Regards,
Lukas Straub


pgpk5683oTPDL.pgp
Description: OpenPGP digital signature

Re: [PATCH v1 09/11] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb

[Philippe Mathieu-Daudé]

On 4/10/20 3:08 PM, Stefano Garzarella wrote:

On Thu, Apr 09, 2020 at 10:15:27PM +0100, Alex Bennée wrote:

From: Peter Xu 

We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift.  Without this patch, gdb remote attach will
crash QEMU.

Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Signed-off-by: Peter Xu 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Alex Bennée 
Message-Id: <20200409164954.36902-3-pet...@redhat.com>
---
  target/i386/gdbstub.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614ee..b98a99500ae 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray 
*mem_buf, int n)
  } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
  floatx80 *fp = (floatx80 *) >fpregs[n - IDX_FP_REGS];
  int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
  return len;
  } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
  n -= IDX_XMM_REGS;
--
2.20.1




I had the following issue while attaching to qemu started with gdbserver
listening:



Alex, if possible can you amend this info please?

<---


(gdb) target remote :1234
Remote debugging using :1234
Remote communication error.  Target disconnected.: Connection reset by peer.

$ qemu-system-x86_64 -m 1G -smp 4 ... -s
ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion failed: (len == 
gdbserver_state.mem_buf->len)
Bail out! ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion failed: (len 
== gdbserver_state.mem_buf->len)


--->

Thanks!




Thanks to Philippe, I tried this patch and it solves my issue:

Tested-by: Stefano Garzarella 

Thanks,
Stefano

Re: [PULL 0/8] Misc patches for QEMU 5.0-rc3

[Peter Maydell]

On Sat, 11 Apr 2020 at 14:04, Paolo Bonzini  wrote:
>
> The following changes since commit 53ef8a92eb04ee19640f5aad3bff36cd4a36c250:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20200406' into staging (2020-04-06 
> 12:36:45 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 8c12bd8e32faf8c483cd96c1557899120bc67eea:
>
>   module: increase dirs array size by one (2020-04-11 08:49:26 -0400)
>
> 
> Bugfixes, and reworking of the atomics documentation.
>

>
>  docs/devel/atomics.rst   | 507 
> +++
>  docs/devel/atomics.txt   | 403 
>  docs/devel/index.rst |   1 +
>  docs/devel/rcu.txt   |   4 +-
>  hw/i386/pc_piix.c|  19 +-
>  include/exec/memory.h|   4 +-
>  roms/SLOF|   2 +-

It looks like this roms/SLOF change is bogus (it's in your
"atomics: update documentation" commit) and it makes git
complain about the merge attempt:

Failed to merge submodule roms/SLOF (commits don't follow merge-base)
Auto-merging roms/SLOF
CONFLICT (submodule): Merge conflict in roms/SLOF
Removing docs/devel/atomics.txt
Automatic merge failed; fix conflicts and then commit the result.

thanks
-- PMM

Re: [PULL for-5.0 0/3] Block patches

[Peter Maydell]

On Thu, 9 Apr 2020 at 18:42, Stefan Hajnoczi  wrote:
>
> The following changes since commit 8bac3ba57eecc466b7e73dabf7d19328a59f684e:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-rx-20200408' into 
> staging (2020-04-09 13:23:30 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 5710a3e09f9b85801e5ce70797a4a511e5fc9e2c:
>
>   async: use explicit memory barriers (2020-04-09 16:17:14 +0100)
>
> 
> Pull request
>
> Fixes for QEMU on aarch64 ARM hosts and fdmon-io_uring.
>
> 


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM

Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels

[Nicholas Piggin]

Nicholas Piggin's on April 11, 2020 7:32 pm:
> Nathan Chancellor's on April 11, 2020 10:53 am:
>> The tt.config values are needed to reproduce but I did not verify that
>> ONLY tt.config was needed. Other than that, no, we are just building
>> either pseries_defconfig or powernv_defconfig with those configs and
>> letting it boot up with a simple initramfs, which prints the version
>> string then shuts the machine down.
>> 
>> Let me know if you need any more information, cheers!
> 
> Okay I can reproduce it. Sometimes it eventually recovers after a long
> pause, and some keyboard input often helps it along. So that seems like 
> it might be a lost interrupt.
> 
> POWER8 vs POWER9 might just be a timing thing if P9 is still hanging
> sometimes. I wasn't able to reproduce it with defconfig+tt.config, I
> needed your other config with various other debug options.
> 
> Thanks for the very good report. I'll let you know what I find.

It looks like a qemu bug. Booting with '-d int' shows the decrementer 
simply stops firing at the point of the hang, even though MSR[EE]=1 and 
the DEC register is wrapping. Linux appears to be doing the right thing 
as far as I can tell (not losing interrupts).

This qemu patch fixes the boot hang for me. I don't know that qemu 
really has the right idea of "context synchronizing" as defined in the
powerpc architecture -- mtmsrd L=1 is not context synchronizing but that
does not mean it can avoid looking at exceptions until the next such
event. It looks like the decrementer exception goes high but the
execution of mtmsrd L=1 is ignoring it.

Prior to the Linux patch 3282a3da25b you bisected to, interrupt replay
code would return with an 'rfi' instruction as part of interrupt return,
which probably helped to get things moving along a bit. However it would
not be foolproof, and Cedric did say he encountered some mysterious
lockups under load with qemu powernv before that patch was merged, so
maybe it's the same issue?

Thanks,
Nick

The patch is a bit of a hack, but if you can run it and verify it fixes
your boot hang would be good.
---

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b207fb5386..1d997f5c32 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4364,12 +4364,21 @@ static void gen_mtmsrd(DisasContext *ctx)
 if (ctx->opcode & 0x0001) {
 /* Special form that does not need any synchronisation */
 TCGv t0 = tcg_temp_new();
+TCGv t1 = tcg_temp_new();
 tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
 (1 << MSR_RI) | (1 << MSR_EE));
-tcg_gen_andi_tl(cpu_msr, cpu_msr,
+tcg_gen_andi_tl(t1, cpu_msr,
 ~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
-tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
+tcg_gen_or_tl(t1, t1, t0);
+
+gen_update_nip(ctx, ctx->base.pc_next);
+gen_helper_store_msr(cpu_env, t1);
 tcg_temp_free(t0);
+tcg_temp_free(t1);
+/* Must stop the translation as machine state (may have) changed */
+/* Note that mtmsr is not always defined as context-synchronizing */
+gen_stop_exception(ctx);
+
 } else {
 /*
  * XXX: we need to update nip before the store if we enter

[Bug 1872113] Re: qemu docs fails to build with Sphinx 3.0.x

[Stig-Ørjan Smelror]

You are right. Wrong choice of words.

However, the change is a breaking change from Sphinx.

See https://github.com/sphinx-
doc/sphinx/issues/7457#issuecomment-612413080

** Bug watch added: github.com/sphinx-doc/sphinx/issues #7457
   https://github.com/sphinx-doc/sphinx/issues/7457

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

Title:
  qemu docs fails to build with Sphinx 3.0.x

Status in QEMU:
  New

Bug description:
  We've just updated Sphinx to version 3.0.1 and qemu fails to build the
  docs with this version.

  Here's the relevant section in the build log.

  CONFDIR="/etc/qemu" /usr/bin/sphinx-build-3  -W -b html -D version=4.2.92 -D 
release="4.2.92 (qemu-5.0.0-0.rc2.0.1.mga8)" -d .doctrees/devel-html 
/home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/devel docs/devel
  Running Sphinx v3.0.1
  making output directory... done
  building [mo]: targets for 0 po files that are out of date
  building [html]: targets for 14 source files that are out of date
  updating environment: [new config] 14 added, 0 changed, 0 removed
  reading sources... [  7%] bitops
  reading sources... [ 14%] decodetree
  reading sources... [ 21%] index
  reading sources... [ 28%] kconfig
  reading sources... [ 35%] loads-stores
  reading sources... [ 42%] memory
  reading sources... [ 50%] migration
  reading sources... [ 57%] reset
  reading sources... [ 64%] s390-dasd-ipl
  reading sources... [ 71%] secure-coding-practices
  reading sources... [ 78%] stable-process
  reading sources... [ 85%] tcg
  reading sources... [ 92%] tcg-plugins
  reading sources... [100%] testing

  Warning, treated as error:
  /home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/../include/exec/memory.h:3:Type 
must be either just a name or a typedef-like declaration.
  If just a name:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name, got keyword: 
struct [error at 6]
  struct MemoryListener
  --^
  If typedef-like declaration:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name. [error at 21]
  struct MemoryListener
  -^

  make: *** [Makefile:1095: docs/devel/index.html] Error 2
  make: *** Waiting for unfinished jobs

  I found this commit for memory.h that includes the section that faults.
  
https://github.com/qemu/qemu/commit/5d248213180749e674fbccbacc6ee9c38499abb3#diff-d892cbf314945b44699534cc1de4ebbd

  You can see the whole build log here.
  
https://pkgsubmit.mageia.org/uploads/failure/cauldron/core/release/20200410161120.tv.duvel.699/log/qemu-5.0.0-0.rc2.0.1.mga8/build.0.20200410161338.log

  System: Mageia Cauldron

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

Re: [PATCH v5 2/2] lockable: replaced locks with lock guard macros where appropriate

[Paolo Bonzini]

On 11/04/20 13:19, Daniel Brodsky wrote:
> Just making sure this patch didn't get lost.
> ping http://patchwork.ozlabs.org/patch/1266336/

The patch looks good, but it will be included in QEMU only after 5.0 is
released.

Thanks,

Paolo

[PULL 7/8] memory: Do not allow direct write access to rom_device regions

[Paolo Bonzini]

From: Alexander Duyck 

According to the documentation in memory.h a ROM memory region will be
backed by RAM for reads, but is supposed to go through a callback for
writes. Currently we were not checking for the existence of the rom_device
flag when determining if we could perform a direct write or not.

To correct that add a check to memory_region_is_direct so that if the
memory region has the rom_device flag set we will return false for all
checks where is_write is set.

Signed-off-by: Alexander Duyck 
Message-Id: <20200410034150.24738.98143.stgit@localhost.localdomain>
Signed-off-by: Paolo Bonzini 
---
 include/exec/memory.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1614d9a02c..e000bd2f97 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2351,8 +2351,8 @@ void address_space_write_cached_slow(MemoryRegionCache 
*cache,
 static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
 {
 if (is_write) {
-return memory_region_is_ram(mr) &&
-   !mr->readonly && !memory_region_is_ram_device(mr);
+return memory_region_is_ram(mr) && !mr->readonly &&
+   !mr->rom_device && !memory_region_is_ram_device(mr);
 } else {
 return (memory_region_is_ram(mr) && !memory_region_is_ram_device(mr)) 
||
memory_region_is_romd(mr);
-- 
2.18.2

[PULL 3/8] atomics: convert to reStructuredText

[Paolo Bonzini]

No attempts to fix or update the text; these are left for the next
patch in the series.

Signed-off-by: Paolo Bonzini 
---
 docs/devel/atomics.rst | 446 +
 docs/devel/atomics.txt | 403 -
 docs/devel/index.rst   |   1 +
 3 files changed, 447 insertions(+), 403 deletions(-)
 create mode 100644 docs/devel/atomics.rst
 delete mode 100644 docs/devel/atomics.txt

diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
new file mode 100644
index 00..83ed3d6981
--- /dev/null
+++ b/docs/devel/atomics.rst
@@ -0,0 +1,446 @@
+=
+Atomic operations in QEMU
+=
+
+CPUs perform independent memory operations effectively in random order.
+but this can be a problem for CPU-CPU interaction (including interactions
+between QEMU and the guest).  Multi-threaded programs use various tools
+to instruct the compiler and the CPU to restrict the order to something
+that is consistent with the expectations of the programmer.
+
+The most basic tool is locking.  Mutexes, condition variables and
+semaphores are used in QEMU, and should be the default approach to
+synchronization.  Anything else is considerably harder, but it's
+also justified more often than one would like.  The two tools that
+are provided by ``qemu/atomic.h`` are memory barriers and atomic operations.
+
+Macros defined by ``qemu/atomic.h`` fall in three camps:
+
+- compiler barriers: ``barrier()``;
+
+- weak atomic access and manual memory barriers: ``atomic_read()``,
+  ``atomic_set()``, ``smp_rmb()``, ``smp_wmb()``, ``smp_mb()``, 
``smp_mb_acquire()``,
+  ``smp_mb_release()``, ``smp_read_barrier_depends()``;
+
+- sequentially consistent atomic access: everything else.
+
+
+Compiler memory barrier
+===
+
+``barrier()`` prevents the compiler from moving the memory accesses either
+side of it to the other side.  The compiler barrier has no direct effect
+on the CPU, which may then reorder things however it wishes.
+
+``barrier()`` is mostly used within ``qemu/atomic.h`` itself.  On some
+architectures, CPU guarantees are strong enough that blocking compiler
+optimizations already ensures the correct order of execution.  In this
+case, ``qemu/atomic.h`` will reduce stronger memory barriers to simple
+compiler barriers.
+
+Still, ``barrier()`` can be useful when writing code that can be interrupted
+by signal handlers.
+
+
+Sequentially consistent atomic access
+=
+
+Most of the operations in the ``qemu/atomic.h`` header ensure *sequential
+consistency*, where "the result of any execution is the same as if the
+operations of all the processors were executed in some sequential order,
+and the operations of each individual processor appear in this sequence
+in the order specified by its program".
+
+``qemu/atomic.h`` provides the following set of atomic read-modify-write
+operations::
+
+void atomic_inc(ptr)
+void atomic_dec(ptr)
+void atomic_add(ptr, val)
+void atomic_sub(ptr, val)
+void atomic_and(ptr, val)
+void atomic_or(ptr, val)
+
+typeof(*ptr) atomic_fetch_inc(ptr)
+typeof(*ptr) atomic_fetch_dec(ptr)
+typeof(*ptr) atomic_fetch_add(ptr, val)
+typeof(*ptr) atomic_fetch_sub(ptr, val)
+typeof(*ptr) atomic_fetch_and(ptr, val)
+typeof(*ptr) atomic_fetch_or(ptr, val)
+typeof(*ptr) atomic_fetch_xor(ptr, val)
+typeof(*ptr) atomic_fetch_inc_nonzero(ptr)
+typeof(*ptr) atomic_xchg(ptr, val)
+typeof(*ptr) atomic_cmpxchg(ptr, old, new)
+
+all of which return the old value of ``*ptr``.  These operations are
+polymorphic; they operate on any type that is as wide as a pointer.
+
+Similar operations return the new value of ``*ptr``::
+
+typeof(*ptr) atomic_inc_fetch(ptr)
+typeof(*ptr) atomic_dec_fetch(ptr)
+typeof(*ptr) atomic_add_fetch(ptr, val)
+typeof(*ptr) atomic_sub_fetch(ptr, val)
+typeof(*ptr) atomic_and_fetch(ptr, val)
+typeof(*ptr) atomic_or_fetch(ptr, val)
+typeof(*ptr) atomic_xor_fetch(ptr, val)
+
+Sequentially consistent loads and stores can be done using::
+
+atomic_fetch_add(ptr, 0) for loads
+atomic_xchg(ptr, val) for stores
+
+However, they are quite expensive on some platforms, notably POWER and
+Arm.  Therefore, qemu/atomic.h provides two primitives with slightly
+weaker constraints::
+
+typeof(*ptr) atomic_mb_read(ptr)
+void atomic_mb_set(ptr, val)
+
+The semantics of these primitives map to Java volatile variables,
+and are strongly related to memory barriers as used in the Linux
+kernel (see below).
+
+As long as you use atomic_mb_read and atomic_mb_set, accesses cannot
+be reordered with each other, and it is also not possible to reorder
+"normal" accesses around them.
+
+However, and this is the important difference between
+atomic_mb_read/atomic_mb_set and sequential consistency, it is important
+for both threads to access the same volatile variable.  It is 

[PULL 5/8] rcu: do not mention atomic_mb_read/set in documentation

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 docs/devel/rcu.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/devel/rcu.txt b/docs/devel/rcu.txt
index d83fed2f79..0ce15ba198 100644
--- a/docs/devel/rcu.txt
+++ b/docs/devel/rcu.txt
@@ -132,7 +132,7 @@ The core RCU API is small:
 
  typeof(*p) atomic_rcu_read(p);
 
-atomic_rcu_read() is similar to atomic_mb_read(), but it makes
+atomic_rcu_read() is similar to atomic_load_acquire(), but it makes
 some assumptions on the code that calls it.  This allows a more
 optimized implementation.
 
@@ -154,7 +154,7 @@ The core RCU API is small:
 
  void atomic_rcu_set(p, typeof(*p) v);
 
-atomic_rcu_set() is also similar to atomic_mb_set(), and it also
+atomic_rcu_set() is similar to atomic_store_release(), though it also
 makes assumptions on the code that calls it in order to allow a more
 optimized implementation.
 
-- 
2.18.2

[PULL 2/8] oslib-posix: take lock before qemu_cond_broadcast

[Paolo Bonzini]

From: Bauerchen 

In touch_all_pages, if the mutex is not taken around qemu_cond_broadcast,
qemu_cond_broadcast may be called before all touch page threads enter
qemu_cond_wait. In this case, the touch page threads wait forever for the
main thread to wake them up, causing a deadlock.

Signed-off-by: Bauerchen 
Signed-off-by: Paolo Bonzini 
---
 util/oslib-posix.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 4dd6d7d4b4..062236a1ab 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -492,8 +492,11 @@ static bool touch_all_pages(char *area, size_t hpagesize, 
size_t numpages,
QEMU_THREAD_JOINABLE);
 addr += memset_thread[i].numpages * hpagesize;
 }
+
+qemu_mutex_lock(_mutex);
 threads_created_flag = true;
 qemu_cond_broadcast(_cond);
+qemu_mutex_unlock(_mutex);
 
 for (i = 0; i < memset_num_threads; i++) {
 qemu_thread_join(_thread[i].pgthread);
-- 
2.18.2

[PULL 4/8] atomics: update documentation

[Paolo Bonzini]

Some of the constraints on operand sizes have been relaxed, so adjust the
documentation.

Deprecate atomic_mb_read and atomic_mb_set; it is not really possible to
use them correctly because they do not interoperate with sequentially-consistent
RMW operations.

Finally, extend the memory barrier pairing section to cover acquire and
release semantics in general, roughly based on the KVM Forum 2016 talk,
" weapons".

Signed-off-by: Paolo Bonzini 
---
 docs/devel/atomics.rst | 481 +++--
 roms/SLOF  |   2 +-
 2 files changed, 272 insertions(+), 211 deletions(-)

diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
index 83ed3d6981..445c3b3503 100644
--- a/docs/devel/atomics.rst
+++ b/docs/devel/atomics.rst
@@ -11,10 +11,15 @@ that is consistent with the expectations of the programmer.
 The most basic tool is locking.  Mutexes, condition variables and
 semaphores are used in QEMU, and should be the default approach to
 synchronization.  Anything else is considerably harder, but it's
-also justified more often than one would like.  The two tools that
-are provided by ``qemu/atomic.h`` are memory barriers and atomic operations.
+also justified more often than one would like;
+the most performance-critical parts of QEMU in particular require
+a very low level approach to concurrency, involving memory barriers
+and atomic operations.  The semantics of concurrent memory accesses are 
governed
+by the C11 memory model.
 
-Macros defined by ``qemu/atomic.h`` fall in three camps:
+QEMU provides a header, ``qemu/atomic.h``, which wraps C11 atomics to
+provide better portability and a less verbose syntax.  ``qemu/atomic.h``
+provides macros that fall in three camps:
 
 - compiler barriers: ``barrier()``;
 
@@ -24,13 +29,21 @@ Macros defined by ``qemu/atomic.h`` fall in three camps:
 
 - sequentially consistent atomic access: everything else.
 
+In general, use of ``qemu/atomic.h`` should be wrapped with more easily
+used data structures (e.g. the lock-free singly-linked list operations
+``QSLIST_INSERT_HEAD_ATOMIC`` and ``QSLIST_MOVE_ATOMIC``) or synchronization
+primitives (such as RCU, ``QemuEvent`` or ``QemuLockCnt``).  Bare use of
+atomic operations and memory barriers should be limited to inter-thread
+checking of flags and documented thoroughly.
+
+
 
 Compiler memory barrier
 ===
 
-``barrier()`` prevents the compiler from moving the memory accesses either
-side of it to the other side.  The compiler barrier has no direct effect
-on the CPU, which may then reorder things however it wishes.
+``barrier()`` prevents the compiler from moving the memory accesses on
+either side of it to the other side.  The compiler barrier has no direct
+effect on the CPU, which may then reorder things however it wishes.
 
 ``barrier()`` is mostly used within ``qemu/atomic.h`` itself.  On some
 architectures, CPU guarantees are strong enough that blocking compiler
@@ -73,7 +86,8 @@ operations::
 typeof(*ptr) atomic_cmpxchg(ptr, old, new)
 
 all of which return the old value of ``*ptr``.  These operations are
-polymorphic; they operate on any type that is as wide as a pointer.
+polymorphic; they operate on any type that is as wide as a pointer or
+smaller.
 
 Similar operations return the new value of ``*ptr``::
 
@@ -85,36 +99,28 @@ Similar operations return the new value of ``*ptr``::
 typeof(*ptr) atomic_or_fetch(ptr, val)
 typeof(*ptr) atomic_xor_fetch(ptr, val)
 
-Sequentially consistent loads and stores can be done using::
-
-atomic_fetch_add(ptr, 0) for loads
-atomic_xchg(ptr, val) for stores
-
-However, they are quite expensive on some platforms, notably POWER and
-Arm.  Therefore, qemu/atomic.h provides two primitives with slightly
-weaker constraints::
+``qemu/atomic.h`` also provides loads and stores that cannot be reordered
+with each other::
 
 typeof(*ptr) atomic_mb_read(ptr)
 void atomic_mb_set(ptr, val)
 
-The semantics of these primitives map to Java volatile variables,
-and are strongly related to memory barriers as used in the Linux
-kernel (see below).
+However these do not provide sequential consistency and, in particular,
+they do not participate in the total ordering enforced by
+sequentially-consistent operations.  For this reason they are deprecated.
+They should instead be replaced with any of the following (ordered from
+easiest to hardest):
 
-As long as you use atomic_mb_read and atomic_mb_set, accesses cannot
-be reordered with each other, and it is also not possible to reorder
-"normal" accesses around them.
+- accesses inside a mutex or spinlock
 
-However, and this is the important difference between
-atomic_mb_read/atomic_mb_set and sequential consistency, it is important
-for both threads to access the same volatile variable.  It is not the
-case that everything visible to thread A when it writes volatile field f
-becomes visible to thread B after it reads volatile field g. The store
-and 

[PULL 1/8] piix: fix xenfv regression, add compat machine xenfv-4.2

[Paolo Bonzini]

From: Olaf Hering 

With QEMU 4.0 an incompatible change was added to pc_piix, which makes it
practical impossible to migrate domUs started with qemu2 or qemu3 to
newer qemu versions. Commit 7fccf2a06890e3bc3b30e29827ad3fb93fe88fea
added and enabled a new member "smbus_no_migration_support". In commit
4ab2f2a8aabfea95cc53c64e13b3f67960b27fdf the vmstate_acpi got new
elements, which are conditionally filled. As a result, an incoming
migration expected smbus related data unless smbus migration was
disabled for a given MachineClass. Since first commit forgot to handle
'xenfv', domUs started with QEMU 4.x are incompatible with their QEMU
siblings.

Using other existing machine types, such as 'pc-i440fx-3.1', is not
possible because 'xenfv' creates the 'xen-platform' PCI device at
00:02.0, while all other variants to run a domU would create it at
00:04.0.

To cover both the existing and the broken case of 'xenfv' in a single
qemu binary, a new compatibility variant of 'xenfv-4.2' must be added
which targets domUs started with qemu 4.2. The existing 'xenfv' restores
compatibility of QEMU 5.x with qemu 3.1.

Host admins who started domUs with QEMU 4.x (preferrable QEMU 4.2)
have to use a wrapper script which appends '-machine xenfv-4.2' to
the device-model command line.  This is only required if there is no
maintenance window which allows to temporary shutdown the domU and
restart it with a fixed device-model.

The wrapper script is as simple as this:
  #!/bin/sh
  exec /usr/bin/qemu-system-i386 "$@" -machine xenfv-4.2

With xl this script will be enabled with device_model_override=, see
xl.cfg(5). To live migrate a domU, adjust the existing domU.cfg and pass
it to xl migrate or xl save/restore:
  xl migrate -C new-domU.cfg domU remote-host
  xl save domU CheckpointFile new-domU.cfg
  xl restore new-domU.cfg CheckpointFile

With libvirt this script will be enabled with the  element in
domU.xml. Use 'virsh edit' prior 'virsh migrate' to replace the existing
 element to point it to the wrapper script.

Signed-off-by: Olaf Hering 
Message-Id: <20200327151841.13877-1-o...@aepfle.de>
[Adjust tests for blacklisted machine types, simplifying the one in
 qom-test. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 hw/i386/pc_piix.c| 19 +++--
 tests/qtest/device-introspect-test.c |  2 +-
 tests/qtest/qom-test.c   | 42 ++--
 tests/qtest/test-hmp.c   |  2 +-
 4 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9cceae3e2c..22dee0e76c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -948,13 +948,26 @@ DEFINE_PC_MACHINE(isapc, "isapc", pc_init_isa,
 
 
 #ifdef CONFIG_XEN
-static void xenfv_machine_options(MachineClass *m)
+static void xenfv_4_2_machine_options(MachineClass *m)
 {
+pc_i440fx_4_2_machine_options(m);
+m->desc = "Xen Fully-virtualized PC";
+m->max_cpus = HVM_MAX_VCPUS;
+m->default_machine_opts = "accel=xen";
+}
+
+DEFINE_PC_MACHINE(xenfv_4_2, "xenfv-4.2", pc_xen_hvm_init,
+  xenfv_4_2_machine_options);
+
+static void xenfv_3_1_machine_options(MachineClass *m)
+{
+pc_i440fx_3_1_machine_options(m);
 m->desc = "Xen Fully-virtualized PC";
+m->alias = "xenfv";
 m->max_cpus = HVM_MAX_VCPUS;
 m->default_machine_opts = "accel=xen";
 }
 
-DEFINE_PC_MACHINE(xenfv, "xenfv", pc_xen_hvm_init,
-  xenfv_machine_options);
+DEFINE_PC_MACHINE(xenfv, "xenfv-3.1", pc_xen_hvm_init,
+  xenfv_3_1_machine_options);
 #endif
diff --git a/tests/qtest/device-introspect-test.c 
b/tests/qtest/device-introspect-test.c
index 04f22903b0..f2c1576cae 100644
--- a/tests/qtest/device-introspect-test.c
+++ b/tests/qtest/device-introspect-test.c
@@ -288,7 +288,7 @@ static void add_machine_test_case(const char *mname)
 char *path, *args;
 
 /* Ignore blacklisted machines */
-if (g_str_equal("xenfv", mname) || g_str_equal("xenpv", mname)) {
+if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) {
 return;
 }
 
diff --git a/tests/qtest/qom-test.c b/tests/qtest/qom-test.c
index 4f94cc678c..e338a41194 100644
--- a/tests/qtest/qom-test.c
+++ b/tests/qtest/qom-test.c
@@ -15,35 +15,6 @@
 #include "qemu/cutils.h"
 #include "libqtest.h"
 
-static const char *blacklist_x86[] = {
-"xenfv", "xenpv", NULL
-};
-
-static const struct {
-const char *arch;
-const char **machine;
-} blacklists[] = {
-{ "i386", blacklist_x86 },
-{ "x86_64", blacklist_x86 },
-};
-
-static bool is_blacklisted(const char *arch, const char *mach)
-{
-int i;
-const char **p;
-
-for (i = 0; i < ARRAY_SIZE(blacklists); i++) {
-if (!strcmp(blacklists[i].arch, arch)) {
-for (p = blacklists[i].machine; *p; p++) {
-if (!strcmp(*p, mach)) {
-return true;
-}
-}
-}
-}
-return false;
-}
-
 static void 

[PULL 6/8] vl.c: error out if -mem-path is used together with -M memory-backend

[Paolo Bonzini]

From: Igor Mammedov 

the former is not actually used by explicit backend, so instead of
silently ignoring the option in non valid context, exit with error.

Signed-off-by: Igor Mammedov 
Message-Id: <20200409134133.11339-1-imamm...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 softmmu/vl.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 58a40bcfc1..32c0047889 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -4315,6 +4315,11 @@ void qemu_init(int argc, char **argv, char **envp)
  "explicitly specified 'memory-backend' property");
 exit(EXIT_FAILURE);
 }
+if (mem_path) {
+error_report("'-mem-path' can't be used together with"
+ "'-machine memory-backend'");
+exit(EXIT_FAILURE);
+}
 ram_size = backend_size;
 }
 
-- 
2.18.2

[PULL 0/8] Misc patches for QEMU 5.0-rc3

[Paolo Bonzini]

The following changes since commit 53ef8a92eb04ee19640f5aad3bff36cd4a36c250:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200406' 
into staging (2020-04-06 12:36:45 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 8c12bd8e32faf8c483cd96c1557899120bc67eea:

  module: increase dirs array size by one (2020-04-11 08:49:26 -0400)


Bugfixes, and reworking of the atomics documentation.


Alexander Duyck (1):
  memory: Do not allow direct write access to rom_device regions

Bauerchen (1):
  oslib-posix: take lock before qemu_cond_broadcast

Bruce Rogers (1):
  module: increase dirs array size by one

Igor Mammedov (1):
  vl.c: error out if -mem-path is used together with -M memory-backend

Olaf Hering (1):
  piix: fix xenfv regression, add compat machine xenfv-4.2

Paolo Bonzini (3):
  atomics: convert to reStructuredText
  atomics: update documentation
  rcu: do not mention atomic_mb_read/set in documentation

 docs/devel/atomics.rst   | 507 +++
 docs/devel/atomics.txt   | 403 
 docs/devel/index.rst |   1 +
 docs/devel/rcu.txt   |   4 +-
 hw/i386/pc_piix.c|  19 +-
 include/exec/memory.h|   4 +-
 roms/SLOF|   2 +-
 softmmu/vl.c |   5 +
 tests/qtest/device-introspect-test.c |   2 +-
 tests/qtest/qom-test.c   |  42 +--
 tests/qtest/test-hmp.c   |   2 +-
 util/module.c|   2 +-
 util/oslib-posix.c   |   3 +
 13 files changed, 548 insertions(+), 448 deletions(-)
 create mode 100644 docs/devel/atomics.rst
 delete mode 100644 docs/devel/atomics.txt
-- 
2.18.2

[PULL 8/8] module: increase dirs array size by one

[Paolo Bonzini]

From: Bruce Rogers 

With the module upgrades code change, the statically sized dirs array
can now overflow. Increase it's size by one, according to the new
maximum possible usage.

Fixes: bd83c861c0 ("modules: load modules from versioned /var/run dir")
Signed-off-by: Bruce Rogers 
Message-Id: <20200411010746.472295-1-brog...@suse.com>
Signed-off-by: Paolo Bonzini 
---
 util/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/module.c b/util/module.c
index 5f7896870a..e48d9aacc0 100644
--- a/util/module.c
+++ b/util/module.c
@@ -177,7 +177,7 @@ bool module_load_one(const char *prefix, const char 
*lib_name)
 char *version_dir;
 #endif
 const char *search_dir;
-char *dirs[4];
+char *dirs[5];
 char *module_name;
 int i = 0, n_dirs = 0;
 int ret;
-- 
2.18.2

Re: [PATCH v1 10/11] linux-user: fix /proc/self/stat handling

[Alex Bennée]

Brice Goglin  writes:

> Le 10/04/2020 à 14:33, Alex Bennée a écrit :
>> That was by inspection on my system which seems to truncate a lot
>> earlier. It would be nice to find where in the Linux kernel it is
>> output but I failed to grep the relevant function last night.
>
>
> It's in proc/array.c, do_task_stat() calls proc_task_name(). In the end,
> it seems to use task->tcomm or task->comm which is limited by
>
> #define TASK_COMM_LEN 16

Thanks. I'll amend the commit message. Are you happy with the fix on
your end?

>
> Brice
>
>
>
>>
>> On Fri, 10 Apr 2020, 12:11 Philippe Mathieu-Daudé, > > wrote:
>>
>> Cc'ing Ludovic in case he can test with Guix-HPC.
>>
>> On 4/9/20 11:15 PM, Alex Bennée wrote:
>> > In the original bug report long files names in Guix caused
>> > /proc/self/stat be truncated without the trailing ") " as
>> specified in
>> > proc manpage which says:
>> >  (2) comm  %s
>> > The  filename of the executable, in parentheses.  This
>> > is visible whether or not the  executable  is  swapped
>> > out.
>> >
>> > Additionally it should only be reporting the executable name rather
>> > than the full path. Fix both these failings while cleaning up
>> the code
>> > to use GString to build up the reported values. As the whole
>> function
>> > is cleaned up also adjust the white space to the current coding
>> style.
>> >
>> > Message-ID: > >
>> > Reported-by: Brice Goglin > >
>> > Cc: Philippe_Mathieu-Daudé > >
>> > Signed-off-by: Alex Bennée > >
>> > ---
>> >   linux-user/syscall.c | 43
>> +++
>> >   1 file changed, 19 insertions(+), 24 deletions(-)
>> >
>> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> > index 6495ddc4cda..674f70e70a5 100644
>> > --- a/linux-user/syscall.c
>> > +++ b/linux-user/syscall.c
>> > @@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env,
>> int fd)
>> >   {
>> >   CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
>> >   TaskState *ts = cpu->opaque;
>> > -abi_ulong start_stack = ts->info->start_stack;
>> > +g_autoptr(GString) buf = g_string_new(NULL);
>> >   int i;
>> >   
>> >   for (i = 0; i < 44; i++) {
>> > -  char buf[128];
>> > -  int len;
>> > -  uint64_t val = 0;
>> > -
>> > -  if (i == 0) {
>> > -/* pid */
>> > -val = getpid();
>> > -snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>> > -  } else if (i == 1) {
>> > -/* app name */
>> > -snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>> > -  } else if (i == 27) {
>> > -/* stack bottom */
>> > -val = start_stack;
>> > -snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>> > -  } else {
>> > -/* for the rest, there is MasterCard */
>> > -snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
>> > -  }
>> > +if (i == 0) {
>> > +/* pid */
>> > +g_string_printf(buf, FMT_pid " ", getpid());
>> > +} else if (i == 1) {
>> > +/* app name */
>> > +gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
>> > +bin = bin ? bin + 1 : ts->bprm->argv[0];
>> > +g_string_printf(buf, "(%.15s) ", bin);
>>
>> 15 or 125? 15 seems short. From your previous test I understood it
>> was
>> 124, for
>> 
>> sizeof("cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40").
>>
>> > +} else if (i == 27) {
>> > +/* stack bottom */
>> > +g_string_printf(buf, TARGET_ABI_FMT_ld " ",
>> ts->info->start_stack);
>> > +} else {
>> > +/* for the rest, there is MasterCard */
>> > +g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
>> > +}
>> >   
>> > -  len = strlen(buf);
>> > -  if (write(fd, buf, len) != len) {
>> > -  return -1;
>> > -  }
>> > +if (write(fd, buf->str, buf->len) != buf->len) {
>> > +return -1;
>> > +}
>> >   }
>> >   
>> >   return 0;
>> >
>>


-- 
Alex Bennée

Re: [PATCH v1 09/11] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb

[Alex Bennée]

Stefano Garzarella  writes:

> On Thu, Apr 09, 2020 at 10:15:27PM +0100, Alex Bennée wrote:
>> From: Peter Xu 
>> 
>> We should only pass in gdb_get_reg16() with the GByteArray* object
>> itself, no need to shift.  Without this patch, gdb remote attach will
>> crash QEMU.
>> 
>> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
>> Signed-off-by: Peter Xu 
>> Reviewed-by: Philippe Mathieu-Daudé 
>> Signed-off-by: Alex Bennée 
>> Message-Id: <20200409164954.36902-3-pet...@redhat.com>
>> ---
>>  target/i386/gdbstub.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
>> index f3d23b614ee..b98a99500ae 100644
>> --- a/target/i386/gdbstub.c
>> +++ b/target/i386/gdbstub.c
>> @@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray 
>> *mem_buf, int n)
>>  } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
>>  floatx80 *fp = (floatx80 *) >fpregs[n - IDX_FP_REGS];
>>  int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
>> -len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
>> +len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
>>  return len;
>>  } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
>>  n -= IDX_XMM_REGS;
>> -- 
>> 2.20.1
>> 
>>
>
> I had the following issue while attaching to qemu started with gdbserver
> listening:
>
> (gdb) target remote :1234
> Remote debugging using :1234
> Remote communication error.  Target disconnected.: Connection reset by peer.
>
> $ qemu-system-x86_64 -m 1G -smp 4 ... -s
> ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion failed: (len == 
> gdbserver_state.mem_buf->len)
> Bail out! ERROR:qemu/gdbstub.c:1843:handle_read_all_regs: assertion
> failed: (len == gdbserver_state.mem_buf->len)

I'll see if the new gdb testcases can be generalised - they would have
caught these snafus.

>
>
> Thanks to Philippe, I tried this patch and it solves my issue:
>
> Tested-by: Stefano Garzarella 
>
> Thanks,
> Stefano


-- 
Alex Bennée

Re: [PULL 08/13] softfloat: Fix BAD_SHIFT from normalizeFloatx80Subnormal

[Alex Bennée]

Peter Maydell  writes:

> On Fri, 10 Apr 2020 at 16:17, Richard Henderson
>  wrote:
>> Although why Alex didn't add his own R-b to my patch when merging it to his
>> branch, I don't know.
>
> I think this is one of those areas where different submaintainers
> have different work practices. Personally I distinguish "did I
> actually review this" from "did I just put this into my tree and
> rely on others doing the review" and use r-by for the former
> and not on the latter (although obviously everything I put in
> my tree I will have at least very very briefly looked over).
> But I think some submaintainers don't bother to add r-by tags
> for things they review in the process of assembling their
> tree because they see it as implicit in the process.

It was exactly this - pulling in via my tree and adding my own s-o-b
implies I'm happy enough with it. Typically for longer series that
gestate on the list the total number of r-b tags grows with each re-roll
until the series gets pulled into a maintainer branch. This PR is
atypical in that regard because it's a fairly random collection of fixes.

I think the only patches we should be wary of are those with only a
single s-o-b tag from the author. I have to admit there was one such
patch in this PR:

  Subject: [PULL 09/13] linux-user: factor out reading of /proc/self/maps
  Date: Tue,  7 Apr 2020 16:51:14 +0100
  Message-Id: <20200407155118.20139-10-alex.ben...@linaro.org>

I made an executive decision to include it as it was part of the bug fix
for patch 10 and as we approach RC releases I wanted to get it merged.
If you follow the msg-id in the patch you will see the changes in the
patch are purely in response to review comments so while missing a r-b
tag it's not like it's not been on the list and had some scrutiny.
However we should certainly aim for most patches to be fully reviewed
even if we never achieve that level of perfection.

>
> thanks
> -- PMM


-- 
Alex Bennée

[PATCH] tcg/mips: mips sync* encode error

[lixinyu]

OPC_SYNC_WMB, OPC_SYNC_MB, OPC_SYNC_ACQUIRE, OPC_SYNC_RELEASE and
OPC_SYNC_RMB have wrong encode. According to the mips manual,
their encode should be 'OPC_SYNC | 0x?? << 6' rather than
'OPC_SYNC | 0x?? << 5'. Wrong encode can lead illegal instruction
errors. These instructions often appear with multi-threaded
simulation.

Signed-off-by: lixinyu 
---
 tcg/mips/tcg-target.inc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c
index 1da663ce84..4d32ebc1df 100644
--- a/tcg/mips/tcg-target.inc.c
+++ b/tcg/mips/tcg-target.inc.c
@@ -404,11 +404,11 @@ typedef enum {
 
 /* MIPS r6 introduced names for weaker variants of SYNC.  These are
backward compatible to previous architecture revisions.  */
-OPC_SYNC_WMB = OPC_SYNC | 0x04 << 5,
-OPC_SYNC_MB  = OPC_SYNC | 0x10 << 5,
-OPC_SYNC_ACQUIRE = OPC_SYNC | 0x11 << 5,
-OPC_SYNC_RELEASE = OPC_SYNC | 0x12 << 5,
-OPC_SYNC_RMB = OPC_SYNC | 0x13 << 5,
+OPC_SYNC_WMB = OPC_SYNC | 0x04 << 6,
+OPC_SYNC_MB  = OPC_SYNC | 0x10 << 6,
+OPC_SYNC_ACQUIRE = OPC_SYNC | 0x11 << 6,
+OPC_SYNC_RELEASE = OPC_SYNC | 0x12 << 6,
+OPC_SYNC_RMB = OPC_SYNC | 0x13 << 6,
 
 /* Aliases for convenience.  */
 ALIAS_PADD = sizeof(void *) == 4 ? OPC_ADDU : OPC_DADDU,
-- 
2.20.1

[Bug 1872113] Re: qemu docs fails to build with Sphinx 3.0.x

[Peter Maydell]

I'm a bit confused: you say "however there are still errors" but the
build log you quote ends with "build succeeded, 4 warnings" and it looks
like it has indeed just produced warnings and continued.

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

Title:
  qemu docs fails to build with Sphinx 3.0.x

Status in QEMU:
  New

Bug description:
  We've just updated Sphinx to version 3.0.1 and qemu fails to build the
  docs with this version.

  Here's the relevant section in the build log.

  CONFDIR="/etc/qemu" /usr/bin/sphinx-build-3  -W -b html -D version=4.2.92 -D 
release="4.2.92 (qemu-5.0.0-0.rc2.0.1.mga8)" -d .doctrees/devel-html 
/home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/devel docs/devel
  Running Sphinx v3.0.1
  making output directory... done
  building [mo]: targets for 0 po files that are out of date
  building [html]: targets for 14 source files that are out of date
  updating environment: [new config] 14 added, 0 changed, 0 removed
  reading sources... [  7%] bitops
  reading sources... [ 14%] decodetree
  reading sources... [ 21%] index
  reading sources... [ 28%] kconfig
  reading sources... [ 35%] loads-stores
  reading sources... [ 42%] memory
  reading sources... [ 50%] migration
  reading sources... [ 57%] reset
  reading sources... [ 64%] s390-dasd-ipl
  reading sources... [ 71%] secure-coding-practices
  reading sources... [ 78%] stable-process
  reading sources... [ 85%] tcg
  reading sources... [ 92%] tcg-plugins
  reading sources... [100%] testing

  Warning, treated as error:
  /home/iurt/rpmbuild/BUILD/qemu-5.0.0-rc2/docs/../include/exec/memory.h:3:Type 
must be either just a name or a typedef-like declaration.
  If just a name:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name, got keyword: 
struct [error at 6]
  struct MemoryListener
  --^
  If typedef-like declaration:
    Error in declarator or parameters
    Invalid C declaration: Expected identifier in nested name. [error at 21]
  struct MemoryListener
  -^

  make: *** [Makefile:1095: docs/devel/index.html] Error 2
  make: *** Waiting for unfinished jobs

  I found this commit for memory.h that includes the section that faults.
  
https://github.com/qemu/qemu/commit/5d248213180749e674fbccbacc6ee9c38499abb3#diff-d892cbf314945b44699534cc1de4ebbd

  You can see the whole build log here.
  
https://pkgsubmit.mageia.org/uploads/failure/cauldron/core/release/20200410161120.tv.duvel.699/log/qemu-5.0.0-0.rc2.0.1.mga8/build.0.20200410161338.log

  System: Mageia Cauldron

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

[Bug 1863441] Re: cmd_mode_sense always reports 0x70, no CDROM present

[Thomas Huth]

** Changed in: qemu
   Status: New => Won't Fix

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

Title:
  cmd_mode_sense always reports 0x70, no CDROM present

Status in QEMU:
  Won't Fix

Bug description:
  cmd_mode_sense

https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ide/atapi.c;hb=refs/heads/master#l852
  always reports 0x70 in byte 2 returned, indicating no CD-ROM present.

  If CD-ROM is present, should report 0x01 (or 0x11).
  If CD-ROM absent, should report 0x70.

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

Re: [PATCH v5 2/2] lockable: replaced locks with lock guard macros where appropriate

[Daniel Brodsky]

On Fri, Apr 3, 2020 at 9:21 PM  wrote:
>
> From: Daniel Brodsky 
>
> - ran regexp "qemu_mutex_lock\(.*\).*\n.*if" to find targets
> - replaced result with QEMU_LOCK_GUARD if all unlocks at function end
> - replaced result with WITH_QEMU_LOCK_GUARD if unlock not at end
>
> Signed-off-by: Daniel Brodsky 
> ---
>  block/iscsi.c |  7 ++
>  block/nfs.c   | 51 ---
>  cpus-common.c | 14 +---
>  hw/display/qxl.c  | 43 +---
>  hw/vfio/platform.c|  5 ++---
>  migration/migration.c |  3 +--
>  migration/multifd.c   |  8 +++
>  migration/ram.c   |  3 +--
>  monitor/misc.c|  4 +---
>  ui/spice-display.c| 14 ++--
>  util/log.c|  4 ++--
>  util/qemu-timer.c | 17 +++
>  util/rcu.c|  8 +++
>  util/thread-pool.c|  3 +--
>  util/vfio-helpers.c   |  5 ++---

Just making sure this patch didn't get lost.
ping http://patchwork.ozlabs.org/patch/1266336/

[PATCH RESEND v3 2/2] Makefile: remove old compatibility gunks

[Claudio Fontana]

Signed-off-by: Claudio Fontana 
---
 Makefile | 6 --
 1 file changed, 6 deletions(-)

diff --git a/Makefile b/Makefile
index 7be15eeb7c..00377f28b9 100644
--- a/Makefile
+++ b/Makefile
@@ -567,12 +567,6 @@ slirp/all: .git-submodule-status
CC="$(CC)" AR="$(AR)"   LD="$(LD)" RANLIB="$(RANLIB)"   \
CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)")
 
-# Compatibility gunk to keep make working across the rename of targets
-# for recursion, to be removed some time after 4.1.
-subdir-dtc: dtc/all
-subdir-capstone: capstone/all
-subdir-slirp: slirp/all
-
 $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
$(qom-obj-y)
 
-- 
2.16.4

[PATCH RESEND v3 0/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana]

v2 -> v3:

* changed into a 2 patch series; in the second patch we remove the old
  compatibility gunks that were meant for removal some time after 4.1.

* renamed the libfdt PHONY rule to dtc/all, with the intent to make
  existing working trees forward and backward compatible across the change.

v1 -> v2:

* fix error generated when running UNCHECKED_GOALS without prior configure,
  for example during make docker-image-fedora. Without configure, DSOSUF is
  empty, and the module pattern rule in rules.mak that uses this variable
  can match too much; provide a default in the Makefile to avoid it.

* only attempt to build the archive when there is a non-empty list of objects.
  This could be done in general for the %.a: pattern in rules.mak, but maybe
  there are valid reasons to build an empty .a?

* removed some intermediate variables that did not add much value
  (LIBFDT_srcdir, LIBFDT_archive)

Tested locally with 3 VPATH configurations (no-, VPATH, VPATH in src subdir),
and with docker-image-fedora, docker-test-debug@fedora that failed before.

Claudio Fontana (2):
  Makefile: libfdt: build only the strict necessary
  Makefile: remove old compatibility gunks

 Makefile  | 32 
 configure |  6 +-
 rules.mak |  2 ++
 3 files changed, 19 insertions(+), 21 deletions(-)

-- 
2.16.4

[PATCH RESEND v3 1/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana]

when building dtc/libfdt, we were previously using dtc/Makefile,
which tries to build some artifacts that are not needed,
and can complain on stderr about the absence of tools that
are not required to build just libfdt.

Instead, build only the strict necessary to get libfdt.a .

Signed-off-by: Claudio Fontana 
---
 Makefile  | 21 +
 configure |  4 
 rules.mak |  2 ++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 84ef881600..7be15eeb7c 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
   $(error main directory cannot contain spaces nor colons)
 endif
 
+# some pattern rules in rules.mak are confused by an empty DSOSUF,
+# and UNCHECKED_GOALS for testing (docker-) can run without prior configure.
+DSOSUF ?= ".so"
+
 # Always point to the root of the build tree (needs GNU make).
 BUILD_DIR=$(CURDIR)
 
@@ -526,15 +530,17 @@ $(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
 $(TARGET_DIRS_RULES):
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-DTC_MAKE_ARGS=-I$(SRC_PATH)/dtc VPATH=$(SRC_PATH)/dtc -C dtc V="$(V)" 
LIBFDT_srcdir=$(SRC_PATH)/dtc/libfdt
-DTC_CFLAGS=$(CFLAGS) $(QEMU_CFLAGS)
-DTC_CPPFLAGS=-I$(BUILD_DIR)/dtc -I$(SRC_PATH)/dtc -I$(SRC_PATH)/dtc/libfdt
-
+LIBFDT_objdir = dtc/libfdt
+-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
+LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
+# the name dtc/all is for backward compatibility
 .PHONY: dtc/all
-dtc/all: .git-submodule-status dtc/libfdt dtc/tests
-   $(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) 
CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)" 
ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) 
libfdt/libfdt.a,)
+dtc/all: .git-submodule-status $(LIBFDT_objdir)/libfdt.a
+$(LIBFDT_objdir)/libfdt.a: $(LIBFDT_objects)
+   $(if $(LIBFDT_objects),$(call quiet-command,rm -f $@ && $(AR) rcs $@ 
$^,"AR","$(TARGET_DIR)$@"),)
 
-dtc/%: .git-submodule-status
+$(LIBFDT_objects): | $(LIBFDT_objdir)
+$(LIBFDT_objdir): .git-submodule-status
@mkdir -p $@
 
 # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
@@ -821,7 +827,6 @@ distclean: clean
rm -rf $$d || exit 1 ; \
 done
rm -Rf .sdk
-   if test -f dtc/version_gen.h; then $(MAKE) $(DTC_MAKE_ARGS) clean; fi
 
 KEYMAPS=da en-gb  et  fr fr-ch  is  lt  no  pt-br  sv \
 ar  de en-us  fi  fr-be  hr it  lv  nl pl  ru th \
diff --git a/configure b/configure
index 233c671aaa..cf32bfb75b 100755
--- a/configure
+++ b/configure
@@ -4278,10 +4278,6 @@ EOF
   if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" 
; then
   fdt=git
   mkdir -p dtc
-  if [ "$pwd_is_source_path" != "y" ] ; then
-  symlink "$source_path/dtc/Makefile" "dtc/Makefile"
-  symlink "$source_path/dtc/scripts" "dtc/scripts"
-  fi
   fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
   fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
   fdt_libs="$fdt_libs"
diff --git a/rules.mak b/rules.mak
index 694865b63e..61eb474ba4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -105,6 +105,8 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) 
$(QEMU_LDFLAGS) -o $@ \
 
 DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
 module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
+
+# Note: DSOSUF must not be empty, or these rules will try to match too much
 %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
 %$(DSOSUF): %.mo
$(call LINK,$^)
-- 
2.16.4

[PATCH 1/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana]

when building dtc/libfdt, we were previously using dtc/Makefile,
which tries to build some artifacts that are not needed,
and can complain on stderr about the absence of tools that
are not required to build just libfdt.

Instead, build only the strict necessary to get libfdt.a .

Signed-off-by: Claudio Fontana 
---
 Makefile  | 21 +
 configure |  4 
 rules.mak |  2 ++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 84ef881600..7be15eeb7c 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
   $(error main directory cannot contain spaces nor colons)
 endif
 
+# some pattern rules in rules.mak are confused by an empty DSOSUF,
+# and UNCHECKED_GOALS for testing (docker-) can run without prior configure.
+DSOSUF ?= ".so"
+
 # Always point to the root of the build tree (needs GNU make).
 BUILD_DIR=$(CURDIR)
 
@@ -526,15 +530,17 @@ $(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
 $(TARGET_DIRS_RULES):
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-DTC_MAKE_ARGS=-I$(SRC_PATH)/dtc VPATH=$(SRC_PATH)/dtc -C dtc V="$(V)" 
LIBFDT_srcdir=$(SRC_PATH)/dtc/libfdt
-DTC_CFLAGS=$(CFLAGS) $(QEMU_CFLAGS)
-DTC_CPPFLAGS=-I$(BUILD_DIR)/dtc -I$(SRC_PATH)/dtc -I$(SRC_PATH)/dtc/libfdt
-
+LIBFDT_objdir = dtc/libfdt
+-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
+LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
+# the name dtc/all is for backward compatibility
 .PHONY: dtc/all
-dtc/all: .git-submodule-status dtc/libfdt dtc/tests
-   $(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) 
CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)" 
ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) 
libfdt/libfdt.a,)
+dtc/all: .git-submodule-status $(LIBFDT_objdir)/libfdt.a
+$(LIBFDT_objdir)/libfdt.a: $(LIBFDT_objects)
+   $(if $(LIBFDT_objects),$(call quiet-command,rm -f $@ && $(AR) rcs $@ 
$^,"AR","$(TARGET_DIR)$@"),)
 
-dtc/%: .git-submodule-status
+$(LIBFDT_objects): | $(LIBFDT_objdir)
+$(LIBFDT_objdir): .git-submodule-status
@mkdir -p $@
 
 # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
@@ -821,7 +827,6 @@ distclean: clean
rm -rf $$d || exit 1 ; \
 done
rm -Rf .sdk
-   if test -f dtc/version_gen.h; then $(MAKE) $(DTC_MAKE_ARGS) clean; fi
 
 KEYMAPS=da en-gb  et  fr fr-ch  is  lt  no  pt-br  sv \
 ar  de en-us  fi  fr-be  hr it  lv  nl pl  ru th \
diff --git a/configure b/configure
index 233c671aaa..cf32bfb75b 100755
--- a/configure
+++ b/configure
@@ -4278,10 +4278,6 @@ EOF
   if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" 
; then
   fdt=git
   mkdir -p dtc
-  if [ "$pwd_is_source_path" != "y" ] ; then
-  symlink "$source_path/dtc/Makefile" "dtc/Makefile"
-  symlink "$source_path/dtc/scripts" "dtc/scripts"
-  fi
   fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
   fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
   fdt_libs="$fdt_libs"
diff --git a/rules.mak b/rules.mak
index 694865b63e..61eb474ba4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -105,6 +105,8 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) 
$(QEMU_LDFLAGS) -o $@ \
 
 DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
 module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
+
+# Note: DSOSUF must not be empty, or these rules will try to match too much
 %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
 %$(DSOSUF): %.mo
$(call LINK,$^)
-- 
2.16.4

[PATCH 2/2] Makefile: remove old compatibility gunks

[Claudio Fontana]

Signed-off-by: Claudio Fontana 
---
 Makefile | 6 --
 1 file changed, 6 deletions(-)

diff --git a/Makefile b/Makefile
index 7be15eeb7c..00377f28b9 100644
--- a/Makefile
+++ b/Makefile
@@ -567,12 +567,6 @@ slirp/all: .git-submodule-status
CC="$(CC)" AR="$(AR)"   LD="$(LD)" RANLIB="$(RANLIB)"   \
CFLAGS="$(QEMU_CFLAGS) $(CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)")
 
-# Compatibility gunk to keep make working across the rename of targets
-# for recursion, to be removed some time after 4.1.
-subdir-dtc: dtc/all
-subdir-capstone: capstone/all
-subdir-slirp: slirp/all
-
 $(filter %/all, $(TARGET_DIRS_RULES)): libqemuutil.a $(common-obj-y) \
$(qom-obj-y)
 
-- 
2.16.4

[PATCH v3 0/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana]

v2 -> v3:

* changed into a 2 patch series; in the second patch we remove the old
  compatibility gunks that were meant for removal some time after 4.1.

* renamed the libfdt PHONY rule to dtc/all, with the intent to make
  existing working trees forward and backward compatible across the change.

v1 -> v2:

* fix error generated when running UNCHECKED_GOALS without prior configure,
  for example during make docker-image-fedora. Without configure, DSOSUF is
  empty, and the module pattern rule in rules.mak that uses this variable
  can match too much; provide a default in the Makefile to avoid it.

* only attempt to build the archive when there is a non-empty list of objects.
  This could be done in general for the %.a: pattern in rules.mak, but maybe
  there are valid reasons to build an empty .a?

* removed some intermediate variables that did not add much value
  (LIBFDT_srcdir, LIBFDT_archive)

Tested locally with 3 VPATH configurations (no-, VPATH, VPATH in src subdir),
and with docker-image-fedora, docker-test-debug@fedora that failed before.

Claudio Fontana (2):
  Makefile: libfdt: build only the strict necessary
  Makefile: remove old compatibility gunks

 Makefile  | 32 
 configure |  6 +-
 rules.mak |  2 ++
 3 files changed, 19 insertions(+), 21 deletions(-)

-- 
2.16.4

[PATCH 1/2] Makefile: libfdt: build only the strict necessary

[Claudio Fontana]

when building dtc/libfdt, we were previously using dtc/Makefile,
which tries to build some artifacts that are not needed,
and can complain on stderr about the absence of tools that
are not required to build just libfdt.

Instead, build only the strict necessary to get libfdt.a .

Signed-off-by: Claudio Fontana 
---
 Makefile  | 21 +
 configure |  4 
 rules.mak |  2 ++
 3 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/Makefile b/Makefile
index 84ef881600..7be15eeb7c 100644
--- a/Makefile
+++ b/Makefile
@@ -4,6 +4,10 @@ ifneq ($(words $(subst :, ,$(CURDIR))), 1)
   $(error main directory cannot contain spaces nor colons)
 endif
 
+# some pattern rules in rules.mak are confused by an empty DSOSUF,
+# and UNCHECKED_GOALS for testing (docker-) can run without prior configure.
+DSOSUF ?= ".so"
+
 # Always point to the root of the build tree (needs GNU make).
 BUILD_DIR=$(CURDIR)
 
@@ -526,15 +530,17 @@ $(SOFTMMU_FUZZ_RULES): $(edk2-decompressed)
 $(TARGET_DIRS_RULES):
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" 
TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-DTC_MAKE_ARGS=-I$(SRC_PATH)/dtc VPATH=$(SRC_PATH)/dtc -C dtc V="$(V)" 
LIBFDT_srcdir=$(SRC_PATH)/dtc/libfdt
-DTC_CFLAGS=$(CFLAGS) $(QEMU_CFLAGS)
-DTC_CPPFLAGS=-I$(BUILD_DIR)/dtc -I$(SRC_PATH)/dtc -I$(SRC_PATH)/dtc/libfdt
-
+LIBFDT_objdir = dtc/libfdt
+-include $(SRC_PATH)/dtc/libfdt/Makefile.libfdt
+LIBFDT_objects = $(addprefix $(LIBFDT_objdir)/, $(LIBFDT_OBJS))
+# the name dtc/all is for backward compatibility
 .PHONY: dtc/all
-dtc/all: .git-submodule-status dtc/libfdt dtc/tests
-   $(call quiet-command,$(MAKE) $(DTC_MAKE_ARGS) 
CPPFLAGS="$(DTC_CPPFLAGS)" CFLAGS="$(DTC_CFLAGS)" LDFLAGS="$(QEMU_LDFLAGS)" 
ARFLAGS="$(ARFLAGS)" CC="$(CC)" AR="$(AR)" LD="$(LD)" $(SUBDIR_MAKEFLAGS) 
libfdt/libfdt.a,)
+dtc/all: .git-submodule-status $(LIBFDT_objdir)/libfdt.a
+$(LIBFDT_objdir)/libfdt.a: $(LIBFDT_objects)
+   $(if $(LIBFDT_objects),$(call quiet-command,rm -f $@ && $(AR) rcs $@ 
$^,"AR","$(TARGET_DIR)$@"),)
 
-dtc/%: .git-submodule-status
+$(LIBFDT_objects): | $(LIBFDT_objdir)
+$(LIBFDT_objdir): .git-submodule-status
@mkdir -p $@
 
 # Overriding CFLAGS causes us to lose defines added in the sub-makefile.
@@ -821,7 +827,6 @@ distclean: clean
rm -rf $$d || exit 1 ; \
 done
rm -Rf .sdk
-   if test -f dtc/version_gen.h; then $(MAKE) $(DTC_MAKE_ARGS) clean; fi
 
 KEYMAPS=da en-gb  et  fr fr-ch  is  lt  no  pt-br  sv \
 ar  de en-us  fi  fr-be  hr it  lv  nl pl  ru th \
diff --git a/configure b/configure
index 233c671aaa..cf32bfb75b 100755
--- a/configure
+++ b/configure
@@ -4278,10 +4278,6 @@ EOF
   if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" 
; then
   fdt=git
   mkdir -p dtc
-  if [ "$pwd_is_source_path" != "y" ] ; then
-  symlink "$source_path/dtc/Makefile" "dtc/Makefile"
-  symlink "$source_path/dtc/scripts" "dtc/scripts"
-  fi
   fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt"
   fdt_ldflags="-L\$(BUILD_DIR)/dtc/libfdt"
   fdt_libs="$fdt_libs"
diff --git a/rules.mak b/rules.mak
index 694865b63e..61eb474ba4 100644
--- a/rules.mak
+++ b/rules.mak
@@ -105,6 +105,8 @@ LINK = $(call quiet-command, $(LINKPROG) $(CFLAGS) 
$(QEMU_LDFLAGS) -o $@ \
 
 DSO_OBJ_CFLAGS := -fPIC -DBUILD_DSO
 module-common.o: CFLAGS += $(DSO_OBJ_CFLAGS)
+
+# Note: DSOSUF must not be empty, or these rules will try to match too much
 %$(DSOSUF): QEMU_LDFLAGS += $(LDFLAGS_SHARED)
 %$(DSOSUF): %.mo
$(call LINK,$^)
-- 
2.16.4

[PATCH-for-5.0 2/2] qtest: Test the Drawing Engine of the SM501 companion

[Philippe Mathieu-Daudé]

Run some PCI commands to call the COPY_AREA() macro in
sm501_2d_operation(), and verify that there is no more
overflow as reported in BZ#1786026 [*].

The SM501 is used by the R2D-PLUS and aCube Sam460ex
machines, but since it is a PCI card and we already have
an easy way to test PCI daughter cards on the sPAPR
machine, test it there.

[*] https://bugzilla.redhat.com/show_bug.cgi?id=1786026
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/qtest/sm501-test.c | 106 +++
 tests/qtest/Makefile.include |   2 +
 2 files changed, 108 insertions(+)
 create mode 100644 tests/qtest/sm501-test.c

diff --git a/tests/qtest/sm501-test.c b/tests/qtest/sm501-test.c
new file mode 100644
index 00..79bf7c2c21
--- /dev/null
+++ b/tests/qtest/sm501-test.c
@@ -0,0 +1,106 @@
+/*
+ * QEMU test for the SM501 companion
+ *
+ * SPDX-FileCopyrightText: 2020 Philippe Mathieu-Daudé 
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu-common.h"
+#include "libqtest.h"
+#include "libqos/libqos-spapr.h"
+#include "hw/pci/pci_ids.h"
+
+typedef struct {
+QOSState *qs;
+QPCIDevice *dev;
+QPCIBar bar;
+} PciSm501State;
+
+static void save_fn(QPCIDevice *dev, int devfn, void *data)
+{
+*((QPCIDevice **)data) = dev;
+}
+
+static void sm501_init(PciSm501State *s)
+{
+uint64_t barsize;
+
+s->dev = NULL;
+qpci_device_foreach(s->qs->pcibus, PCI_VENDOR_ID_SILICON_MOTION,
+PCI_DEVICE_ID_SM501, save_fn, >dev);
+g_assert(s->dev != NULL);
+
+qpci_device_enable(s->dev);
+
+/* BAR#0: VRAM, BAR#1: MMIO registers */
+s->bar = qpci_iomap(s->dev, 1, );
+g_assert_cmpuint(barsize, ==, 2 * MiB);
+}
+
+static void sm501_deinit(PciSm501State *s)
+{
+g_free(s->dev);
+}
+
+static uint32_t sm501_read(PciSm501State *s, uint64_t off)
+{
+uint32_t val;
+
+s->dev->bus->memread(s->dev->bus, s->bar.addr + off, , sizeof(val));
+
+return val;
+}
+
+static void sm501_write(PciSm501State *s, uint64_t off, uint32_t val)
+{
+s->dev->bus->memwrite(s->dev->bus, s->bar.addr + off, , sizeof(val));
+}
+
+static void sm501_check_device_id(PciSm501State *s)
+{
+g_assert_cmphex(sm501_read(s, 0x60) >> 16, ==, 0x501); /* DEVICEID reg */
+}
+
+/*
+ * Try to reproduce the heap overflow reported in
+ * https://bugzilla.redhat.com/show_bug.cgi?id=1786026
+ */
+static void test_sm501_2d_drawing_engine_op(void)
+{
+PciSm501State s;
+
+s.qs = qtest_spapr_boot("-machine pseries -device sm501");
+
+sm501_init();
+sm501_check_device_id();
+
+/*
+ * Commands listed in BZ#1786026 to access
+ * COPY_AREA() in sm501_2d_operation().
+ */
+sm501_write(, 0x10,0x0);  /* src: (x, y) = (0, 0) */
+sm501_write(, 0x14,0x0);  /* dst: (x, y) = (0, 0) */
+sm501_write(, 0x18, 0x00100010);  /* dim: height = width = 16 */
+sm501_write(, 0x100010, 0x00100010);  /* pitch: height = width = 16 */
+sm501_write(, 0x1c, 0xcc88);  /* ctrl: op = copy area, RTL */
+
+/* If the overflow occured, the next call will fail. */
+sm501_check_device_id();
+
+sm501_deinit();
+
+qtest_shutdown(s.qs);
+}
+
+int main(int argc, char *argv[])
+{
+g_test_init(, , NULL);
+
+if (!strcmp(qtest_get_arch(), "ppc64")) {
+qtest_add_func("spapr/sm501_2d_op", test_sm501_2d_drawing_engine_op);
+}
+
+return g_test_run();
+}
diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 9e5a51d033..7ec894a7a9 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -109,6 +109,7 @@ check-qtest-ppc64-$(CONFIG_VGA) += display-vga-test
 check-qtest-ppc64-y += numa-test
 check-qtest-ppc64-$(CONFIG_IVSHMEM_DEVICE) += ivshmem-test
 check-qtest-ppc64-y += cpu-plug-test
+check-qtest-ppc64-$(CONFIG_SM501) += sm501-test
 
 check-qtest-sh4-$(CONFIG_ISA_TESTDEV) = endianness-test
 
@@ -253,6 +254,7 @@ tests/qtest/pflash-cfi02$(EXESUF): 
tests/qtest/pflash-cfi02-test.o
 tests/qtest/endianness-test$(EXESUF): tests/qtest/endianness-test.o
 tests/qtest/prom-env-test$(EXESUF): tests/qtest/prom-env-test.o $(libqos-obj-y)
 tests/qtest/rtas-test$(EXESUF): tests/qtest/rtas-test.o $(libqos-spapr-obj-y)
+tests/qtest/sm501-test$(EXESUF): tests/qtest/sm501-test.o $(libqos-spapr-obj-y)
 tests/qtest/fdc-test$(EXESUF): tests/qtest/fdc-test.o
 tests/qtest/ide-test$(EXESUF): tests/qtest/ide-test.o $(libqos-pc-obj-y)
 tests/qtest/ahci-test$(EXESUF): tests/qtest/ahci-test.o $(libqos-pc-obj-y) 
qemu-img$(EXESUF)
-- 
2.21.1

[PATCH-for-5.0 0/2] hw/display/sm501: Avoid heap overflow in sm501_2d_operation()

[Philippe Mathieu-Daudé]

I once setup a Bugzilla 'Component Watching' rule on 'QEMU + CVE',
and recently found a notification for BZ#1786026 about a heap
overflow in sm501_2d_operation():
https://bugzilla.redhat.com/show_bug.cgi?id=1786026
As this is from december I suppose there was some embargo that
recently expired. Apparently there is a CVE assigned but the
information about it is private.
I'm not sure the upstream community is already aware of this
problem, but since we are in hard freeze and the bug can easily
be avoided, I believe a 3-lines patch is appropriate.

Philippe Mathieu-Daudé (2):
  hw/display/sm501: Avoid heap overflow in sm501_2d_operation()
  qtest: Test the Drawing Engine of the SM501 companion

 hw/display/sm501.c   |   6 ++
 tests/qtest/sm501-test.c | 106 +++
 tests/qtest/Makefile.include |   2 +
 3 files changed, 114 insertions(+)
 create mode 100644 tests/qtest/sm501-test.c

-- 
2.21.1

[PATCH-for-5.0 1/2] hw/display/sm501: Avoid heap overflow in sm501_2d_operation()

[Philippe Mathieu-Daudé]

Zhang Zi Ming reported a heap overflow in the Drawing Engine of
the SM501 companion chip model, in particular in the COPY_AREA()
macro in sm501_2d_operation().

As I have no idea what this code is supposed to do, add a simple
check to avoid the heap overflow. This fixes:

  =
  ==20518==ERROR: AddressSanitizer: heap-buffer-overflow on address 
0x7f6f4c3f at pc 0x55b1e1d358f0 bp 0x7ffce464dfb0 sp 0x7ffce464dfa8
  READ of size 1 at 0x7f6f4c3f thread T0
  #0 0x55b1e1d358ef in sm501_2d_operation hw/display/sm501.c:788:13
  #1 0x55b1e1d32c38 in sm501_2d_engine_write hw/display/sm501.c:1466:13
  #2 0x55b1e0cd19d8 in memory_region_write_accessor memory.c:483:5
  #3 0x55b1e0cd1404 in access_with_adjusted_size memory.c:544:18
  #4 0x55b1e0ccfb9d in memory_region_dispatch_write memory.c:1476:16
  #5 0x55b1e0ae55a8 in flatview_write_continue exec.c:3125:23
  #6 0x55b1e0ad3e87 in flatview_write exec.c:3165:14
  #7 0x55b1e0ad3a24 in address_space_write exec.c:3256:18

  0x7f6f4c3f is located 4194303 bytes to the right of 4194304-byte region 
[0x7f6f4bc0,0x7f6f4c00)
  allocated by thread T0 here:
  #0 0x55b1e0a6e715 in __interceptor_posix_memalign 
(ppc64-softmmu/qemu-system-ppc64+0x19c0715)
  #1 0x55b1e31c1482 in qemu_try_memalign util/oslib-posix.c:189:11
  #2 0x55b1e31c168c in qemu_memalign util/oslib-posix.c:205:27
  #3 0x55b1e11a00b3 in spapr_reallocate_hpt hw/ppc/spapr.c:1560:23
  #4 0x55b1e11a0ce4 in spapr_setup_hpt hw/ppc/spapr.c:1593:5
  #5 0x55b1e11c2fba in spapr_machine_reset hw/ppc/spapr.c:1644:9
  #6 0x55b1e1368b01 in qemu_system_reset softmmu/vl.c:1391:9
  #7 0x55b1e1375af3 in qemu_init softmmu/vl.c:4436:5
  #8 0x55b1e2fc8a59 in main softmmu/main.c:48:5
  #9 0x7f6f8150bf42 in __libc_start_main (/lib64/libc.so.6+0x23f42)

  SUMMARY: AddressSanitizer: heap-buffer-overflow hw/display/sm501.c:788:13 in 
sm501_2d_operation
  Shadow bytes around the buggy address:
0x0fee69877fa0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0fee69877fb0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0fee69877fc0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0fee69877fd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0fee69877fe0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  =>0x0fee69877ff0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa]
0x0fee69878000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fee69878010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fee69878020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fee69878030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0fee69878040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable:   00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone:   fa
Freed heap region:   fd
Poisoned by user:f7
ASan internal:   fe
  ==20518==ABORTING

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1786026
Reported-by: Zhang Zi Ming <1015138...@qq.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
Per the links on
https://bugzilla.redhat.com/show_bug.cgi?id=1808510 there is probably
a CVE assigned to this, but I don't have access to the information,
https://bugzilla.redhat.com/show_bug.cgi?id=1786593 only show:

  You are not authorized to access bug #1786593.
  Most likely the bug has been restricted for internal development processes 
and we cannot grant access.
---
 hw/display/sm501.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index de0ab9d977..902acb3875 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -726,6 +726,12 @@ static void sm501_2d_operation(SM501State *s)
 int crt = (s->dc_crt_control & SM501_DC_CRT_CONTROL_SEL) ? 1 : 0;
 int fb_len = get_width(s, crt) * get_height(s, crt) * get_bpp(s, crt);
 
+if (rtl && (src_x < operation_width || src_y < operation_height)) {
+qemu_log_mask(LOG_GUEST_ERROR, "sm501: Illegal RTL address (%i, %i)\n",
+  src_x, src_y);
+return;
+}
+
 if (addressing != 0x0) {
 printf("%s: only XY addressing is supported.\n", __func__);
 abort();
-- 
2.21.1

[PATCH v2] virtiofsd/passthrough_ll: don't remove O_DIRECT when cache=none

[Catherine Ho]

cache=none means to bypass host cache. So we can't remove O_DIRECT flag in
unconditionally in update_open_flags();

Signed-off-by: Catherine Ho 
---
v2: Fix to keep flags unchanged if cache=none, otherwise changed the file
without O_DIRECT incorrectly.
 tools/virtiofsd/passthrough_ll.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4c35c95..59e64dd 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1677,7 +1677,8 @@ static void lo_releasedir(fuse_req_t req, fuse_ino_t ino,
 fuse_reply_err(req, 0);
 }
 
-static void update_open_flags(int writeback, struct fuse_file_info *fi)
+static void update_open_flags(int writeback, int cache_mode,
+  struct fuse_file_info *fi)
 {
 /*
  * With writeback cache, kernel may send read requests even
@@ -1702,10 +1703,11 @@ static void update_open_flags(int writeback, struct 
fuse_file_info *fi)
 
 /*
  * O_DIRECT in guest should not necessarily mean bypassing page
- * cache on host as well. If somebody needs that behavior, it
- * probably should be a configuration knob in daemon.
+ * cache on host as well. If cache=none, keep the flag unchanged
  */
-fi->flags &= ~O_DIRECT;
+if (cache_mode != CACHE_NONE) {
+fi->flags &= ~O_DIRECT;
+}
 }
 
 static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
@@ -1737,7 +1739,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, 
const char *name,
 goto out;
 }
 
-update_open_flags(lo->writeback, fi);
+update_open_flags(lo->writeback, lo->cache, fi);
 
 fd = openat(parent_inode->fd, name, (fi->flags | O_CREAT) & ~O_NOFOLLOW,
 mode);
@@ -1947,7 +1949,7 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, 
struct fuse_file_info *fi)
 fuse_log(FUSE_LOG_DEBUG, "lo_open(ino=%" PRIu64 ", flags=%d)\n", ino,
  fi->flags);
 
-update_open_flags(lo->writeback, fi);
+update_open_flags(lo->writeback, lo->cache, fi);
 
 sprintf(buf, "%i", lo_fd(req, ino));
 fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW);
-- 
1.7.1

[PATCH] virtiofsd/passthrough_ll: don't remove O_DIRECT when cache=none

[Catherine Ho]

cache=none means to bypass host cache. So we can't remove O_DIRECT flag in
unconditionally in update_open_flags();

Signed-off-by: Catherine Ho 
---
 tools/virtiofsd/passthrough_ll.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4c35c95..889e144 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1677,7 +1677,8 @@ static void lo_releasedir(fuse_req_t req, fuse_ino_t ino,
 fuse_reply_err(req, 0);
 }
 
-static void update_open_flags(int writeback, struct fuse_file_info *fi)
+static void update_open_flags(int writeback, int cache_mode,
+  struct fuse_file_info *fi)
 {
 /*
  * With writeback cache, kernel may send read requests even
@@ -1702,10 +1703,13 @@ static void update_open_flags(int writeback, struct 
fuse_file_info *fi)
 
 /*
  * O_DIRECT in guest should not necessarily mean bypassing page
- * cache on host as well. If somebody needs that behavior, it
- * probably should be a configuration knob in daemon.
+ * cache on host as well. If cache=none, set the flag to O_DIRECT
  */
-fi->flags &= ~O_DIRECT;
+if (cache_mode == CACHE_NONE) {
+fi->flags |= O_DIRECT;
+} else {
+fi->flags &= ~O_DIRECT;
+}
 }
 
 static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
@@ -1737,7 +1741,7 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, 
const char *name,
 goto out;
 }
 
-update_open_flags(lo->writeback, fi);
+update_open_flags(lo->writeback, lo->cache, fi);
 
 fd = openat(parent_inode->fd, name, (fi->flags | O_CREAT) & ~O_NOFOLLOW,
 mode);
@@ -1947,7 +1951,7 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, 
struct fuse_file_info *fi)
 fuse_log(FUSE_LOG_DEBUG, "lo_open(ino=%" PRIu64 ", flags=%d)\n", ino,
  fi->flags);
 
-update_open_flags(lo->writeback, fi);
+update_open_flags(lo->writeback, lo->cache, fi);
 
 sprintf(buf, "%i", lo_fd(req, ino));
 fd = openat(lo->proc_self_fd, buf, fi->flags & ~O_NOFOLLOW);
-- 
1.7.1