[Bug 1880539] Re: I/O write make QXL abort in qxl_set_mode()

2020-07-10 Thread Alexander Bulekov
Here's a qtest reproducer for this:
cat << EOF | ./i386-softmmu/qemu-system-i386 -M q35,accel=qtest -qtest null 
-nographic -vga qxl -qtest stdio -nodefaults
outl 0xcf8 0x8804
outb 0xcfc 0xff
outl 0xcf8 0x8819
outl 0xcfc 0x87caff7a
outb 0x86 0x23
EOF

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

Title:
  I/O write make QXL abort in qxl_set_mode()

Status in QEMU:
  New

Bug description:
  libFuzzer found:

  qxl-0: guest bug: qxl_add_memslot: guest_start > guest_end 0x 
> 0x3ff
  qemu-fuzz-i386: hw/display/qxl.c:1611: void qxl_set_mode(PCIQXLDevice *, 
unsigned int, int): Assertion `qxl_add_memslot(d, 0, devmem, QXL_SYNC) == 0' 
failed.
  ==8134== ERROR: libFuzzer: deadly signal
  #0 0x55fddfcfb3f0 in __sanitizer_print_stack_trace 
(qemu-fuzz-i386+0xcb13f0)
  #1 0x55fddfc0a3e1 in fuzzer::PrintStackTrace() (qemu-fuzz-i386+0xbc03e1)
  #2 0x55fddfbeac6f in fuzzer::Fuzzer::CrashCallback() 
(qemu-fuzz-i386+0xba0c6f)
  #3 0x55fddfbeacc3 in fuzzer::Fuzzer::StaticCrashSignalCallback() 
(qemu-fuzz-i386+0xba0cc3)
  #4 0x7fd640644c6f  (/lib64/libpthread.so.0+0x12c6f)
  #5 0x7fd640483e34 in __GI_raise (/lib64/libc.so.6+0x37e34)
  #6 0x7fd64046e894 in __GI_abort (/lib64/libc.so.6+0x22894)
  #7 0x7fd64046e768 in __assert_fail_base.cold (/lib64/libc.so.6+0x22768)
  #8 0x7fd64047c565 in __GI___assert_fail (/lib64/libc.so.6+0x30565)
  #9 0x55fde08afd8b in qxl_set_mode (qemu-fuzz-i386+0x1865d8b)
  #10 0x55fde08b9602 in ioport_write (qemu-fuzz-i386+0x186f602)
  #11 0x55fddff170a7 in memory_region_write_accessor 
(qemu-fuzz-i386+0xecd0a7)
  #12 0x55fddff16c13 in access_with_adjusted_size (qemu-fuzz-i386+0xeccc13)
  #13 0x55fddff157b4 in memory_region_dispatch_write 
(qemu-fuzz-i386+0xecb7b4)

  Can be reproduce doing "writeb 0x06 0x23" on QXL I/O (PCI BAR #3).

  Command line: 'qemu-system-i386 -display none -M pc -vga qxl'

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



Re: [PATCH v5 1/4] target/i386: add missing vmx features for several CPU models

2020-07-10 Thread Chenyi Qiang




On 7/11/2020 12:48 AM, Eduardo Habkost wrote:

On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:



On 7/10/2020 6:12 AM, Eduardo Habkost wrote:


I'm very sorry for taking so long to review this.  Question
below:

On Fri, Jun 19, 2020 at 03:31:11PM +0800, Chenyi Qiang wrote:

Add some missing VMX features in Skylake-Server, Cascadelake-Server and
Icelake-Server CPU models based on the output of Paolo's script.

Signed-off-by: Chenyi Qiang 


Why are you changing the v1 definition instead adding those new
features in a new version of the CPU model, just like you did in
patch 3/4?



I suppose these missing vmx features are not quite necessary for customers.
Just post it here to see if they are worth being added.
Adding a new version is reasonable. Is it appropriate to put all the missing
features in patch 1/4, 3/4, 4/4 in a same version?


Yes, it would be OK to add only one new version with all the new
features.



OK, I'll do it in the next version of patch.



Re: [PATCH v2 0/5] hw/i2c: Rename method names for consistency and add documentation

2020-07-10 Thread Corey Minyard
On Fri, Jul 10, 2020 at 11:53:13AM +0200, Philippe Mathieu-Daudé wrote:
> Corey, this series is now fully reviewed :)

Ok, I've pulled this in and added the extra reviews that came in.  I'll
work on getting it out Monday.

Thanks,

-corey

> 
> On 7/6/20 12:41 AM, Philippe Mathieu-Daudé wrote:
> > In commit d88c42ff2c we added 2 methods: i2c_try_create_slave()
> > and i2c_realize_and_unref().
> > Markus noted their name could be improved for consistency [1],
> > and Peter reported the lack of documentation [2]. Fix that now.
> > 
> > Since v1:
> > - Addressed Markus review comments
> > - Added Markus/Corey R-b tags
> > 
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg07060.html
> > [2] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg08997.html
> > 
> > $ git backport-diff -u -v1
> > Key:
> > [] : patches are identical
> > [] : number of functional differences between upstream/downstream patch
> > [down] : patch is downstream-only
> > The flags [FC] indicate (F)unctional and (C)ontextual differences, 
> > respectively
> > 
> > 001/5:[] [--] 'hw/i2c/aspeed_i2c: Simplify aspeed_i2c_get_bus()'
> > 002/5:[0006] [FC] 'hw/i2c: Rename i2c_try_create_slave() as i2c_slave_new()'
> > 003/5:[0004] [FC] 'hw/i2c: Rename i2c_realize_and_unref() as 
> > i2c_slave_realize_and_unref()'
> > 004/5:[0006] [FC] 'hw/i2c: Rename i2c_create_slave() as 
> > i2c_slave_create_simple()'
> > 005/5:[0012] [FC] 'hw/i2c: Document the I2C qdev helpers'
> 



[PULL v3 41/47] cpu-throttle: new module, extracted from cpus.c

2020-07-10 Thread Paolo Bonzini
From: Claudio Fontana 

move the vcpu throttling functionality into its own module.

This functionality is not specific to any accelerator,
and it is used currently by migration to slow down guests to try to
have migrations converge, and by the cocoa MacOS UI to throttle speed.

cpu-throttle contains the controls to adjust and inspect throttle
settings, start (set) and stop vcpu throttling, and the throttling
function itself that is run periodically on vcpus to make them take a nap.

Execution of the throttling function on all vcpus is triggered by a timer,
registered at module initialization.

No functionality change.

Signed-off-by: Claudio Fontana 
Reviewed-by: Alex Bennée 
Reviewed-by: Laurent Vivier 
Message-Id: <20200629093504.3228-3-cfont...@suse.de>
Signed-off-by: Paolo Bonzini 
---
 MAINTAINERS   |   1 +
 include/hw/core/cpu.h |  37 ---
 include/qemu/main-loop.h  |   5 ++
 include/sysemu/cpu-throttle.h |  68 +++
 migration/migration.c |   1 +
 migration/ram.c   |   1 +
 softmmu/Makefile.objs |   1 +
 softmmu/cpu-throttle.c| 122 ++
 softmmu/cpus.c|  95 +++---
 ui/cocoa.m|   1 +
 10 files changed, 208 insertions(+), 124 deletions(-)
 create mode 100644 include/sysemu/cpu-throttle.h
 create mode 100644 softmmu/cpu-throttle.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 28f33123ec..361ae5c662 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2247,6 +2247,7 @@ F: util/qemu-timer.c
 F: softmmu/vl.c
 F: softmmu/main.c
 F: softmmu/cpus.c
+F: softmmu/cpu-throttle.c
 F: qapi/run-state.json
 
 Human Monitor (HMP)
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b3f4b79318..5542577d2b 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -822,43 +822,6 @@ bool cpu_exists(int64_t id);
  */
 CPUState *cpu_by_arch_id(int64_t id);
 
-/**
- * cpu_throttle_set:
- * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99.
- *
- * Throttles all vcpus by forcing them to sleep for the given percentage of
- * time. A throttle_percentage of 25 corresponds to a 75% duty cycle roughly.
- * (example: 10ms sleep for every 30ms awake).
- *
- * cpu_throttle_set can be called as needed to adjust new_throttle_pct.
- * Once the throttling starts, it will remain in effect until cpu_throttle_stop
- * is called.
- */
-void cpu_throttle_set(int new_throttle_pct);
-
-/**
- * cpu_throttle_stop:
- *
- * Stops the vcpu throttling started by cpu_throttle_set.
- */
-void cpu_throttle_stop(void);
-
-/**
- * cpu_throttle_active:
- *
- * Returns: %true if the vcpus are currently being throttled, %false otherwise.
- */
-bool cpu_throttle_active(void);
-
-/**
- * cpu_throttle_get_percentage:
- *
- * Returns the vcpu throttle percentage. See cpu_throttle_set for details.
- *
- * Returns: The throttle percentage in range 1 to 99.
- */
-int cpu_throttle_get_percentage(void);
-
 #ifndef CONFIG_USER_ONLY
 
 typedef void (*CPUInterruptHandler)(CPUState *, int);
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index a6d20b0719..8e98613656 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -303,6 +303,11 @@ void qemu_mutex_unlock_iothread(void);
  */
 void qemu_cond_wait_iothread(QemuCond *cond);
 
+/*
+ * qemu_cond_timedwait_iothread: like the previous, but with timeout
+ */
+void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
+
 /* internal interfaces */
 
 void qemu_fd_register(int fd);
diff --git a/include/sysemu/cpu-throttle.h b/include/sysemu/cpu-throttle.h
new file mode 100644
index 00..d65bdef6d0
--- /dev/null
+++ b/include/sysemu/cpu-throttle.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see
+ * 
+ */
+
+#ifndef SYSEMU_CPU_THROTTLE_H
+#define SYSEMU_CPU_THROTTLE_H
+
+#include "qemu/timer.h"
+
+/**
+ * cpu_throttle_init:
+ *
+ * Initialize the CPU throttling API.
+ */
+void cpu_throttle_init(void);
+
+/**
+ * cpu_throttle_set:
+ * @new_throttle_pct: Percent of sleep time. Valid range is 1 to 99.
+ *
+ * Throttles all vcpus by forcing them to sleep for the given percentage of
+ * time. A throttle_percentage of 25 corresponds to a 75% duty cycle roughly.
+ * (example: 10ms sleep for every 30ms awake).
+ *
+ * 

[PULL v3 00/47] Misc patches for QEMU 5.1 soft freeze

2020-07-10 Thread Paolo Bonzini
The following changes since commit 45db94cc90c286a9965a285ba19450f448760a09:

  Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-20200707' 
into staging (2020-07-10 16:43:40 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 56908dc5041aa424fd1495b6c6beb78c539d93e1:

  linux-headers: update again to 5.8 (2020-07-10 19:26:55 -0400)


* Make checkpatch say 'qemu' instead of 'kernel' (Aleksandar)
* Fix PSE guests with emulated NPT (Alexander B. #1)
* Fix leak (Alexander B. #2)
* HVF fixes (Roman, Cameron)
* New Sapphire Rapids CPUID bits (Cathy)
* cpus.c and softmmu/ cleanups (Claudio)
* TAP driver tweaks (Daniel, Havard)
* object-add bugfix and testcases (Eric A.)
* Fix Coverity MIN_CONST and MAX_CONST (Eric B.)
* "info lapic" improvement (Jan)
* SSE fixes (Joseph)
* "-msg guest-name" option (Mario)
* support for AMD nested live migration (myself)
* Small i386 TCG fixes (myself)
* improved error reporting for Xen (myself)
* fix "-cpu host -overcommit cpu-pm=on" (myself)
* Add accel/Kconfig (Philippe)
* iscsi sense handling fixes (Yongji)
* Misc bugfixes


v2->v3: dropped Philippe's KVM series as it seemed to make some tests flaky
added Linux kernel headers update/fix
fixed Cocoa includes

Aleksandar Markovic (1):
  checkpatch: Change occurences of 'kernel' to 'qemu' in user messages

Alexander Boettcher (1):
  tcg/svm: use host cr4 during NPT page table walk

Alexander Bulekov (1):
  pc: fix leak in pc_system_flash_cleanup_unused

Cameron Esfahani (1):
  i386: hvf: Make long mode enter and exit clearer

Cathy Zhang (2):
  target/i386: Add SERIALIZE cpu feature
  target/i386: Enable TSX Suspend Load Address Tracking feature

Claudio Fontana (2):
  softmmu: move softmmu only files from root
  cpu-throttle: new module, extracted from cpus.c

Daniel P. Berrangé (1):
  scripts: improve message when TAP based tests fail

Eric Auger (3):
  qom: Introduce object_property_try_add_child()
  tests/qmp-cmd-test: Add qmp/object-add-duplicate-id
  tests/qmp-cmd-test: Add qmp/object-add-failure-modes

Eric Blake (1):
  coverity: provide Coverity-friendly MIN_CONST and MAX_CONST

Havard Skinnemoen (1):
  tests: Inject test name also when the test fails

Jan Kiszka (1):
  apic: Report current_count via 'info lapic'

Joseph Myers (2):
  target/i386: set SSE FTZ in correct floating-point state
  target/i386: fix IEEE SSE floating-point exception raising

Luwei Kang (1):
  target/i386: Correct the warning message of Intel PT

Mario Smarduch (1):
  util/qemu-error: prepend guest name to error message to identify affected 
VM owner

Paolo Bonzini (8):
  KVM: add support for AMD nested live migration
  Makefile: simplify MINIKCONF rules
  target/i386: remove gen_io_end
  target/i386: implement undocumented "smsw r32" behavior
  KVM: x86: believe what KVM says about WAITPKG
  target/i386: sev: provide proper error reporting for 
query-sev-capabilities
  target/i386: sev: fail query-sev-capabilities if QEMU cannot use SEV
  linux-headers: update again to 5.8

Philippe Mathieu-Daudé (10):
  hw/core/null-machine: Do not initialize unused chardev backends
  MAINTAINERS: Fix KVM path expansion glob
  MAINTAINERS: Add an 'overall' entry for accelerators
  MAINTAINERS: Cover the HAX accelerator stub
  Makefile: Remove dangerous EOL trailing backslash
  Makefile: Write MINIKCONF variables as one entry per line
  accel/Kconfig: Extract accel selectors into their own config
  accel/Kconfig: Add the TCG selector
  accel/tcg: Add stub for probe_access()
  cpus: Move CPU code from exec.c to cpus-common.c

Roman Bolshakov (7):
  i386: hvf: Set env->eip in macvm_set_rip()
  i386: hvf: Move synchronize functions to sysemu
  i386: hvf: Add hvf_cpu_synchronize_pre_loadvm()
  i386: hvf: Move Guest LMA reset to macvm_set_cr0()
  i386: hvf: Don't duplicate register reset
  i386: hvf: Clean up synchronize functions
  MAINTAINERS: Add Cameron as HVF co-maintainer

Thomas Huth (1):
  softmmu/vl: Remove the check for colons in -accel parameters

Xie Yongji (2):
  iscsi: handle check condition status in retry loop
  iscsi: return -EIO when sense fields are meaningless

 Kconfig  |   4 +
 Kconfig.host |   7 -
 MAINTAINERS  |  29 +-
 Makefile |  12 +-
 Makefile.target  |   7 +-
 accel/Kconfig|   9 +
 accel/stubs/tcg-stub.c   |   7 +
 block/iscsi.c|  22 +-
 cpus-common.c  

[PULL v3 47/47] linux-headers: update again to 5.8

2020-07-10 Thread Paolo Bonzini
5.8-rc1 inadvertently broke userspace ABI compatibility.  Merge
again with latest kvm/master to undo that.

Signed-off-by: Paolo Bonzini 
---
 linux-headers/asm-arm/unistd-common.h | 1 +
 linux-headers/asm-x86/kvm.h   | 5 +++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/linux-headers/asm-arm/unistd-common.h 
b/linux-headers/asm-arm/unistd-common.h
index 23de64e44c..57cd1f21db 100644
--- a/linux-headers/asm-arm/unistd-common.h
+++ b/linux-headers/asm-arm/unistd-common.h
@@ -392,5 +392,6 @@
 #define __NR_clone3 (__NR_SYSCALL_BASE + 435)
 #define __NR_openat2 (__NR_SYSCALL_BASE + 437)
 #define __NR_pidfd_getfd (__NR_SYSCALL_BASE + 438)
+#define __NR_faccessat2 (__NR_SYSCALL_BASE + 439)
 
 #endif /* _ASM_ARM_UNISTD_COMMON_H */
diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index 17c5a038f4..0780f97c18 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -408,14 +408,15 @@ struct kvm_vmx_nested_state_data {
 };
 
 struct kvm_vmx_nested_state_hdr {
-   __u32 flags;
__u64 vmxon_pa;
__u64 vmcs12_pa;
-   __u64 preemption_timer_deadline;
 
struct {
__u16 flags;
} smm;
+
+   __u32 flags;
+   __u64 preemption_timer_deadline;
 };
 
 struct kvm_svm_nested_state_data {
-- 
2.26.2




[PATCH for-5.1] tcg: Save/restore vecop_list around minmax fallback

2020-07-10 Thread Richard Henderson
Forgetting this asserts when tcg_gen_cmp_vec is called from
within tcg_gen_cmpsel_vec.

Fixes: 72b4c792c7a
Signed-off-by: Richard Henderson 
---

I found this while testing SVE2 (and the patch is included in the
large SVE2 patch set), but it seems like it should be reproducible
with master.  What's needed is a guest vector minmax operation of
a size that is not supported by the host.  In the case of x86_64
host, that would be a 64-bit minmax.


r~

---
 tcg/tcg-op-vec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tcg/tcg-op-vec.c b/tcg/tcg-op-vec.c
index f784517d84..ed6fb55fe1 100644
--- a/tcg/tcg-op-vec.c
+++ b/tcg/tcg-op-vec.c
@@ -657,7 +657,9 @@ static void do_minmax(unsigned vece, TCGv_vec r, TCGv_vec a,
   TCGv_vec b, TCGOpcode opc, TCGCond cond)
 {
 if (!do_op3(vece, r, a, b, opc)) {
+const TCGOpcode *hold_list = tcg_swap_vecop_list(NULL);
 tcg_gen_cmpsel_vec(cond, vece, r, a, b, a, b);
+tcg_swap_vecop_list(hold_list);
 }
 }
 
-- 
2.25.1




Re: [PATCH 3/3] cpu-timers, icount: new modules

2020-07-10 Thread Paolo Bonzini
On 10/07/20 06:36, Thomas Huth wrote:
> 
> In short this goes away if I again set icount to enabled for qtest,
> basically ensuring that --enable-tcg is there and then reenabling icount.
> 
> qtest was forcing icount and shift=0 by creating qemu options, in order to 
> misuse its counter feature,
> instead of using a separate counter.

Why would it need a separate counter?  In both cases it's a
manually-updated counter that is used for QEMU_CLOCK_VIRTUAL.  The only
difference is that shift > 0 doesn't make sense for qtest.

Paolo




[Bug 697510] Re: Machine shut off after tons of lsi_scsi: error: MSG IN data too long

2020-07-10 Thread Alexander Bulekov
Here is a qtest reproducer:

cat << EOF | ./i386-softmmu/qemu-system-i386 -nographic -M q35,accel=qtest 
-qtest stdio -drive 
if=none,id=drive0,file=null-co://,file.read-zeroes=on,format=raw   -device 
lsi53c895a,id=scsi0 -device 
scsi-hd,drive=drive0,bus=scsi0.0,channel=0,scsi-id=0,lun=0 -monitor none 
-serial none 
outl 0xcf8 0x80001814
outl 0xcfc 0xe1068000
outl 0xcf8 0x80001818
outl 0xcf8 0x80001804
outw 0xcfc 0x7
outl 0xcf8 0x80002010
write 0xe106802e 0x1 0xff
write 0xe106802f 0x1 0xff
EOF

With -trace lsi\*:
...
[R +0.037396] write 0xe106802e 0x1 0xff
15257@1594419708.889733:lsi_reg_write Write reg DSP2 0x2e = 0xff
OK
[S +0.037420] OK
[R +0.037434] write 0xe106802f 0x1 0xff
15257@1594419708.889814:lsi_reg_write Write reg DSP3 0x2f = 0xff
15257@1594419708.889862:lsi_execute_script SCRIPTS dsp=0x opcode 
0x105e8b06 arg 0x89084e8b
qemu-system-i386: /home/alxndr/Development/qemu/hw/scsi/lsi53c895a.c:624: void 
lsi_do_dma(LSIState *, int): Assertion `s->current' failed.

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

Title:
  Machine shut off after tons of lsi_scsi: error: MSG IN data too long

Status in QEMU:
  New

Bug description:
  The problem mostly happens during our backup, syslog does not have any
  problematic messages.

  Host is Ubuntu 10.10 x86 64 bits
  Guest is Windows 2003 Server 32 bits
  Using Virtio and Red Hat driver I get a STOP screen 0x10d1 and machine 
either reboot, stay frozen or shut off.
  Using SCSI the machine shuts off and I get tons of message on stdout;

  LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -S -M pc-0.12 -enable-kvm -m 3500 -smp 
4,sockets=4,cores=1,threads=1 -name BMSRV0101 -uuid 
6ccbb5fa-5880-a1ab-3b7a-fb7ccc7c8ccf -nodefaults -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/BMSRV0101.monitor,server,nowait 
-mon chardev=monitor,mode=readline -rtc base=localtime -boot c -device 
lsi,id=scsi0,bus=pci.0,addr=0x4 -drive 
if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device 
ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -drive 
file=/dev/vgUbuntu/vmBMSRV0101,if=none,id=drive-scsi0-0-0,boot=on,format=raw 
-device scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 
-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:5d:7b:07,bus=pci.0,addr=0x3 
-net tap,fd=63,vlan=0,name=hostnet0 -chardev pty,id=serial0 -device 
isa-serial,chardev=serial0 -usb -device usb-tablet,id=input0 -vnc 127.0.0.1:0 
-vga cirrus -device usb-host,hostbus=002,hostaddr=005,id=hostdev0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 
  char device redirected to /dev/pts/0
  pci_add_option_rom: failed to find romfile "pxe-virtio.bin"
  husb: open device 2.5
  husb: config #1 need -1
  husb: 1 interfaces claimed for configuration 1
  husb: grabbed usb device 2.5
  husb: config #1 need 1
  husb: 1 interfaces claimed for configuration 1

  lsi_scsi: error: Unimplemented message 0x00
  (x8)

  lsi_scsi: error: MSG IN data too long
  lsi_scsi: error: Unimplemented message 0x00
  (x760)

  lsi_scsi: error: MSG IN data too long
  lsi_scsi: error: MSG IN data too long
  kvm: /build/buildd/qemu-kvm-0.12.5+noroms/hw/lsi53c895a.c:512: lsi_do_dma: 
Assertion `s->current' failed.

  
  I can include minidump file if needed.
  I am currently using IDE and it seems OK.

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



[PATCH 0/1] MAINTAINERS: Add Python library stanza

2020-07-10 Thread John Snow



John Snow (1):
  MAINTAINERS: Add Python library stanza

 MAINTAINERS | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

-- 
2.21.3




[PATCH 1/1] MAINTAINERS: Add Python library stanza

2020-07-10 Thread John Snow
I'm proposing that I split the actual Python library off from the other
miscellaneous python scripts we have and declare it maintained. Add
myself as a maintainer of this folder, along with Cleber.

Signed-off-by: John Snow 
---
 MAINTAINERS | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6aa54f7f8f..fe1dcd5a76 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2280,11 +2280,18 @@ S: Maintained
 F: include/sysemu/cryptodev*.h
 F: backends/cryptodev*.c
 
+Python library
+M: John Snow 
+M: Cleber Rosa 
+R: Eduardo Habkost 
+S: Maintained
+F: python/*
+T: git https://gitlab.com/jsnow/qemu.git python
+
 Python scripts
 M: Eduardo Habkost 
 M: Cleber Rosa 
 S: Odd fixes
-F: python/qemu/*py
 F: scripts/*.py
 F: tests/*.py
 
-- 
2.21.3




Re: [RFC 00/65] target/riscv: support vector extension v0.9

2020-07-10 Thread Alistair Francis
On Fri, Jul 10, 2020 at 5:59 AM  wrote:
>
> From: Frank Chang 
>
> This patchset implements the vector extension v0.9 for RISC-V on QEMU.
>
> This patchset is sent as RFC because RVV v0.9 is still in draft state.
> However, as RVV v1.0 should be ratified soon and there shouldn't be
> critical changes between RVV v1.0 and RVV v0.9. We would like to have
> the community to review RVV v0.9 implementations. Once RVV v1.0 is
> ratified, we will then upgrade to RVV v1.0.
>
> You can change the cpu argument: vext_spec to v0.9 (i.e. vext_spec=v0.9)
> to run with RVV v0.9 instructions.

Hello,

First off thanks for the patches!

QEMU has a policy of accepting draft specs as experimental. We
currently support the v0.7.1 Vector extension for example, so this
does not need to be an RFC and can be a full patch series that can be
merged into master.

I have applied the first few patches (PR should be out next week) and
they should be in the QEMU 5.1 release. QEMU is currently in a freeze
so I won't be able to merge this series for 5.1. In saying that please
feel free to continue to send patches to the list, they can still be
reviewed.

In general we would need to gracefully handle extension upgrades and
maintain backwards compatibility. This same principle doesn't apply to
experimental features though (such as the vector extension) so you are
free to remove support for the v0.7.1. For users who want v0.7.1
support they can always use the QEMU 5.1. release. Just make sure that
your series does not break bisectability.

Thanks again for the patches!

Alistair

>
> Chih-Min Chao (2):
>   fpu: fix float16 nan check
>   fpu: add api to handle alternative sNaN propagation
>
> Frank Chang (58):
>   target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion
>   target/riscv: correct the gvec IR called in gen_vec_rsub16_i64()
>   target/riscv: fix return value of do_opivx_widen()
>   target/riscv: fix vill bit index in vtype register
>   target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec
>   target/riscv: rvv-0.9: remove MLEN calculations
>   target/riscv: rvv-0.9: add fractional LMUL, VTA and VMA
>   target/riscv: rvv-0.9: update check functions
>   target/riscv: rvv-0.9: configure instructions
>   target/riscv: rvv-0.9: stride load and store instructions
>   target/riscv: rvv-0.9: index load and store instructions
>   target/riscv: rvv-0.9: fix address index overflow bug of indexed
> load/store insns
>   target/riscv: rvv-0.9: fault-only-first unit stride load
>   target/riscv: rvv-0.9: amo operations
>   target/riscv: rvv-0.9: load/store whole register instructions
>   target/riscv: rvv-0.9: update vext_max_elems() for load/store insns
>   target/riscv: rvv-0.9: take fractional LMUL into vector max elements
> calculation
>   target/riscv: rvv-0.9: floating-point square-root instruction
>   target/riscv: rvv-0.9: floating-point classify instructions
>   target/riscv: rvv-0.9: mask population count instruction
>   target/riscv: rvv-0.9: find-first-set mask bit instruction
>   target/riscv: rvv-0.9: set-X-first mask bit instructions
>   target/riscv: rvv-0.9: iota instruction
>   target/riscv: rvv-0.9: element index instruction
>   target/riscv: rvv-0.9: integer scalar move instructions
>   target/riscv: rvv-0.9: floating-point scalar move instructions
>   target/riscv: rvv-0.9: whole register move instructions
>   target/riscv: rvv-0.9: integer extension instructions
>   target/riscv: rvv-0.9: single-width averaging add and subtract
> instructions
>   target/riscv: rvv-0.9: integer add-with-carry/subtract-with-borrow
>   target/riscv: rvv-0.9: narrowing integer right shift instructions
>   target/riscv: rvv-0.9: widening integer multiply-add instructions
>   target/riscv: rvv-0.9: quad-widening integer multiply-add instructions
>   target/riscv: rvv-0.9: integer merge and move instructions
>   target/riscv: rvv-0.9: single-width saturating add and subtract
> instructions
>   target/riscv: rvv-0.9: integer comparison instructions
>   target/riscv: rvv-0.9: floating-point compare instructions
>   target/riscv: rvv-0.9: single-width integer reduction instructions
>   target/riscv: rvv-0.9: widening integer reduction instructions
>   target/riscv: rvv-0.9: mask-register logical instructions
>   target/riscv: rvv-0.9: register gather instructions
>   target/riscv: rvv-0.9: slide instructions
>   target/riscv: rvv-0.9: floating-point slide instructions
>   target/riscv: rvv-0.9: narrowing fixed-point clip instructions
>   target/riscv: rvv-0.9: floating-point move instructions
>   target/riscv: rvv-0.9: floating-point/integer type-convert
> instructions
>   target/riscv: rvv-0.9: single-width floating-point reduction
>   target/riscv: rvv-0.9: widening floating-point reduction instructions
>   target/riscv: rvv-0.9: single-width scaling shift instructions
>   target/riscv: rvv-0.9: remove widening saturating scaled multiply-add
>   target/riscv: rvv-0.9: remove vmford.vv and vmford.vf
>   

Re: [PATCH v1 10/13] tests/plugins: add -Wno-unknown-warning-option to handle -Wpsabi

2020-07-10 Thread Richard Henderson
On 7/9/20 7:13 AM, Alex Bennée wrote:
> Not all compilers support the -Wpsabi (clang-9 in my case).
> 
> Fixes: bac8d222a
> Signed-off-by: Alex Bennée 
> ---
>  tests/plugin/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/plugin/Makefile b/tests/plugin/Makefile
> index 0cb8e35ae407..dcfbd99b15b8 100644
> --- a/tests/plugin/Makefile
> +++ b/tests/plugin/Makefile
> @@ -18,7 +18,7 @@ NAMES += hwprofile
>  
>  SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
>  
> -QEMU_CFLAGS += -fPIC -Wpsabi
> +QEMU_CFLAGS += -fPIC -Wno-unknown-warning-option -Wpsabi

Surely -Wno-unknown-warning-option is in the same boat?  E.g. I don't see any
version of gcc that supports it.

Originally, I tried to grab -Wno-psabi out of the existing QEMU_CFLAGS and
transforming it, but I couldn't make that work.


r~

>  QEMU_CFLAGS += -I$(SRC_PATH)/include/qemu
>  
>  all: $(SONAMES)
> 




Re: [PATCH v1 09/13] target/sh4: revert to using cpu_lduw_code to decode gusa

2020-07-10 Thread Richard Henderson
On 7/9/20 7:13 AM, Alex Bennée wrote:
> The translator_ld* functions very much expect us to be decoding one
> instruction at a time. Otherwise we will see weirdness such as:
> 
>   qemu-sh4: warning: plugin_disas: 6 bytes left over
> 
> when we use the disas functions. For what SH4 is doing here (scanning
> ahead in the instruction stream) this is the right function to use.

It is not just scanning ahead.  It does that, but after having done so it may
also commit to translating them all at once.

In the case to which you refer, above, we have translated 4 insns into one
operation.  Just having plugin_disas see the first one is not really correct
either.


r~

> 
> Reported-by: Claudio Fontana 
> Signed-off-by: Alex Bennée 
> ---
>  target/sh4/translate.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/sh4/translate.c b/target/sh4/translate.c
> index 6192d83e8c66..919da72a0c98 100644
> --- a/target/sh4/translate.c
> +++ b/target/sh4/translate.c
> @@ -1915,9 +1915,13 @@ static void decode_gusa(DisasContext *ctx, CPUSH4State 
> *env)
>  goto fail;
>  }
>  
> -/* Read all of the insns for the region.  */
> +/*
> + * Read all of the insns for the region. We do this directly with
> + * cpu_lduw_code to avoid confusing the plugins by decoding
> + * multiple instructions.
> + */
>  for (i = 0; i < max_insns; ++i) {
> -insns[i] = translator_lduw(env, pc + i * 2);
> +insns[i] = cpu_lduw_code(env, pc + i * 2);
>  }
>  
>  ld_adr = ld_dst = ld_mop = -1;
> 




Re: [PATCH v1 04/13] cputlb: ensure we save the IOTLB data in case of reset

2020-07-10 Thread Richard Henderson
On 7/9/20 7:13 AM, Alex Bennée wrote:
> Any write to a device might cause a re-arrangement of memory
> triggering a TLB flush and potential re-size of the TLB invalidating
> previous entries. This would cause users of qemu_plugin_get_hwaddr()
> to see the warning:
> 
>   invalid use of qemu_plugin_get_hwaddr
> 
> because of the failed tlb_lookup which should always succeed. To
> prevent this we save the IOTLB data in case it is later needed by a
> plugin doing a lookup.
> 
> Signed-off-by: Alex Bennée 
> 
> ---
> v2
>   - save the entry instead of re-running the tlb_fill.
> v3
>   - don't abuse TLS, use CPUState to store data
>   - just use g_free_rcu() to avoid ugliness
>   - verify addr matches before returning data
>   - ws fix
> ---
>  include/hw/core/cpu.h   |  4 +++
>  include/qemu/typedefs.h |  1 +
>  accel/tcg/cputlb.c  | 57 +++--
>  3 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index b3f4b7931823..bedbf098dc57 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -417,7 +417,11 @@ struct CPUState {
>  
>  DECLARE_BITMAP(plugin_mask, QEMU_PLUGIN_EV_MAX);
>  
> +#ifdef CONFIG_PLUGIN
>  GArray *plugin_mem_cbs;
> +/* saved iotlb data from io_writex */
> +SavedIOTLB *saved_iotlb;
> +#endif
>  
>  /* TODO Move common fields from CPUArchState here. */
>  int cpu_index;
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 15f5047bf1dc..427027a9707a 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -116,6 +116,7 @@ typedef struct QObject QObject;
>  typedef struct QString QString;
>  typedef struct RAMBlock RAMBlock;
>  typedef struct Range Range;
> +typedef struct SavedIOTLB SavedIOTLB;
>  typedef struct SHPCDevice SHPCDevice;
>  typedef struct SSIBus SSIBus;
>  typedef struct VirtIODevice VirtIODevice;
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index 1e815357c709..8636b66e036a 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1073,6 +1073,42 @@ static uint64_t io_readx(CPUArchState *env, 
> CPUIOTLBEntry *iotlbentry,
>  return val;
>  }
>  
> +#ifdef CONFIG_PLUGIN
> +
> +typedef struct SavedIOTLB {
> +struct rcu_head rcu;
> +hwaddr addr;
> +MemoryRegionSection *section;
> +hwaddr mr_offset;
> +} SavedIOTLB;
> +
> +/*
> + * Save a potentially trashed IOTLB entry for later lookup by plugin.
> + *
> + * We also need to track the thread storage address because the RCU
> + * cleanup that runs when we leave the critical region (the current
> + * execution) is actually in a different thread.
> + */
> +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection 
> *section, hwaddr mr_offset)

Overlong line.

> +{
> +SavedIOTLB *old, *new = g_new(SavedIOTLB, 1);
> +new->addr = addr;
> +new->section = section;
> +new->mr_offset = mr_offset;
> +old = atomic_rcu_read(>saved_iotlb);
> +atomic_rcu_set(>saved_iotlb, new);
> +if (old) {
> +g_free_rcu(old, rcu);
> +}
> +}

I'm a bit confused by this.  Why all the multiple allocation?  How many
consumers are you expecting, and more are you expecting multiple memory
operations in flight at once?

If multiple memory operations in flight, then why aren't we chaining them
together, so that you can search through multiple alternatives.

If only one memory operation in flight, why are you allocating memory at all,
much less managing it with rcu?  Just put one structure (or a collection of
fields) into CPUState and be done.

> +
> +#else
> +static void save_iotlb_data(CPUState *cs, hwaddr addr, MemoryRegionSection 
> *section, hwaddr mr_offset)
> +{
> +/* do nothing */
> +}
> +#endif

Surely better to move the ifdef inside the function so that you don't have to
replicate the definition?

> +SavedIOTLB *saved = atomic_rcu_read(>saved_iotlb);
> +if (saved && saved->addr == tlb_addr) {
> +data->is_io = true;
> +data->v.io.section = saved->section;
> +data->v.io.offset = saved->mr_offset;
> +return true;
> +}

Should that test in fact be an assert?  Why would this fail?


r~



Re: [PATCH 1/2] iotests: Drop readarray from _do_filter_img_create

2020-07-10 Thread Alex Bennée


Max Reitz  writes:

> Some systems where we run tests on do not have a 4.x bash, so they do
> not have readarray.  While it looked a bit nicer than messing with
> `head` and `tail`, we do not really need it, so we might as well not use
> it.

I've fixed the cirrus build failure by brew installing a more recent
bash. However if we prefer we could do this.

>
> Reported-by: Claudio Fontana 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/common.filter | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qemu-iotests/common.filter 
> b/tests/qemu-iotests/common.filter
> index 3833206327..345c3ca03e 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -138,13 +138,13 @@ _do_filter_img_create()
>  # Split the line into the pre-options part ($filename_part, which
>  # precedes ", fmt=") and the options part ($options, which starts
>  # with "fmt=")
> -# (And just echo everything before the first "^Formatting")
> -readarray formatting_line < <($SED -e 's/, fmt=/\n/')
> +read formatting_line
>  
> -filename_part=${formatting_line[0]}
> -unset formatting_line[0]
> +# Split line at the first ", fmt="
> +formatting_line=$(echo "$formatting_line" | $SED -e 's/, fmt=/\nfmt=/')
>  
> -options="fmt=${formatting_line[@]}"
> +filename_part=$(echo "$formatting_line" | head -n 1)
> +options=$(echo "$formatting_line" | tail -n +2)
>  
>  # Set grep_data_file to '\|data_file' to keep it; make it empty
>  # to drop it.


-- 
Alex Bennée



[PATCH] Emulate dip switch language layout settings on SUN keyboard

2020-07-10 Thread Henrik Carlqvist
SUN Type 4, 5 and 5c keyboards have dip switches to choose the language
layout of the keyboard. Solaris makes an ioctl to query the value of the
dipswitches and uses that value to select keyboard layout. Also the SUN
bios like the one in the file ss5.bin uses this value to support at least
some keyboard layouts. However, the OpenBIOS provided with qemu is
hardcoded to allways use an US keyboard layout.

Before this patch, qemu allways gave dip switch value 0x21 (US keyboard),
this patch uses the command line switch "-k" (keyboard layout) to select
dip switch value. A table is used to lookup values from arguments like:

-k fr
-k es

But the patch also accepts numeric dip switch values directly to the -k
switch:

-k 0x2b
-k 43

Both values above are the same and select swedish keyboard as explained in
table 3-15 at
https://docs.oracle.com/cd/E19683-01/806-6642/new-43/index.html

Unless you want to do a full Solaris installation but happen to have
access to a bios file, the easiest way to test that the patch works is to:

qemu-system-sparc -k sv -bios /path/to/ss5.bin

If you already happen to have a Solaris installation in a qemu disk image
file you can easily try different keyboard layouts after this patch is
applied.

Unfortunately my glib version is too old to compile later versions of qemu
so even though this patch is made from latest git I have only been able to
test it myself with qemu version 4.1.1. I think and hope that this patch will
compile and work also with the latest version of git as it only affects one
file and there hasn't been much changes to that file since tested version
4.1.1.

Best regards Henrik

>From 2f86bd60750d44206b9181f76115e77b58dff544 Mon Sep 17 00:00:00 2001
From: Henrik Carlqvist 
Date: Fri, 10 Jul 2020 19:21:08 +0200
Subject: [PATCH] Emulating sun keyboard languate layout dip switches, taking
 the value for the dip switches from the "-k" option to qemu.

Signed-off-by: Henrik Carlqvist 
---
 hw/char/escc.c | 74
+- 1 file changed, 73
insertions(+), 1 deletion(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 7d16ee8688..7287056b5f 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -30,6 +30,8 @@
 #include "qemu/module.h"
 #include "hw/char/escc.h"
 #include "ui/console.h"
+#include "sysemu/sysemu.h"
+#include "qemu/cutils.h"
 #include "trace.h"
 
 /*
@@ -175,6 +177,7 @@
 #define R_MISC1I 14
 #define R_EXTINT 15
 
+static unsigned char sun_keyboard_layout_dip_switch(void);
 static void handle_kbd_command(ESCCChannelState *s, int val);
 static int serial_can_receive(void *opaque);
 static void serial_receive_byte(ESCCChannelState *s, int ch);
@@ -730,6 +733,75 @@ static QemuInputHandler sunkbd_handler = {
 .event = sunkbd_handle_event,
 };
 
+static unsigned char sun_keyboard_layout_dip_switch(void)
+{
+/* Return the value of the dip-switches in a SUN Type 5 keyboard */
+static unsigned char ret = 0xff;
+
+if ((ret == 0xff) && keyboard_layout) {
+int i;
+struct layout_values {
+const char *lang;
+unsigned char dip;
+} languages[] =
+/* Dip values from table 3-16 Layouts for Type 4, 5, and 5c Keyboards */
+{
+{"en-us", 0x21}, /* U.S.A. (US5.kt) */
+ /* 0x22 is some other US (US_UNIX5.kt)*/
+{"fr",0x23}, /* France (France5.kt) */
+{"da",0x24}, /* Denmark (Denmark5.kt) */
+{"de",0x25}, /* Germany (Germany5.kt) */
+{"it",0x26}, /* Italy (Italy5.kt) */
+{"nl",0x27}, /* The Netherlands (Netherland5.kt) */
+{"no",0x28}, /* Norway (Norway.kt) */
+{"pt",0x29}, /* Portugal (Portugal5.kt) */
+{"es",0x2a}, /* Spain (Spain5.kt) */
+{"sv",0x2b}, /* Sweden (Sweden5.kt) */
+{"fr-ch", 0x2c}, /* Switzerland/French (Switzer_Fr5.kt) */
+{"de-ch", 0x2d}, /* Switzerland/German (Switzer_Ge5.kt) */
+{"en-gb", 0x2e}, /* Great Britain (UK5.kt) */
+{"ko",0x2f}, /* Korea (Korea5.kt) */
+{"tw",0x30}, /* Taiwan (Taiwan5.kt) */
+{"ja",0x31}, /* Japan (Japan5.kt) */
+{"fr-ca", 0x32}, /* Canada/French (Canada_Fr5.kt) */
+{"hu",0x33}, /* Hungary (Hungary5.kt) */
+{"pl",0x34}, /* Poland (Poland5.kt) */
+{"cz",0x35}, /* Czech (Czech5.kt) */
+{"ru",0x36}, /* Russia (Russia5.kt) */
+{"lv",0x37}, /* Latvia (Latvia5.kt) */
+{"tr",0x38}, /* Turkey-Q5 (TurkeyQ5.kt) */
+{"gr",0x39}, /* Greece (Greece5.kt) */
+{"ar",0x3a}, /* Arabic (Arabic5.kt) */
+{"lt",0x3b}, /* Lithuania (Lithuania5.kt) */
+{"nl-be", 0x3c}, /* Belgium (Belgian5.kt) */
+

[PATCH 1/2] tests: fix "make check-qtest" for modular builds

2020-07-10 Thread Gerd Hoffmann
Signed-off-by: Gerd Hoffmann 
---
 tests/qtest/Makefile.include | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include
index 98af2c2d9338..6a0276fd42dd 100644
--- a/tests/qtest/Makefile.include
+++ b/tests/qtest/Makefile.include
@@ -277,6 +277,7 @@ tests/qtest/tco-test$(EXESUF): tests/qtest/tco-test.o 
$(libqos-pc-obj-y)
 tests/qtest/virtio-ccw-test$(EXESUF): tests/qtest/virtio-ccw-test.o
 tests/qtest/display-vga-test$(EXESUF): tests/qtest/display-vga-test.o
 tests/qtest/qom-test$(EXESUF): tests/qtest/qom-test.o
+tests/qtest/modules-test$(EXESUF): tests/qtest/modules-test.o
 tests/qtest/test-hmp$(EXESUF): tests/qtest/test-hmp.o
 tests/qtest/machine-none-test$(EXESUF): tests/qtest/machine-none-test.o
 tests/qtest/device-plug-test$(EXESUF): tests/qtest/device-plug-test.o
-- 
2.18.4




[PATCH 2/2] Revert "vga: build virtio-gpu as module"

2020-07-10 Thread Gerd Hoffmann
This reverts commit 8d5a24c83dba90b08ef163bbf166d6dfbad9019b.

Compiling all virtio-gpu objects into a single module isn't a good plan
because the individual objects have different CONFIG_* dependencies.
Leads to module load failures on s390x due to vga support being
disabled, which in turn breaks '-device virtio-gpu-device' (flagged by
travis ci).

So back to the drawing board for modular virtio-gpu ...

Signed-off-by: Gerd Hoffmann 
---
 util/module.c|  6 --
 hw/display/Makefile.objs | 23 ++-
 2 files changed, 10 insertions(+), 19 deletions(-)

diff --git a/util/module.c b/util/module.c
index 32b0547b8266..90e9bd42c6c7 100644
--- a/util/module.c
+++ b/util/module.c
@@ -266,12 +266,6 @@ static struct {
 { "usb-redir", "hw-", "usb-redirect"  },
 { "qxl-vga",   "hw-", "display-qxl"   },
 { "qxl",   "hw-", "display-qxl"   },
-{ "virtio-gpu-device", "hw-", "display-virtio-gpu"},
-{ "virtio-gpu-pci","hw-", "display-virtio-gpu"},
-{ "virtio-vga","hw-", "display-virtio-gpu"},
-{ "vhost-user-gpu-device", "hw-", "display-virtio-gpu"},
-{ "vhost-user-gpu-pci","hw-", "display-virtio-gpu"},
-{ "vhost-user-vga","hw-", "display-virtio-gpu"},
 { "chardev-braille",   "chardev-", "baum" },
 };
 
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index e907f3182b0c..d619594ad4d3 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -49,19 +49,16 @@ common-obj-m += qxl.mo
 qxl.mo-objs = qxl.o qxl-logger.o qxl-render.o
 endif
 
-ifeq ($(CONFIG_VIRTIO_GPU),y)
-common-obj-m += virtio-gpu.mo
-virtio-gpu-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o 
virtio-gpu-3d.o
-virtio-gpu-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
-virtio-gpu-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += 
virtio-gpu-pci.o
-virtio-gpu-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += 
vhost-user-gpu-pci.o
-virtio-gpu-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
-virtio-gpu-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
-virtio-gpu.mo-objs := $(virtio-gpu-obj-y)
-virtio-gpu.mo-cflags := $(VIRGL_CFLAGS)
-virtio-gpu.mo-libs := $(VIRGL_LIBS)
-endif
-
+common-obj-$(CONFIG_VIRTIO_GPU) += virtio-gpu-base.o virtio-gpu.o 
virtio-gpu-3d.o
+common-obj-$(CONFIG_VHOST_USER_GPU) += vhost-user-gpu.o
+common-obj-$(call land,$(CONFIG_VIRTIO_GPU),$(CONFIG_VIRTIO_PCI)) += 
virtio-gpu-pci.o
+common-obj-$(call land,$(CONFIG_VHOST_USER_GPU),$(CONFIG_VIRTIO_PCI)) += 
vhost-user-gpu-pci.o
+common-obj-$(CONFIG_VIRTIO_VGA) += virtio-vga.o
+common-obj-$(CONFIG_VHOST_USER_VGA) += vhost-user-vga.o
+virtio-gpu.o-cflags := $(VIRGL_CFLAGS)
+virtio-gpu.o-libs += $(VIRGL_LIBS)
+virtio-gpu-3d.o-cflags := $(VIRGL_CFLAGS)
+virtio-gpu-3d.o-libs += $(VIRGL_LIBS)
 common-obj-$(CONFIG_DPCD) += dpcd.o
 common-obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx_dp.o
 
-- 
2.18.4




[PATCH 0/2] module fixes

2020-07-10 Thread Gerd Hoffmann



Gerd Hoffmann (2):
  tests: fix "make check-qtest" for modular builds
  Revert "vga: build virtio-gpu as module"

 util/module.c|  6 --
 hw/display/Makefile.objs | 23 ++-
 tests/qtest/Makefile.include |  1 +
 3 files changed, 11 insertions(+), 19 deletions(-)

-- 
2.18.4




Re: [PATCH v11 00/34] Add subcluster allocation to qcow2

2020-07-10 Thread Alberto Garcia
On Fri 10 Jul 2020 06:43:59 PM CEST, no-re...@patchew.org wrote:
> /tmp/qemu-test/src/block/qcow2-cluster.c:473:19: error: 'expected_type' may 
> be used uninitialized in this function [-Werror=maybe-uninitialized]
>  } else if (type != expected_type) {
>^
> /tmp/qemu-test/src/block/qcow2-cluster.c:449:25: note: 'expected_type' was 
> declared here
>  QCow2SubclusterType expected_type, type;
>  ^

Meh, this is a false positive but I forgot to fix it. I'll do it if
there is a new version, otherwise please someone just initialize
expected_type to 0 when committing.

Berto



Re: [PATCH v1] target/riscv: fix pmp implementation

2020-07-10 Thread Alistair Francis
On Mon, Jul 6, 2020 at 2:45 AM Alexandre Mergnat  wrote:
>
> The end address calculation for NA4 mode is wrong because the address
> used isn't shifted.
>
> That imply all NA4 setup are not applied by the PMP.

I'm not sure what you mean here, can you clarify this?

>
> The solution is to use the shifted address calculated for start address
> variable.
>
> Modifications are tested on Zephyr OS userspace test suite which works
> for other RISC-V boards (E31 and E34 core).
>
> Signed-off-by: Alexandre Mergnat 

Otherwise:

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 9418660f1b..2a2b9f5363 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, uint32_t 
> pmp_index)
>
>  case PMP_AMATCH_NA4:
>  sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> -ea = (this_addr + 4u) - 1u;
> +ea = (sa + 4u) - 1u;
>  break;
>
>  case PMP_AMATCH_NAPOT:
> --
> 2.17.1
>
>



Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Eduardo Habkost
On Fri, Jul 10, 2020 at 05:02:38PM -0300, Thiago Jung Bauermann wrote:
> 
> Philippe Mathieu-Daudé  writes:
> 
> > On 7/9/20 5:26 AM, Thiago Jung Bauermann wrote:
> >>
> >> Thiago Jung Bauermann  writes:
> >>
> >>> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug.
> >>> Both of them are before cpu_reset() and ppc_cpu_reset().
> >>
> >> Hm, rereading the message obviously the above is partially wrong. The
> >> second case happens during ppc_cpu_reset().
> >>
> >>> Here's the second:
> >>>
> >>> #0  in qemu_cpu_kick_thread ()
> >>> #1  in qemu_cpu_kick ()
> >>> #2  in queue_work_on_cpu ()
> >>> #3  in async_run_on_cpu ()
> >>> #4  in tlb_flush_by_mmuidx ()
> >>> #5  in tlb_flush ()
> >>> #6  in ppc_tlb_invalidate_all ()
> >>> #7  in ppc_cpu_reset ()
> >>> #8  in device_transitional_reset ()
> >>> #9  in resettable_phase_hold ()
> >>> #10 in resettable_assert_reset ()
> >>> #11 in device_set_realized ()
> >
> > Dunno if related, might be helpful:
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg686477.html
> 
> Yes, it's helpful. Thanks!
> 
> So is was it resolved whether it's appropriate to do a cpu_reset()
> within realize?
> 
> Is the core of the problem that device_set_realize() ends up calling
> ppc_cpu_reset()?

There are 15 realize functions which call cpu_reset(), currently.
Making it safe seems more appropriate than forbidding it.

-- 
Eduardo




Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Thiago Jung Bauermann


Alex Bennée  writes:

> Thiago Jung Bauermann  writes:
>
>> Eduardo Habkost  writes:
>>
>>> On Wed, Jul 08, 2020 at 09:11:55PM +0100, Peter Maydell wrote:
 On Wed, 8 Jul 2020 at 18:36, Eduardo Habkost  wrote:
 >
 > On Wed, Jul 08, 2020 at 06:09:49PM +0100, Peter Maydell wrote:
 > > Exactly. It appears that there's a bug in our mechanisms,
 > > which is why I'm suggesting that the right thing is
 > > to fix that bug rather than marking the CPU as halted
 > > earlier in the reset process so that the KVM_RUN happens
 > > to do nothing...
 >
 > I agree this is necessary, but it doesn't seem sufficient.
 >
 > Having cpu_reset() set halted=0 on spapr (and probably other
 > machines) is also a bug, as it could still trigger unwanted
 > KVM_RUN when cpu_reset() returns (and before machine code sets
 > halted=1).

 The Arm handling of starting-halted sets halted=1 within cpu_reset,
 based on whether the CPU object was created with a
 "start-powered-off" property.
>>>
>>> Making this mechanism generic sounds like a good idea.
>>
>> I'll take a stab at doing that and using it for the spapr machine.
>>
 I'm not sure in practice that anything can get in asynchronously
 and cause a KVM_RUN in between spapr_reset_vcpu() calling
 cpu_reset() and it setting cs->halted (and the other stuff),
 though. This function ought to be called with the iothread
 lock held, so KVM_RUN will only happen if it calls some
 other function which incorrectly lets the CPU run.
>>>
>>> Yeah, maybe it won't happen in practice.  It just seems fragile.
>>> The same way ppc_cpu_reset() kicked the CPU by accident, code
>>> outside cpu_reset() might one day kick the CPU by accident before
>>> setting halted=1.
>>
>> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug.
>> Both of them are before cpu_reset() and ppc_cpu_reset().
>>
>> Here's the backtrace for the first of them (redacted for clarity):
>>
>> #0  in cpu_resume ()
>> #1  in cpu_common_realizefn ()
>> #2  in ppc_cpu_realize ()
>> #3  in device_set_realized ()
>> #4  in property_set_bool ()
>> #5  in object_property_set ()
>> #6  in object_property_set_qobject ()
>> #7  in object_property_set_bool ()
>> #8  in qdev_realize ()
> 
>> #18 in qmp_device_add ()
>
> Is this a hotplug event?

Yes, the way I reproduce the problem is starting a pseries guest with
`-smp 2,maxcpus=32,sockets=1,cores=16,threads=2` and then use qmp-shell to
send the command:

device_add id=device-2 driver=host-spapr-cpu-core core-id=2 node-id=0

>> Here's the second:
>>
>> #0  in qemu_cpu_kick_thread ()
>> #1  in qemu_cpu_kick ()
>> #2  in queue_work_on_cpu ()
>> #3  in async_run_on_cpu ()
>> #4  in tlb_flush_by_mmuidx ()
>> #5  in tlb_flush ()
>> #6  in ppc_tlb_invalidate_all ()
>
> FWIW tcg_flush_softmmu_tlb handles a tlb_flush in the common reset code.

Ok, maybe KVM should be doing that too? Or maybe it does but pseries
isn't relying on it. I'll dig further.

--
Thiago Jung Bauermann
IBM Linux Technology Center



Re: [RFC PATCH 1/3] hw/i2c/smbus_eeprom: Set QOM parent

2020-07-10 Thread Eduardo Habkost
On Fri, Jul 10, 2020 at 11:12:33AM +0200, Philippe Mathieu-Daudé wrote:
> On 7/9/20 10:05 PM, Eduardo Habkost wrote:
> > On Fri, Jun 26, 2020 at 04:15:40PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 6/26/20 4:03 PM, BALATON Zoltan wrote:
> >>> On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
>  + Eduardo / Mark / Edgard / Alistair / Fred for QOM design.
> 
>  On 6/26/20 12:54 PM, BALATON Zoltan wrote:
> > On Fri, 26 Jun 2020, BALATON Zoltan wrote:
> >> On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
> >>> Suggested-by: Markus Armbruster 
> >>> Signed-off-by: Philippe Mathieu-Daudé 
> >>> ---
> >>> Aspeed change pending latest ARM pull-request, so meanwhile sending
> >>> as RFC.
> >>> ---
> >>> include/hw/i2c/smbus_eeprom.h |  9 ++---
> >>> hw/i2c/smbus_eeprom.c         | 13 
> >>> ++---
> >>> hw/mips/fuloong2e.c           |  2 +-
> >>> hw/ppc/sam460ex.c             
> >>> |  2 +-
> >>> 4 files changed, 18 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/include/hw/i2c/smbus_eeprom.h
> >>> b/include/hw/i2c/smbus_eeprom.h
> >>> index 68b0063ab6..037612 100644
> >>> --- a/include/hw/i2c/smbus_eeprom.h
> >>> +++ b/include/hw/i2c/smbus_eeprom.h
> >>> @@ -26,9 +26,12 @@
> >>> #include "exec/cpu-common.h"
> >>> #include "hw/i2c/i2c.h"
> >>>
> >>> -void smbus_eeprom_init_one(I2CBus *bus, uint8_t address, uint8_t
> >>> *eeprom_buf);
> >>> -void smbus_eeprom_init(I2CBus *bus, int nb_eeprom,
> >>> -                 
> >>>       const uint8_t *eeprom_spd, int size);
> >>> +void smbus_eeprom_init_one(Object *parent_obj, const char
> >>> *child_name,
> >>> +                 
> >>>           I2CBus *smbus, uint8_t address,
> >>> +                 
> >>>           uint8_t *eeprom_buf);
> >>> +void smbus_eeprom_init(Object *parent_obj, const char
> >>> *child_name_prefix,
> >>> +                 
> >>>       I2CBus *smbus, int nb_eeprom,
> >>> +                 
> >>>       const uint8_t *eeprom_spd, int
> >>> eeprom_spd_size);
> >>
> >> Keeping I2CBus *smbus and uint8_t address as first parameters before
> >> parent_obj and name looks better to me. These functions still operate
> >> on an I2Cbus so could be regarded as methods of I2CBus therefore first
> >> parameter should be that.
> >
> > Also isn't parent_obj is the I2Cbus itself? Why is that need to be
> > passed? The i2c_init_bus() also takes parent and name params so both
> > I2Cbus and it's parent should be available as parents of the new I2C
> > device here without more parameters. What am I missing here?
> 
>  This is where I'm confused too and what I want to resolve with this
>  RFC series :)
> 
>  The SPD EEPROM is soldered on the DIMM module. The DIMM exposes the
>  memory address/data pins and the i2c pins. We plug DIMMs on a
>  (mother)board.
> 
>  I see the DIMM module being the parent. As we don't model it in QOM,
>  I used the MemoryRegion (which is what the SPD is describing).
> 
>  We could represent the DIMM as a container of DRAM + SPD EEPROM, but
>  it makes the modeling slightly more complex. The only benefit is a
>  clearer modeling.
> 
>  I'm not sure why the I2C bus is expected to be the parent. Maybe an
>  old wrong assumption?
> >>>
> >>> I guess it's a question of what the parent should mean? Is it parent of
> >>> the object in which case it's the I2CBus (which is kind of logical view
> >>> of the object tree modelling the machine) or the parent of the thing
> >>> modelled in the machine (which is physical view of the machine
> >>> components) then it should be the RAM module. The confusion probably
> >>> comes from this question not answered. Also the DIMM module is not
> >>> modelled so when you assign SPD eeproms to memory region it could be
> >>> multiple SPD eeproms will be parented by a single RAM memory region
> >>> (possibly not covering it fully as in the mac_oldworld or sam460ex case
> >>> discussed in another thread). This does not seem too intuitive.
> >>
> >> From the bus perspective, requests are sent hoping for a device to
> >> answer to the requested address ("Hello, do I have children? Hello?
> >> Anybody here?"), if nobody is here, the request timeouts.
> >> So there is not really a strong family relationship here.
> >>
> >> If you unplug a DIMM, you 

Re: [PATCH] tests: improve performance of device-introspect-test

2020-07-10 Thread Markus Armbruster
Daniel P. Berrangé  writes:

> Total execution time with "-m slow" and x86_64 QEMU, drops from 3
> minutes 15 seconds, down to 54 seconds.
>
> Individual tests drop from 17-20 seconds, down to 3-4 seconds.

Nice!

A few observations on this test (impatient readers may skip to
"Conclusions"):

* Your testing rig is a fair bit faster than mine.

* Run time is almost completely spent in the
  /TARGET/device/introspect/concrete/*/* cases.

  Each such case iterates over all known device types.  Each iteration
  introspects one device.

  Cases concrete/*/M use machine type M.  The test ignores "old"
  versioned machine types; see qtest_is_old_versioned_machine().

  Cases concrete/nodefaults/* use -nodefaults, cases concrete/defaults/*
  don't.

* The number of known device types varies between targets from 33
  (tricore) to several hundreds (x86_64+i386: 421, ppc 593, arm 667,
  aarch64 680, ppc64 689).  Median is 215, sum is 7485.

  Aside: why on earth do we need 334 ppc64 CPUs?

* The number of tested machine types varies between targets from two to
  over 60 (arm 62, aarch64 66).  Median is 4, sum is 286.

* Without -m slow, only /TARGET/device/introspect/concrete/defaults/none
  is run.

  This tests #devices introspections, i.e. between 33 and 689 depending
  on target, 7485 for all targets together.

  Run times for master (commit eb2c66b10e) vary for me from well under
  one second to almost 6 seconds, with x86_64 and i386 the slowest,
  followed by aarch64, arm, ppc64, pcc. s390x.  This is what "make
  check" runs.  Total run time for all 32 targets is ~72s.  That's
  roughly 100 introspections per second.

  Note I measured manual runs, like so:

  $ QEMU_AUDIO_DRV=none QTEST_QEMU_BINARY="$TARGET-softmmu/qemu-system-$TARGET" 
QTEST_QEMU_IMG=qemu-img time tests/qtest/device-introspect-test

* With -m slow, we run all cases.  This is relatively recent: commit
  410573aa2c 'tests/device-introspect: Test with all machines, not only
  with "none"'.  The commit message explains:

Certain device introspection crashes used to only happen if you were
using a certain machine, e.g. if the machine was using serial_hd() or
nd_table[], and a device was trying to use these in its instance_init
function, too.

  Aside: these are things an instance_init() should not do.  But
  catching things that should not be done is one reason for having
  tests.  This one is an awfully expensive catcher, but until someone
  has better ideas...

  With -m slow, we test 2 * #machines * #devices introspections,
  i.e. from 132 (tricore) to over 10k (ppc 13046, ppc64 23426, arm
  82708, aarch64 89760).  Median is ~1600, sum is ~260k.

  Except we actually test just 89k now, because the test *fails* for arm
  and aarch64 after some 500 introspections: introspecting device
  msf2-soc with machine ast2600-evb makes QEMU terminate unsuccessfully
  with "Unsupported NIC model: ftgmac100".  Cause: m2sxxx_soc_initfn()
  calls qemu_check_nic_model().  Goes back to commit 05b7374a58 "msf2:
  Add EMAC block to SmartFusion2 SoC", merged some ten weeks ago.  This
  is exactly the kind of mistake the test is designed to catch.  How
  come it wasn't?  Possibly due to unlucky combination with the slowdown
  discussed in the next item (but that was less than four weeks ago).
  I'll post a patch to fix the bug.

  Run time increases even more than the number of introspections.  Total
  run time for all targets is around one hour, which is less than 25
  introspections per second.  ppc64 is the worst by far: 26 *minutes*
  for 23k tests, less than 15 introspections per second.  sparc and rx
  achieve a paltry 5 introspections per second.

* Recent commit e8c9e65816 'qom: Make "info qom-tree" show children
  sorted' made things worse.  Reverting it cuts run time without -m slow
  from 72s to 43s (100 and 170 introspections per second, respectively),
  and with -m slow from one hour to 6 minutes (24 and 250 introspections
  per second, respectively).

  The speedup with -m slow varies much between targets: from less than 2
  (cris, moxie, nios2, or1k, s390x, tricore, unicore32) to more than 20
  (ppc64, rx, sparc).  The most sluggish introspections profit the most.

  Paolo pointed out that the sorting is n^2.  True.  I did not expect n
  to be large enough for that to matter.  What's being sorted is the
  children of each node in the QOM composition tree.  The number of
  children is almost always small.  The performance difference proves
  enough of them are large enough for the sorting to matter.  The
  variance between machine types suggests some machines have nodes with
  a particularly large number of children.

* I found a memory leak in the same commit.  Plugging it makes the test
  slightly slower for me.  I'll post the fix.

* Since some of the n appear to be large enough for n^2 to matter, I
  tried sorting more efficiently.

  With g_queue_insert_sorted() replaced by g_queue_push_head() &
  g_queue_sort(), the test without 

Re: [PATCH] cpu: Add starts_halted() method

2020-07-10 Thread Thiago Jung Bauermann


Philippe Mathieu-Daudé  writes:

> On 7/9/20 5:26 AM, Thiago Jung Bauermann wrote:
>>
>> Thiago Jung Bauermann  writes:
>>
>>> I'm seeing the vcpu being KVM_RUN'd too early twice during hotplug.
>>> Both of them are before cpu_reset() and ppc_cpu_reset().
>>
>> Hm, rereading the message obviously the above is partially wrong. The
>> second case happens during ppc_cpu_reset().
>>
>>> Here's the second:
>>>
>>> #0  in qemu_cpu_kick_thread ()
>>> #1  in qemu_cpu_kick ()
>>> #2  in queue_work_on_cpu ()
>>> #3  in async_run_on_cpu ()
>>> #4  in tlb_flush_by_mmuidx ()
>>> #5  in tlb_flush ()
>>> #6  in ppc_tlb_invalidate_all ()
>>> #7  in ppc_cpu_reset ()
>>> #8  in device_transitional_reset ()
>>> #9  in resettable_phase_hold ()
>>> #10 in resettable_assert_reset ()
>>> #11 in device_set_realized ()
>
> Dunno if related, might be helpful:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg686477.html

Yes, it's helpful. Thanks!

So is was it resolved whether it's appropriate to do a cpu_reset()
within realize?

Is the core of the problem that device_set_realize() ends up calling
ppc_cpu_reset()?

--
Thiago Jung Bauermann
IBM Linux Technology Center



[Bug 1856335] Re: Cache Layout wrong on many Zen Arch CPUs

2020-07-10 Thread Jan Klos
Yep, I read the Reddit thread, had no idea this was possible.

Still, both solutions are ugly workarounds and it would be nice to fix
this properly. But at least I don't have to patch and compile QEMU on my
own anymore.

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

Title:
  Cache Layout wrong on many Zen Arch CPUs

Status in QEMU:
  New

Bug description:
  AMD CPUs have L3 cache per 2, 3 or 4 cores. Currently, TOPOEXT seems
  to always map Cache ass if it was an 4-Core per CCX CPU, which is
  incorrect, and costs upwards 30% performance (more realistically 10%)
  in L3 Cache Layout aware applications.

  Example on a 4-CCX CPU (1950X /w 8 Cores and no SMT):

    
  EPYC-IBPB
  AMD
  

  In windows, coreinfo reports correctly:

    Unified Cache 1, Level 3,8 MB, Assoc  16, LineSize  64
    Unified Cache 6, Level 3,8 MB, Assoc  16, LineSize  64

  On a 3-CCX CPU (3960X /w 6 cores and no SMT):

   
  EPYC-IBPB
  AMD
  

  in windows, coreinfo reports incorrectly:

  --  Unified Cache  1, Level 3,8 MB, Assoc  16, LineSize  64
  **  Unified Cache  6, Level 3,8 MB, Assoc  16, LineSize  64

  Validated against 3.0, 3.1, 4.1 and 4.2 versions of qemu-kvm.

  With newer Qemu there is a fix (that does behave correctly) in using the dies 
parameter:
   

  The problem is that the dies are exposed differently than how AMD does
  it natively, they are exposed to Windows as sockets, which means, that
  if you are nto a business user, you can't ever have a machine with
  more than two CCX (6 cores) as consumer versions of Windows only
  supports two sockets. (Should this be reported as a separate bug?)

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



[PATCH] linux-user: Add several IFTUN ioctls

2020-07-10 Thread Josh Kunz
This change includes most widely-available if_tun ioctls that are
integer typed.

Tested by compiling all linux-user emulators. This patch has also been
used successfully to run several binaries that utilize these ioctls for
several months.

Linux Header:
https://github.com/torvalds/linux/blob/dcde237b9b0eb1d19306e6f48c0a4e058907619f/include/uapi/linux/if_tun.h#L31

Signed-off-by: Josh Kunz 
---
 linux-user/ioctls.h   | 20 
 linux-user/syscall.c  |  1 +
 linux-user/syscall_defs.h | 21 +
 3 files changed, 42 insertions(+)

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 0713ae1311..9b4a67fe84 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -593,3 +593,23 @@
   IOCTL(KCOV_DISABLE, 0, TYPE_NULL)
   IOCTL(KCOV_INIT_TRACE, IOC_R, TYPE_ULONG)
 #endif
+
+  IOCTL(TUNGETFEATURES  , IOC_R , TYPE_INT)
+  IOCTL(TUNGETIFF   , IOC_R , TYPE_INT)
+  IOCTL(TUNGETSNDBUF, IOC_R , TYPE_INT)
+  IOCTL(TUNGETVNETHDRSZ , IOC_R , TYPE_INT)
+  IOCTL(TUNGETVNETLE, IOC_R , TYPE_INT)
+  IOCTL(TUNSETDEBUG , IOC_W , TYPE_INT)
+  IOCTL(TUNSETGROUP , IOC_W , TYPE_INT)
+  IOCTL(TUNSETIFF   , IOC_W , TYPE_INT)
+  IOCTL(TUNSETIFINDEX   , IOC_W , TYPE_INT)
+  IOCTL(TUNSETLINK  , IOC_W , TYPE_INT)
+  IOCTL(TUNSETNOCSUM, IOC_W , TYPE_INT)
+  IOCTL(TUNSETOFFLOAD   , IOC_W , TYPE_INT)
+  IOCTL(TUNSETOWNER , IOC_W , TYPE_INT)
+  IOCTL(TUNSETPERSIST   , IOC_W , TYPE_INT)
+  IOCTL(TUNSETQUEUE , IOC_W , TYPE_INT)
+  IOCTL(TUNSETSNDBUF, IOC_W , TYPE_INT)
+  IOCTL(TUNSETTXFILTER  , IOC_W , TYPE_INT)
+  IOCTL(TUNSETVNETHDRSZ , IOC_W , TYPE_INT)
+  IOCTL(TUNSETVNETLE, IOC_W , TYPE_INT)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 98ea86ca81..4ad4b36a84 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -58,6 +58,7 @@
 #include 
 #include 
 #include 
+#include 
 #ifdef CONFIG_TIMERFD
 #include 
 #endif
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 3c261cff0e..833ef68faf 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -859,6 +859,27 @@ struct target_rtc_pll_info {
 #define TARGET_SIOCSIFPFLAGS   0x8934  /* set extended flags  
*/
 #define TARGET_SIOCGIFPFLAGS   0x8935  /* get extended flags  
*/
 
+/* if_tun ioctls */
+#define TARGET_TUNGETFEATURES   TARGET_IOR('T', 207, unsigned int)
+#define TARGET_TUNGETIFFTARGET_IOR('T', 210, unsigned int)
+#define TARGET_TUNGETSNDBUF TARGET_IOR('T', 211, int)
+#define TARGET_TUNGETVNETHDRSZ  TARGET_IOR('T', 215, int)
+#define TARGET_TUNGETVNETLE TARGET_IOR('T', 221, int)
+#define TARGET_TUNSETDEBUG  TARGET_IOW('T', 201, int)
+#define TARGET_TUNSETGROUP  TARGET_IOW('T', 206, int)
+#define TARGET_TUNSETIFFTARGET_IOW('T', 202, int)
+#define TARGET_TUNSETIFINDEXTARGET_IOW('T', 218, unsigned int)
+#define TARGET_TUNSETLINK   TARGET_IOW('T', 205, int)
+#define TARGET_TUNSETNOCSUM TARGET_IOW('T', 200, int)
+#define TARGET_TUNSETOFFLOADTARGET_IOW('T', 208, unsigned int)
+#define TARGET_TUNSETOWNER  TARGET_IOW('T', 204, int)
+#define TARGET_TUNSETPERSISTTARGET_IOW('T', 203, int)
+#define TARGET_TUNSETQUEUE  TARGET_IOW('T', 217, int)
+#define TARGET_TUNSETSNDBUF TARGET_IOW('T', 212, int)
+#define TARGET_TUNSETTXFILTER   TARGET_IOW('T', 209, unsigned int)
+#define TARGET_TUNSETVNETHDRSZ  TARGET_IOW('T', 216, int)
+#define TARGET_TUNSETVNETLE TARGET_IOW('T', 220, int)
+
 /* Bridging control calls */
 #define TARGET_SIOCGIFBR   0x8940  /* Bridging support 
*/
 #define TARGET_SIOCSIFBR   0x8941  /* Set bridging options 
*/
-- 
2.27.0.383.g050319c2ae-goog




Re: [PATCH v5 19/20] tests/acpi: add microvm test

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:55 +0200
Gerd Hoffmann  wrote:

> Signed-off-by: Gerd Hoffmann 
> ---
>  tests/qtest/bios-tables-test.c | 15 +++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index 24b715dce780..b5b98d5c0742 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -1007,6 +1007,20 @@ static void test_acpi_virt_tcg_memhp(void)
>  
>  }
>  
> +static void test_acpi_microvm_tcg(void)
> +{
> +test_data data;
> +
> +memset(, 0, sizeof(data));
> +data.machine = "microvm";

> +data.required_struct_types = base_required_struct_types;
> +data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
I vaguely recall this belongs to smbios tables, does actually microvm provide 
them?

> +data.blkdev = "virtio-blk-device";
> +test_acpi_one(" -machine microvm,acpi=on,rtc=off",
> +  );
> +free_test_data();
> +}
> +
>  static void test_acpi_virt_tcg_numamem(void)
>  {
>  test_data data = {
> @@ -1118,6 +1132,7 @@ int main(int argc, char *argv[])
>  qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
>  qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
>  qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
> +qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
>  } else if (strcmp(arch, "aarch64") == 0) {
>  qtest_add_func("acpi/virt", test_acpi_virt_tcg);
>  qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);




Re: [PATCH v5 18/20] tests/acpi: allow override blkdev

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:54 +0200
Gerd Hoffmann  wrote:

> microvm needs virtio-blk instead of ide.
> 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Igor Mammedov 

> ---
>  tests/qtest/bios-tables-test.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
> index c315156858f4..24b715dce780 100644
> --- a/tests/qtest/bios-tables-test.c
> +++ b/tests/qtest/bios-tables-test.c
> @@ -72,6 +72,7 @@ typedef struct {
>  const char *variant;
>  const char *uefi_fl1;
>  const char *uefi_fl2;
> +const char *blkdev;
>  const char *cd;
>  const uint64_t ram_start;
>  const uint64_t scan_len;
> @@ -635,9 +636,10 @@ static void test_acpi_one(const char *params, test_data 
> *data)
>  args = g_strdup_printf("-machine %s,kernel-irqchip=off %s -accel tcg 
> "
>  "-net none -display none %s "
>  "-drive id=hd0,if=none,file=%s,format=raw "
> -"-device ide-hd,drive=hd0 ",
> +"-device %s,drive=hd0 ",
>   data->machine, data->tcg_only ? "" : "-accel kvm",
> - params ? params : "", disk);
> + params ? params : "", disk,
> + data->blkdev ?: "ide-hd");
>  }
>  
>  data->qts = qtest_init(args);




Re: [PATCH v5 16/20] microvm: wire up hotplug

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:52 +0200
Gerd Hoffmann  wrote:

> The cpu hotplug code handles the initialization of coldplugged cpus
> too, so it is needed even in case cpu hotplug is not supported.
> 
> Wire cpu hotplug up for microvm.
> Without this we get a broken MADT table.
> 
> Signed-off-by: Gerd Hoffmann 

Blame is on me for calling it hotplug, HotplugHandlerClass is basically
a set of hooks to wire things up regardless if it's hotplug or coldplug.
In hindsight it was obvious at the time it was introduced.

Reviewed-by: Igor Mammedov 

> ---
>  hw/i386/microvm.c | 42 ++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 3d8a66cfc3ac..a5b16b728f9f 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -320,6 +320,39 @@ static void microvm_fix_kernel_cmdline(MachineState 
> *machine)
>  g_free(cmdline);
>  }
>  
> +static void microvm_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> +   DeviceState *dev, Error **errp)
> +{
> +x86_cpu_pre_plug(hotplug_dev, dev, errp);
> +}
> +
> +static void microvm_device_plug_cb(HotplugHandler *hotplug_dev,
> +   DeviceState *dev, Error **errp)
> +{
> +x86_cpu_plug(hotplug_dev, dev, errp);
> +}
> +
> +static void microvm_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> +error_setg(errp, "unplug not supported by microvm");
> +}
> +
> +static void microvm_device_unplug_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> +error_setg(errp, "unplug not supported by microvm");
> +}
> +
> +static HotplugHandler *microvm_get_hotplug_handler(MachineState *machine,
> +   DeviceState *dev)
> +{
> +if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
> +return HOTPLUG_HANDLER(machine);
> +}
> +return NULL;
> +}
> +
>  static void microvm_machine_state_init(MachineState *machine)
>  {
>  MicrovmMachineState *mms = MICROVM_MACHINE(machine);
> @@ -503,6 +536,7 @@ static void microvm_machine_initfn(Object *obj)
>  static void microvm_class_init(ObjectClass *oc, void *data)
>  {
>  MachineClass *mc = MACHINE_CLASS(oc);
> +HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
>  mc->init = microvm_machine_state_init;
>  
> @@ -523,6 +557,13 @@ static void microvm_class_init(ObjectClass *oc, void 
> *data)
>  /* Machine class handlers */
>  mc->reset = microvm_machine_reset;
>  
> +/* hotplug (for cpu coldplug) */
> +mc->get_hotplug_handler = microvm_get_hotplug_handler;
> +hc->pre_plug = microvm_device_pre_plug_cb;
> +hc->plug = microvm_device_plug_cb;
> +hc->unplug_request = microvm_device_unplug_request_cb;
> +hc->unplug = microvm_device_unplug_cb;
> +
>  object_class_property_add(oc, MICROVM_MACHINE_PIC, "OnOffAuto",
>microvm_machine_get_pic,
>microvm_machine_set_pic,
> @@ -572,6 +613,7 @@ static const TypeInfo microvm_machine_info = {
>  .class_size= sizeof(MicrovmMachineClass),
>  .class_init= microvm_class_init,
>  .interfaces = (InterfaceInfo[]) {
> + { TYPE_HOTPLUG_HANDLER },
>   { }
>  },
>  };




Re: [PATCH v7 17/47] block: Re-evaluate backing file handling in reopen

2020-07-10 Thread Andrey Shinkevich

On 25.06.2020 18:21, Max Reitz wrote:

Reopening a node's backing child needs a bit of special handling because
the "backing" child has different defaults than all other children
(among other things).  Adding filter support here is a bit more
difficult than just using the child access functions.  In fact, we often
have to directly use bs->backing because these functions are about the
"backing" child (which may or may not be the COW backing file).

Signed-off-by: Max Reitz 
---
  block.c | 46 ++
  1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 712230ef5c..8131d0b5eb 100644
--- a/block.c
+++ b/block.c
@@ -4026,26 +4026,56 @@ static int bdrv_reopen_parse_backing(BDRVReopenState 
*reopen_state,
  }
  }
  
+/*

+ * Ensure that @bs can really handle backing files, because we are
+ * about to give it one (or swap the existing one)
+ */
+if (bs->drv->is_filter) {
+/* Filters always have a file or a backing child */
+if (!bs->backing) {
+error_setg(errp, "'%s' is a %s filter node that does not support a 
"
+   "backing child", bs->node_name, bs->drv->format_name);
+return -EINVAL;
+}
+} else if (!bs->drv->supports_backing) {
+error_setg(errp, "Driver '%s' of node '%s' does not support backing "
+   "files", bs->drv->format_name, bs->node_name);
+return -EINVAL;
+}
+
  /*
   * Find the "actual" backing file by skipping all links that point
   * to an implicit node, if any (e.g. a commit filter node).
+ * We cannot use any of the bdrv_skip_*() functions here because
+ * those return the first explicit node, while we are looking for
+ * its overlay here.
   */
  overlay_bs = bs;
-while (backing_bs(overlay_bs) && backing_bs(overlay_bs)->implicit) {
-overlay_bs = backing_bs(overlay_bs);
+while (bdrv_filter_or_cow_bs(overlay_bs) &&
+   bdrv_filter_or_cow_bs(overlay_bs)->implicit)
+{
+overlay_bs = bdrv_filter_or_cow_bs(overlay_bs);
  }


I believe that little optimization would work properly:


for (BlockDriverState *below_bs = bdrv_filter_or_cow_bs(overlay_bs);
   below_bs && below_bs->implicit;
   below_bs = bdrv_filter_or_cow_bs(overlay_bs)) {
 overlay_bs = below_bs;
}
  
  /* If we want to replace the backing file we need some extra checks */

-if (new_backing_bs != backing_bs(overlay_bs)) {
+if (new_backing_bs != bdrv_filter_or_cow_bs(overlay_bs)) {
  /* Check for implicit nodes between bs and its backing file */
  if (bs != overlay_bs) {
  error_setg(errp, "Cannot change backing link if '%s' has "
 "an implicit backing file", bs->node_name);
  return -EPERM;
  }
-/* Check if the backing link that we want to replace is frozen */
-if (bdrv_is_backing_chain_frozen(overlay_bs, backing_bs(overlay_bs),
- errp)) {
+/*
+ * Check if the backing link that we want to replace is frozen.
+ * Note that
+ * bdrv_filter_or_cow_child(overlay_bs) == overlay_bs->backing,
+ * because we know that overlay_bs == bs, and that @bs
+ * either is a filter that uses ->backing or a COW format BDS
+ * with bs->drv->supports_backing == true.
+ */
+if (bdrv_is_backing_chain_frozen(overlay_bs,
+ child_bs(overlay_bs->backing), errp))

What would be wrong with bdrv_filter_or_cow_bs(overlay_bs) here?

+{
  return -EPERM;
  }
  reopen_state->replace_backing_bs = true;
@@ -4196,7 +4226,7 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
   * its metadata. Otherwise the 'backing' option can be omitted.
   */
  if (drv->supports_backing && reopen_state->backing_missing &&
-(backing_bs(reopen_state->bs) || reopen_state->bs->backing_file[0])) {

= BlockDriverState*

+(reopen_state->bs->backing || reopen_state->bs->backing_file[0])) {


= BdrvChild*

Are we OK with that?


  error_setg(errp, "backing is missing for '%s'",
 reopen_state->bs->node_name);
  ret = -EINVAL;
@@ -4337,7 +4367,7 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
   * from bdrv_set_backing_hd()) has the new values.
   */
  if (reopen_state->replace_backing_bs) {
-BlockDriverState *old_backing_bs = backing_bs(bs);
+BlockDriverState *old_backing_bs = child_bs(bs->backing);
  assert(!old_backing_bs || !old_backing_bs->implicit);
  /* Abort the permission update on the backing bs we're detaching */
  if (old_backing_bs) {




Re: [PATCH v5 15/20] x86: move cpu plug from pc to x86

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:51 +0200
Gerd Hoffmann  wrote:

> The cpu hotplug code handles the initialization of coldplugged cpus
> too, so it is needed even in case cpu hotplug is not supported.
> 
> Move the code from pc to x86, so microvm can use it.

it's very convinient to have all hotplug handles in one place,
so maybe move along unplug[_request] variants as well?

> Signed-off-by: Gerd Hoffmann 
> ---
>  include/hw/i386/x86.h |   6 ++
>  hw/i386/pc.c  | 234 ++
>  hw/i386/x86.c | 222 +++
>  3 files changed, 234 insertions(+), 228 deletions(-)
> 
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index de74c831c3ab..23c964471802 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -102,6 +102,12 @@ CpuInstanceProperties 
> x86_cpu_index_to_props(MachineState *ms,
>   unsigned cpu_index);
>  int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx);
>  const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms);
> +CPUArchId *x86_find_cpu_slot(MachineState *ms, uint32_t id, int *idx);
> +void x86_rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count);
> +void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp);
> +void x86_cpu_plug(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp);
>  
>  void x86_bios_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw);
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 98d29ead09b0..14036fcd4e3a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -798,19 +798,6 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, 
> Error **errp)
>  }
>  }
>  
> -static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
> -{
> -if (cpus_count > 0xff) {
> -/* If the number of CPUs can't be represented in 8 bits, the
> - * BIOS must use "FW_CFG_NB_CPUS". Set RTC field to 0 just
> - * to make old BIOSes fail more predictably.
> - */
> -rtc_set_memory(rtc, 0x5f, 0);
> -} else {
> -rtc_set_memory(rtc, 0x5f, cpus_count - 1);
> -}
> -}
> -
>  static
>  void pc_machine_done(Notifier *notifier, void *data)
>  {
> @@ -820,7 +807,7 @@ void pc_machine_done(Notifier *notifier, void *data)
>  PCIBus *bus = pcms->bus;
>  
>  /* set the number of CPUs */
> -rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
> +x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
>  
>  if (bus) {
>  int extra_hosts = 0;
> @@ -1373,62 +1360,6 @@ static void pc_memory_unplug(HotplugHandler 
> *hotplug_dev,
>  error_propagate(errp, local_err);
>  }
>  
> -static int pc_apic_cmp(const void *a, const void *b)
> -{
> -   CPUArchId *apic_a = (CPUArchId *)a;
> -   CPUArchId *apic_b = (CPUArchId *)b;
> -
> -   return apic_a->arch_id - apic_b->arch_id;
> -}
> -
> -/* returns pointer to CPUArchId descriptor that matches CPU's apic_id
> - * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
> - * entry corresponding to CPU's apic_id returns NULL.
> - */
> -static CPUArchId *pc_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
> -{
> -CPUArchId apic_id, *found_cpu;
> -
> -apic_id.arch_id = id;
> -found_cpu = bsearch(_id, ms->possible_cpus->cpus,
> -ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
> -pc_apic_cmp);
> -if (found_cpu && idx) {
> -*idx = found_cpu - ms->possible_cpus->cpus;
> -}
> -return found_cpu;
> -}
> -
> -static void pc_cpu_plug(HotplugHandler *hotplug_dev,
> -DeviceState *dev, Error **errp)
> -{
> -CPUArchId *found_cpu;
> -Error *local_err = NULL;
> -X86CPU *cpu = X86_CPU(dev);
> -PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
> -
> -if (x86ms->acpi_dev) {
> -hotplug_handler_plug(x86ms->acpi_dev, dev, _err);
> -if (local_err) {
> -goto out;
> -}
> -}
> -
> -/* increment the number of CPUs */
> -x86ms->boot_cpus++;
> -if (x86ms->rtc) {
> -rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
> -}
> -if (x86ms->fw_cfg) {
> -fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
> -}
> -
> -found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
> -found_cpu->cpu = OBJECT(dev);
> -out:
> -error_propagate(errp, local_err);
> -}
>  static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
> @@ -1443,7 +1374,7 @@ static void pc_cpu_unplug_request_cb(HotplugHandler 
> *hotplug_dev,
>  goto out;
>  }
>  
> -pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, );
> +x86_find_cpu_slot(MACHINE(pcms), cpu->apic_id, );
>  assert(idx != -1);
>  if (idx == 0) {

Re: [PATCH v5 08/20] microvm/acpi: add minimal acpi support

2020-07-10 Thread Igor Mammedov
On Thu, 9 Jul 2020 14:33:32 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > > +scope = aml_scope("\\");
> > > +pkg = aml_package(4);
> > > +aml_append(pkg, aml_int(5)); /* SLEEP_CONTROL_REG.SLP_TYP */  
> > 
> > I'm not sure what does the comment refer to here.  
> 
> It's the register field the value gets written to.
> With full acpi this is PM1a_CNT.SLP_TYP, hw-reduced uses
> SLEEP_CONTROL_REG.SLP_TYP instead.  This is cut from pc/q35
> version + adapted for hw-reduced.
> 
> > Does this 5 match
> > the value IO handler tests against?  
> 
> Yes.  "5" for S5 state (aka poweroff).  Can add a #define.
> 
> > Below is from "7.3.4 System \_Sx states" right?  
> 
> "7.4.2 \_Sx (System States)" here (ACPI 6.3), guess that is the same.
> 
> > > +AcpiFadtData pmfadt = {
> > > +/*
> > > + * minimum version for ACPI_FADT_F_HW_REDUCED_ACPI,
> > > + * see acpi spec "4.1 Hardware-Reduced ACPI"  
> > 
> > Spec version - I'm guessing ACPI spec 5.0.  
> 
> 6.3
> 
> > And I think here is where you refer to
> > Table 5-34 Fixed ACPI Description Table (FADT) Format  
> 
> Table 5-33 FADT Format
> 
> > > + */
> > > +.rev = 5,
> > > +.minor_ver = 1,  
> > 
> > So 5.1 I am guessing just copied from virt/arm?  
> 
> Yes.
> 
> > > +.flags = ((1 << ACPI_FADT_F_HW_REDUCED_ACPI) |
> > > +  (1 << ACPI_FADT_F_RESET_REG_SUP)),
> > > +
> > > +/* Table 5-33 FADT Format -- SLEEP_CONTROL_REG */  
> > 
> > You need to use the earliest spec version that includes
> > a specific feature - and document which one it is.  
> 
> Phew.  Isn't it easier to just use table and field name then, so it is
> easy to find in whatever version of the spec you have at hand?  Also how
> can I figure the earliest spec version easily?
> 
> Sometimes the 6.3 spec documents which table version added specific
> fields, sometimes not ...
> 
> Is the table version synced with the acpi spec version?  Does DSDT v2
> mean the DSDT format was updated for ACPI 2.0 and hasn't changed since?

not necessarily, even if it's pain to check for earliest spec where
feature appeared, it's the best and the least ambiguos comment format to
refference spec, i.e. (do not count on chapter/table numbering/naming being
stable accross versions)

/* Spec name + ver: chapter/table where the feature is described */

something like:

/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefObjectType */
 
Aml *aml_object_type(Aml *object)  

> 
> > But the main poit is AcpiFadtData actually has nothing to do with
> > FADT format. It's an abstracted API   
> 
> The FADT is generated from AcpiFadtData.  There is a 1:1 relationship
> between most AcpiFadtData fields and FADT table entries.  This isn't
> what I would call "has nothing to do with" ...
> 
> > > +xsdt = tables_blob->len;
> > > +build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> > > +
> > > +/* RSDP is in FSEG memory, so allocate it separately */
> > > +{
> > > +AcpiRsdpData rsdp_data = {
> > > +/* Table 5-27 RSDP Structure */  
> > 
> > RSDP is since ACPI 2.0, table number there is different.  
> 
> References to ACPI 2.0 are almost useless.  ACPI 5.0 is the oldest
> version uefi.org offers for download.
all versions are at (starting from 1.0)
https://uefi.org/acpi/specs

> Guess that underlines the point I made above that referencing specific
> versions of the spec doesn't work very well ...

so far that worked well, effect of requiring spec+version was less
questions during review as reviewer doesn't have to guess where
it's documented.

> 
> take care,
>   Gerd
> 




Re: Updates on libcapstone?

2020-07-10 Thread Richard Henderson
On 7/9/20 6:07 AM, Philippe Mathieu-Daudé wrote:
> I remember you had a series related to capstone, but eventually there
> was a problem so you postponed it until some patches were merged
> upstream, do you remember?

I do.

My biggest problem with libcapstone is that it doesn't have an automated way to
merge from upstream llvm, so it doesn't get any new architecture updates.

What I think we should do is drop libcapstone entirely, and use libllvm
directly.  It's a bit more code to use, but llvm is where new architectures
(and architecture extensions) get merged.

A mere matter of coding...


r~



Re: [PATCH 0/2] keepalive default

2020-07-10 Thread Vladimir Sementsov-Ogievskiy

09.07.2020 20:14, Vladimir Sementsov-Ogievskiy wrote:

09.07.2020 18:34, Eric Blake wrote:

On 7/9/20 3:35 AM, Daniel P. Berrangé wrote:

On Wed, Jul 08, 2020 at 10:15:37PM +0300, Vladimir Sementsov-Ogievskiy wrote:

Hi all!

We understood, that keepalive is almost superfluous with default 2 hours
in /proc/tcp_keepalive_time. Forcing user to setup keepalive for the
whole system doesn't seem right, better setup it per-socket.


Adding the ability to explicitly configure the keepalive settings makes
sense for QEMU. Completely ignoring system defaults when no explicit
settings are given though is not valid IMHO.


We already have the ability to add per-socket keepalive (see commit aec21d3175, 
in 4.2).  I guess what you are trying to further do is determine whether the 
default value for that field, when not explicitly specified by the user, can 
have saner semantics (default off for chardev sockets, default on for nbd 
clients where retry was enabled).  But since you already have to explicitly 
opt-in to nbd retry, what's so hard about opting in to keepalive at the same 
time, other than more typing?  Given that the easiest way to do this is via a 
machine-coded generation of the command line or QMP command, it doesn't seem 
that hard to just keep things as they are with explicit opt-in to per-socket 
keepalive.



Hmm. Looking at the code, I remember that reconnect is not optional, it works by default 
now. The option we have is "reconnect-delay" which only specify, after how much 
seconds we'll automatically fail the requests, not retrying them (0 seconds by default). 
Still, NBD tries to reconnect in background anyway.

In our downstream we have now old version of nbd-reconnect interface and enabled non-zero 
"delay" by default.




And now we need to migrate to upstream code. Hmm. So I have several options:

1. Set a downstream default for reconnect-delay to be something like 5min. 
[most simple thing to do]
2. Set an upstream default to 5min. [needs a discussion, but may be useful]
3. Force all users (not customers I mean, but other teams (and even one another 
company)) to set reconnect-delay option. [just for completeness, I will not go 
this way]


So, what do you think about [2]? This includes:

 - non-zero reconnect-delay by default, so requests will wait some time for 
reconnect and retry
 - enabled keep-alive and some keep-alive default parameters, to not hang in 
recvmsg for a long time


Some other related ideas:

 - non-blocking nbd_open
   - move open to the coroutine
   - use non-blocking socket connect (for example use 
qio_channel_socket_connect_async, which runs connect in thread). Currently, if 
you try to blockdev-add some unavailable nbd host, vm hangs during connect() 
call which may be about a minute
 - make blockdev-add to be async qmp command (Kevin's series)
 - allow reconnect on open

also, recently I noted, that bdrv_close may hang due to reconnect: it does 
bdrv_flush, which waits for NBD to reconnect.. This seems not convenient, 
probably we should disable reconnect before bdrv_drained_begin() in 
bdrv_close().

--
Best regards,
Vladimir




Re: [PATCH 3/3] cpu-timers, icount: new modules

2020-07-10 Thread Claudio Fontana
On 7/10/20 8:33 AM, Cornelia Huck wrote:
> On Thu, 9 Jul 2020 20:46:56 +0200
> Claudio Fontana  wrote:
> 
>> On 7/9/20 8:38 PM, Claudio Fontana wrote:
>>> On 7/8/20 5:05 PM, Paolo Bonzini wrote:  
 On 08/07/20 17:00, Claudio Fontana wrote:  
>> Bisectable, 100% failure rate, etc. :(  Can you split the patch in
>> multiple parts, specifically separating any rename or introducing of
>> includes from the final file move?  
> Hi Paolo,
>
> will take a look!
>
> Is this captured by some travis / cirrus-ci / anything I can easily see 
> the result of?
>
>  

 Nope, unfortunately we don't have an s390 CI.  But if you can get your
 hands on one, just "./configure --target-list=s390x-softmmu && make &&
 make check-block" will show it.  
>>>
>>> So this is tricky, but I am making some progress after getting my hands on 
>>> one.
>>> Maybe if someone understands s390 keys better, I could be clued in.  
>>
>>
>> Also adding Cornelia to Cc:.
>>
>> Maybe the savevm_s390_storage_keys SaveVMHandlers etc assume that the icount 
>> state part of the vmstate is there?
> 
> I don't see anything that would deal with icount here. Adding Jason to
> cc: in case he has an idea. (I assume it would behave the same under
> KVM, as the only thing different are the internal callbacks.)

yes, same between tcg and kvm.

> 
>>
>>
>>>
>>> In short this goes away if I again set icount to enabled for qtest,
>>> basically ensuring that --enable-tcg is there and then reenabling icount.
>>>
>>> qtest was forcing icount and shift=0 by creating qemu options, in order to 
>>> misuse its counter feature,
>>> instead of using a separate counter.
>>>
>>> Removing that ugliness we end up with different behavior of save/load, 
>>> because vmstate will now suddenly not contain icount-related values anymore.
>>> What I do not understand is why this causes a problem because save should 
>>> just not store the icount state and load should just not load the icount 
>>> state,
>>> and why we die on the load of s390 keys state (it works just fine for other 
>>> architectures).
> 
> Yes, I don't really see why skeys is so special. No endianness stuff, I
> assume?


No, does not seem to be the issue.

I discovered a way simpler way to "fix" it: 

static bool icount_state_needed(void *opaque)
{
return 1;
}

Ie, making sure that the state is always saved/restored, even when unused.

Really weird.

I logged/debugged the vmstate code, and I can see that things seem symmetric 
between save and load when it comes to timers.

something puts 0s into the key somehow...







> 
>>>
>>> Here is a diff that makes the problem disappear, but needs --enable-tcg:
>>>
>>>
>>> 
>>> diff --git a/accel/qtest.c b/accel/qtest.c
>>> index 119d0f16a4..4cb16abc2c 100644
>>> --- a/accel/qtest.c
>>> +++ b/accel/qtest.c
>>> @@ -23,6 +23,12 @@
>>>  
>>>  static int qtest_init_accel(MachineState *ms)
>>>  {
>>> +QemuOpts *opts = qemu_opts_create(qemu_find_opts("icount"), NULL, 0,
>>> +  _abort);
>>> +qemu_opt_set(opts, "shift", "0", _abort);
>>> +icount_configure(opts, _abort);
>>> +qemu_opts_del(opts);
>>> +
>>>  return 0;
>>>  }
>>>  
>>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>>> index f39fd5270b..a5e788c86a 100644
>>> --- a/softmmu/vl.c
>>> +++ b/softmmu/vl.c
>>> @@ -2786,10 +2786,12 @@ static void configure_accelerators(const char 
>>> *progname)
>>>  error_report("falling back to %s", ac->name);
>>>  }
>>>  
>>> +/*
>>>  if (icount_enabled() && !tcg_enabled()) {
>>>  error_report("-icount is not allowed with hardware 
>>> virtualization");
>>>  exit(1);
>>> }
>>> +*/
>>>  }
>>>  
>>>  static void create_default_memdev(MachineState *ms, const char *path)
>>> 
>>>
>>> Without this patch, here is the full failure, maybe someone has a good 
>>> hint, otherwise I'll keep digging from here inside the s390-specific code.
>>>
>>> QA output created by 267
>>>
>>> === No block devices at all ===
>>>
>>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>> Testing:
>>> QEMU X.Y.Z monitor - type 'help' for more information
>>> (qemu) savevm snap0
>>> Error: No block device can accept snapshots
>>> (qemu) info snapshots
>>> No available block device supports snapshots
>>> (qemu) loadvm snap0
>>> Error: No block device supports snapshots
>>> (qemu) quit
>>>
>>>
>>> === -drive if=none ===
>>>
>>> Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
>>> Testing: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none
>>> QEMU X.Y.Z monitor - type 'help' for more information
>>> (qemu) savevm snap0
>>> Error: Device 'none0' is writable but does not support snapshots
>>> (qemu) info snapshots
>>> No available block device 

Re: [PULL 10/41] python/qemu: Add ConsoleSocket for optional use in QEMUMachine

2020-07-10 Thread John Snow



On 7/7/20 3:08 AM, Alex Bennée wrote:
> From: Robert Foley 
> 

Hi, this creates some pylint regressions, can they please be addressed
first?

> We add the ConsoleSocket object, which has a socket interface
> and which will consume all arriving characters on the
> socket, placing them into an in memory buffer.
> This will also provide those chars via recv() as
> would a regular socket.
> ConsoleSocket also has the option of dumping
> the console bytes to a log file.
> 
> We also give QEMUMachine the option of using ConsoleSocket
> to drain and to use for logging console to a file.
> By default QEMUMachine does not use ConsoleSocket.
> 
> This is added in preparation for use by basevm.py in a later commit.
> This is a workaround we found was needed for basevm.py since
> there is a known issue where QEMU will hang waiting
> for console characters to be consumed.
> 
> Cc: Eduardo Habkost 
> Cc: Cleber Rosa 
> Signed-off-by: Robert Foley 
> Reviewed-by: Peter Puhov 
> Acked-by: Alex Bennée 
> Tested-by: Philippe Mathieu-Daudé 
> Signed-off-by: Alex Bennée 
> Message-Id: <20200601211421.1277-9-robert.fo...@linaro.org>
> Message-Id: <20200701135652.1366-13-alex.ben...@linaro.org>
> 
> diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
> new file mode 100644
> index ..830cb7c62825
> --- /dev/null
> +++ b/python/qemu/console_socket.py
> @@ -0,0 +1,110 @@
> +#!/usr/bin/env python3

> +#
> +# This python module implements a ConsoleSocket object which is
> +# designed always drain the socket itself, and place
> +# the bytes into a in memory buffer for later processing.
> +#
> +# Optionally a file path can be passed in and we will also
> +# dump the characters to this file for debug.
> +#

Convert to a module docstring, please.

> +# Copyright 2020 Linaro
> +#
> +# Authors:
> +#  Robert Foley 
> +#
> +# This code is licensed under the GPL version 2 or later.  See
> +# the COPYING file in the top-level directory.
> +#
> +import asyncore
> +import socket
> +import threading

vvv

> +import io
> +import os
> +import sys

^^^ unused, remove

> +from collections import deque
> +import time
> +import traceback
> +

Add one more blank line here, for flake8.

> +class ConsoleSocket(asyncore.dispatcher):
> +
> +def __init__(self, address, file=None):
> +self._recv_timeout_sec = 300
> +self._buffer = deque()
> +self._asyncore_thread = None
> +self._sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
> +self._sock.connect(address)
> +self._logfile = None
> +if file:
> +self._logfile = open(file, "w")
> +asyncore.dispatcher.__init__(self, sock=self._sock)
> +self._open = True
> +self._thread_start()
> +
> +def _thread_start(self):
> +"""Kick off a thread to wait on the asyncore.loop"""
> +if self._asyncore_thread is not None:
> +return
> +self._asyncore_thread = threading.Thread(target=asyncore.loop,
> + kwargs={'timeout':1})

Add a space after the colon here.

> +self._asyncore_thread.daemon = True
> +self._asyncore_thread.start()
> +
> +def handle_close(self):
> +"""redirect close to base class"""
> +# Call the base class close, but not self.close() since
> +# handle_close() occurs in the context of the thread which
> +# self.close() attempts to join.
> +asyncore.dispatcher.close(self)
> +
> +def close(self):
> +"""Close the base object and wait for the thread to terminate"""
> +if self._open:
> +self._open = False
> +asyncore.dispatcher.close(self)
> +if self._asyncore_thread is not None:
> +thread, self._asyncore_thread = self._asyncore_thread, None
> +thread.join()
> +if self._logfile:
> +self._logfile.close()
> +self._logfile = None
> +
> +def handle_read(self):
> +"""process arriving characters into in memory _buffer"""
> +try:
> +data = asyncore.dispatcher.recv(self, 1)
> +# latin1 is needed since there are some chars
> +# we are receiving that cannot be encoded to utf-8
> +# such as 0xe2, 0x80, 0xA6.
> +string = data.decode("latin1")
> +except:
> +print("Exception seen.")
> +traceback.print_exc()
> +return

Please, no bare exceptions unless you re-raise the error.
If you want to handle the error, please handle the error in the caller
and not the shared library code.

As it is, I have no idea what errors you saw that might have warranted
this exclusion.

> +if self._logfile:
> +self._logfile.write("{}".format(string))
> +self._logfile.flush()
> +for c in string:
> +self._buffer.extend(c)
> +

Pylint complains about the usage of 'c' 

[Bug 1885332] Re: Error in user-mode calculation of ELF aux vector's AT_PHDR

2020-07-10 Thread Dmitry
@Langston  will do tomorrow. s390x ABI requires heavy changes to the
python script.

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

Title:
  Error in user-mode calculation of ELF aux vector's AT_PHDR

Status in QEMU:
  New

Bug description:
  
  I have an (admittedly strange) statically-linked ELF binary for Linux that 
runs just fine on top of the Linux kernel in QEMU full-system emulation, but 
crashes before main in user-mode emulation. Specifically, it crashes when 
initializing thread-local storage in glibc's _dl_aux_init, because it reads out 
a strange value from the AT_PHDR entry of the ELF aux vector.

  The binary has these program headers:

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  EXIDX  0x065874 0x00075874 0x00075874 0x00570 0x00570 R   0x4
  PHDR   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x00 0x0001 0x0001 0x65de8 0x65de8 R E 0x1
  LOAD   0x066b7c 0x00086b7c 0x00086b7c 0x02384 0x02384 RW  0x1
  NOTE   0x000114 0x00010114 0x00010114 0x00044 0x00044 R   0x4
  TLS0x066b7c 0x00086b7c 0x00086b7c 0x00010 0x00030 R   0x4
  GNU_STACK  0x00 0x 0x 0x0 0x0 RW  0x8
  GNU_RELRO  0x066b7c 0x00086b7c 0x00086b7c 0x00484 0x00484 R   0x1
  LOAD   0x07e000 0x00089000 0x00089000 0x03f44 0x03f44 R E 0x1000
  LOAD   0x098000 0x0003 0x0003 0x01000 0x01000 RW  0x1000

  If I build the Linux kernel with the following patch to the very end
  of create_elf_tables in fs/binfmt_elf.c

/* Put the elf_info on the stack in the right place.  */
elf_addr_t *my_auxv = (elf_addr_t *) mm->saved_auxv;
int i;
for (i = 0; i < 15; i++) {
  printk("0x%x = 0x%x", my_auxv[2*i], my_auxv[(2*i)+ 1]);
}
if (copy_to_user(sp, mm->saved_auxv, ei_index * sizeof(elf_addr_t)))
return -EFAULT;
return 0;

  and run it like this:

qemu-system-arm \
  -M versatilepb \
  -nographic \
  -dtb ./dts/versatile-pb.dtb \
  -kernel zImage \
  -M versatilepb \
  -m 128M \
  -append "earlyprintk=vga,keep" \
  -initrd initramfs

  after I've built the kernel initramfs like this (where "init" is the
  binary in question):

make ARCH=arm versatile_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all -j10
cp "$1" arch/arm/boot/init
cd arch/arm/boot
echo init | cpio -o --format=newc > initramfs

  then I get the following output. This is the kernel's view of the aux
  vector for this binary:

0x10 = 0x1d7
0x6 = 0x1000
0x11 = 0x64
0x3 = 0x90
0x4 = 0x20
0x5 = 0xb
0x7 = 0x0
0x8 = 0x0
0x9 = 0x101b8
0xb = 0x0
0xc = 0x0
0xd = 0x0
0xe = 0x0
0x17 = 0x0
0x19 = 0xbec62fb5

  However, if I run "qemu-arm -g 12345 binary" and use GDB to peek at
  the aux vector at the beginning of __libc_start_init (for example,
  using this Python GDB API script: https://gist.github.com/langston-
  barrett/5573d64ae0c9953e2fa0fe26847a5e1e), then I see the following
  values:

AT_PHDR = 0xae000
AT_PHENT = 0x20
AT_PHNUM = 0xb
AT_PAGESZ = 0x1000
AT_BASE = 0x0
AT_FLAGS = 0x0
AT_ENTRY = 0x10230
AT_UID = 0x3e9
AT_EUID = 0x3e9
AT_GID = 0x3e9
AT_EGID = 0x3e9
AT_HWCAP = 0x1fb8d7
AT_CLKTCK = 0x64
AT_RANDOM = -0x103c0
AT_HWCAP2 = 0x1f
AT_NULL = 0x0

  The crucial difference is in AT_PHDR (0x3), which is indeed the
  virtual address of the PHDR segment when the kernel calculates it, but
  is not when QEMU calculates it.

  qemu-arm --version
  qemu-arm version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.26)

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



Re: [RFC PATCH] configure: remove all dependencies on a (re)configure

2020-07-10 Thread Richard Henderson
On 7/8/20 10:53 AM, Alex Bennée wrote:
> The previous code was brittle and missed cases such as the mipn32
> variants which for some reason has the 64 bit syscalls. This leads to
> a number of binary targets having deps lines like:
> 
>   all.clang-sanitizer/mipsn32el-linux-user/linux-user/signal.d
>   140:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
>   455:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
> 
>   all.clang-sanitizer/mipsn32el-linux-user/linux-user/syscall.d
>   146:  /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
>   485:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
> 
> which in turn would trigger the re-generation of syscall_nr.h in the
> source tree (thanks to generic %/syscall_nr.h rules). The previous
> code attempts to clean it out but misses edge cases but fails.
> 
> After spending a day trying to understand how this was happening I'm
> unconvinced that there are not other such breakages possible with this
> "caching". As we add more auto-generated code to the build it is likely
> to trip up again. Apply a hammer to the problem.
> 
> Fixes: 91e5998f18 (which fixes 5f29856b852d and 4d6a835dea47)
> Signed-off-by: Alex Bennée 
> ---
>  configure | 15 ++-
>  1 file changed, 6 insertions(+), 9 deletions(-)

Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 0/7] riscv: Switch to use generic platform fw_dynamic type opensbi bios images

2020-07-10 Thread Alistair Francis
On Fri, Jul 10, 2020 at 11:59 AM Alistair Francis  wrote:
>
> On Thu, Jul 9, 2020 at 10:05 PM Bin Meng  wrote:
> >
> > From: Bin Meng 
> >
> > The RISC-V generic platform is a flattened device tree (FDT) based
> > platform where all platform specific functionality is provided based
> > on FDT passed by previous booting stage. The support was added in
> > the upstream OpenSBI v0.8 release recently.
> >
> > This series updates QEMU to switch to use generic platform of opensbi
> > bios images. With the recent fw_dynamic image support, let's replace
> > the fw_jump images with fw_dynamic ones too.
> >
> > The patch emails do not contain binary bits, please grab all updates
> > at https://github.com/lbmeng/qemu.git bios branch.
> >
> > Changes in v4:
> > - Remove old binaries in the Makefile for `make install` bisection
> >
> > Changes in v3:
> > - Change fw_jump to fw_dynamic in the make rules
> > - Change to fw_dynamic.bin for virt & sifive_u
> > - Change to fw_dynamic.elf for Spike
> > - Generate fw_dynamic images in the artifacts
> > - change fw_jump to fw_dynamic in the Makefile
> >
> > Changes in v2:
> > - new patch: configure: Create symbolic links for pc-bios/*.elf files
> > - Upgrade OpenSBI to v0.8 release
> > - Copy the ELF images too in the make rules
> > - Include ELF images in the artifacts
> > - new patch: Makefile: Ship the generic platform bios images for RISC-V
> >
> > Bin Meng (7):
> >   configure: Create symbolic links for pc-bios/*.elf files
> >   roms/opensbi: Upgrade from v0.7 to v0.8
> >   roms/Makefile: Build the generic platform for RISC-V OpenSBI firmware
> >   hw/riscv: Use pre-built bios image of generic platform for virt &
> > sifive_u
> >   hw/riscv: spike: Change the default bios to use generic platform image
> >   gitlab-ci/opensbi: Update GitLab CI to build generic platform
> >   Makefile: Ship the generic platform bios images for RISC-V
>
> Reviewed-by: Alistair Francis 

Whoops, wrong canned response.

Thanks!

Applied to riscv-to-apply.next

Alistair

>
> Alistair
>
> >
> >  .gitlab-ci.d/opensbi.yml   |  28 --
> >  Makefile   |   4 ++--
> >  configure  |   1 +
> >  hw/riscv/sifive_u.c|   4 ++--
> >  hw/riscv/spike.c   |   9 +--
> >  hw/riscv/virt.c|   4 ++--
> >  pc-bios/opensbi-riscv32-generic-fw_dynamic.bin | Bin 0 -> 62144 bytes
> >  pc-bios/opensbi-riscv32-generic-fw_dynamic.elf | Bin 0 -> 558668 bytes
> >  pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin   | Bin 49520 -> 0 bytes
> >  pc-bios/opensbi-riscv32-virt-fw_jump.bin   | Bin 49504 -> 0 bytes
> >  pc-bios/opensbi-riscv64-generic-fw_dynamic.bin | Bin 0 -> 70792 bytes
> >  pc-bios/opensbi-riscv64-generic-fw_dynamic.elf | Bin 0 -> 620424 bytes
> >  pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin   | Bin 57936 -> 0 bytes
> >  pc-bios/opensbi-riscv64-virt-fw_jump.bin   | Bin 57920 -> 0 bytes
> >  roms/Makefile  |  32 
> > -
> >  roms/opensbi   |   2 +-
> >  16 files changed, 35 insertions(+), 49 deletions(-)
> >  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
> >  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.elf
> >  delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
> >  delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
> >  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.bin
> >  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.elf
> >  delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
> >  delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin
> >
> > --
> > 2.7.4
> >
> >



Re: [PATCH v4 0/7] riscv: Switch to use generic platform fw_dynamic type opensbi bios images

2020-07-10 Thread Alistair Francis
On Thu, Jul 9, 2020 at 10:05 PM Bin Meng  wrote:
>
> From: Bin Meng 
>
> The RISC-V generic platform is a flattened device tree (FDT) based
> platform where all platform specific functionality is provided based
> on FDT passed by previous booting stage. The support was added in
> the upstream OpenSBI v0.8 release recently.
>
> This series updates QEMU to switch to use generic platform of opensbi
> bios images. With the recent fw_dynamic image support, let's replace
> the fw_jump images with fw_dynamic ones too.
>
> The patch emails do not contain binary bits, please grab all updates
> at https://github.com/lbmeng/qemu.git bios branch.
>
> Changes in v4:
> - Remove old binaries in the Makefile for `make install` bisection
>
> Changes in v3:
> - Change fw_jump to fw_dynamic in the make rules
> - Change to fw_dynamic.bin for virt & sifive_u
> - Change to fw_dynamic.elf for Spike
> - Generate fw_dynamic images in the artifacts
> - change fw_jump to fw_dynamic in the Makefile
>
> Changes in v2:
> - new patch: configure: Create symbolic links for pc-bios/*.elf files
> - Upgrade OpenSBI to v0.8 release
> - Copy the ELF images too in the make rules
> - Include ELF images in the artifacts
> - new patch: Makefile: Ship the generic platform bios images for RISC-V
>
> Bin Meng (7):
>   configure: Create symbolic links for pc-bios/*.elf files
>   roms/opensbi: Upgrade from v0.7 to v0.8
>   roms/Makefile: Build the generic platform for RISC-V OpenSBI firmware
>   hw/riscv: Use pre-built bios image of generic platform for virt &
> sifive_u
>   hw/riscv: spike: Change the default bios to use generic platform image
>   gitlab-ci/opensbi: Update GitLab CI to build generic platform
>   Makefile: Ship the generic platform bios images for RISC-V

Reviewed-by: Alistair Francis 

Alistair

>
>  .gitlab-ci.d/opensbi.yml   |  28 --
>  Makefile   |   4 ++--
>  configure  |   1 +
>  hw/riscv/sifive_u.c|   4 ++--
>  hw/riscv/spike.c   |   9 +--
>  hw/riscv/virt.c|   4 ++--
>  pc-bios/opensbi-riscv32-generic-fw_dynamic.bin | Bin 0 -> 62144 bytes
>  pc-bios/opensbi-riscv32-generic-fw_dynamic.elf | Bin 0 -> 558668 bytes
>  pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin   | Bin 49520 -> 0 bytes
>  pc-bios/opensbi-riscv32-virt-fw_jump.bin   | Bin 49504 -> 0 bytes
>  pc-bios/opensbi-riscv64-generic-fw_dynamic.bin | Bin 0 -> 70792 bytes
>  pc-bios/opensbi-riscv64-generic-fw_dynamic.elf | Bin 0 -> 620424 bytes
>  pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin   | Bin 57936 -> 0 bytes
>  pc-bios/opensbi-riscv64-virt-fw_jump.bin   | Bin 57920 -> 0 bytes
>  roms/Makefile  |  32 
> -
>  roms/opensbi   |   2 +-
>  16 files changed, 35 insertions(+), 49 deletions(-)
>  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
>  create mode 100644 pc-bios/opensbi-riscv32-generic-fw_dynamic.elf
>  delete mode 100644 pc-bios/opensbi-riscv32-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv32-virt-fw_jump.bin
>  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.bin
>  create mode 100644 pc-bios/opensbi-riscv64-generic-fw_dynamic.elf
>  delete mode 100644 pc-bios/opensbi-riscv64-sifive_u-fw_jump.bin
>  delete mode 100644 pc-bios/opensbi-riscv64-virt-fw_jump.bin
>
> --
> 2.7.4
>
>



Re: [PATCH v5 07/20] microvm: make virtio irq base runtime configurable

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:43 +0200
Gerd Hoffmann  wrote:

> Signed-off-by: Gerd Hoffmann 
> Reviewed-by: Sergio Lopez 

Reviewed-by: Igor Mammedov 


> ---
>  include/hw/i386/microvm.h |  2 +-
>  hw/i386/microvm.c | 11 +++
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index fd34b78e0d2a..03e735723726 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -27,7 +27,6 @@
>  
>  /* Platform virtio definitions */
>  #define VIRTIO_MMIO_BASE  0xfeb0
> -#define VIRTIO_IRQ_BASE   5
>  #define VIRTIO_NUM_TRANSPORTS 8
>  #define VIRTIO_CMDLINE_MAXLEN 64
>  
> @@ -57,6 +56,7 @@ typedef struct {
>  bool auto_kernel_cmdline;
>  
>  /* Machine state */
> +uint32_t virtio_irq_base;
>  bool kernel_cmdline_fixed;
>  } MicrovmMachineState;
>  
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 1300c396947b..ab6ee6c67b1a 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -121,10 +121,11 @@ static void microvm_devices_init(MicrovmMachineState 
> *mms)
>  
>  kvmclock_create();
>  
> +mms->virtio_irq_base = 5;
>  for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
>  sysbus_create_simple("virtio-mmio",
>   VIRTIO_MMIO_BASE + i * 512,
> - x86ms->gsi[VIRTIO_IRQ_BASE + i]);
> + x86ms->gsi[mms->virtio_irq_base + i]);
>  }
>  
>  /* Optional and legacy devices */
> @@ -227,7 +228,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
>  x86ms->ioapic_as = _space_memory;
>  }
>  
> -static gchar *microvm_get_mmio_cmdline(gchar *name)
> +static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
>  {
>  gchar *cmdline;
>  gchar *separator;
> @@ -247,7 +248,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
>  ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
>   " virtio_mmio.device=512@0x%lx:%ld",
>   VIRTIO_MMIO_BASE + index * 512,
> - VIRTIO_IRQ_BASE + index);
> + virtio_irq_base + index);
>  if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
>  g_free(cmdline);
>  return NULL;
> @@ -259,6 +260,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
>  static void microvm_fix_kernel_cmdline(MachineState *machine)
>  {
>  X86MachineState *x86ms = X86_MACHINE(machine);
> +MicrovmMachineState *mms = MICROVM_MACHINE(machine);
>  BusState *bus;
>  BusChild *kid;
>  char *cmdline;
> @@ -282,7 +284,8 @@ static void microvm_fix_kernel_cmdline(MachineState 
> *machine)
>  BusState *mmio_bus = _virtio_bus->parent_obj;
>  
>  if (!QTAILQ_EMPTY(_bus->children)) {
> -gchar *mmio_cmdline = 
> microvm_get_mmio_cmdline(mmio_bus->name);
> +gchar *mmio_cmdline = microvm_get_mmio_cmdline
> +(mmio_bus->name, mms->virtio_irq_base);
>  if (mmio_cmdline) {
>  char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, 
> NULL);
>  g_free(mmio_cmdline);




Re: [PATCH v5 04/20] acpi: ged: add control regs

2020-07-10 Thread Igor Mammedov
On Tue,  7 Jul 2020 14:53:40 +0200
Gerd Hoffmann  wrote:

> Add control regs (sleep, reset) for hw-reduced acpi.
> 
> Signed-off-by: Gerd Hoffmann 

with below comments addressed:

   Reviewed-by: Igor Mammedov 

> ---
>  include/hw/acpi/generic_event_device.h |  7 
>  hw/acpi/generic_event_device.c | 44 ++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/include/hw/acpi/generic_event_device.h 
> b/include/hw/acpi/generic_event_device.h
> index 90a9180db572..474c92198080 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -72,6 +72,12 @@
>  #define ACPI_GED_EVT_SEL_OFFSET0x0
>  #define ACPI_GED_EVT_SEL_LEN   0x4
>  
> +#define ACPI_GED_REG_SLEEP_CTL 0x00
> +#define ACPI_GED_REG_SLEEP_STS 0x01
> +#define ACPI_GED_REG_RESET 0x02
> +#define   ACPI_GED_RESET_VALUE 0x42
  ^^ too many ' '

also it would be nice to point out where vaule comes from
(if it's from spec then ref to spec pls)

> +#define ACPI_GED_REG_COUNT 0x03
> +
>  #define GED_DEVICE  "GED"
>  #define AML_GED_EVT_REG "EREG"
>  #define AML_GED_EVT_SEL "ESEL"
> @@ -87,6 +93,7 @@
>  
>  typedef struct GEDState {
>  MemoryRegion evt;
> +MemoryRegion regs;
>  uint32_t sel;
>  } GEDState;
>  
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index b8abdefa1c77..491df80a5cc7 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -20,6 +20,7 @@
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>  #include "qemu/error-report.h"
> +#include "sysemu/runstate.h"
>  
>  static const uint32_t ged_supported_events[] = {
>  ACPI_GED_MEM_HOTPLUG_EVT,
> @@ -176,6 +177,45 @@ static const MemoryRegionOps ged_evt_ops = {
>  },
>  };
>  
> +static uint64_t ged_regs_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +return 0;
> +}
> +
> +static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
> +   unsigned int size)
> +{
> +bool slp_en;
> +int slp_typ;
> +
> +switch (addr) {
> +case ACPI_GED_REG_SLEEP_CTL:
> +slp_typ = (data >> 2) & 0x07;
> +slp_en  = (data >> 5) & 0x01;
> +if (slp_en && slp_typ == 5) {

replace magic 5 with something more descriptive and use it also in 8/20
during initializing _S5 package

> +qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +}
> +return;
> +case ACPI_GED_REG_SLEEP_STS:
> +return;
> +case ACPI_GED_REG_RESET:
> +if (data == ACPI_GED_RESET_VALUE) {
> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +}
> +return;
> +}
> +}
> +
> +static const MemoryRegionOps ged_regs_ops = {
> +.read = ged_regs_read,
> +.write = ged_regs_write,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> +.valid = {
> +.min_access_size = 1,
> +.max_access_size = 1,
> +},
> +};
> +
>  static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
>  DeviceState *dev, Error **errp)
>  {
> @@ -332,6 +372,10 @@ static void acpi_ged_initfn(Object *obj)
>   sysbus_init_mmio(sbd, >container_memhp);
>   acpi_memory_hotplug_init(>container_memhp, OBJECT(dev),
>>memhp_state, 0);
> +
> +memory_region_init_io(_st->regs, obj, _regs_ops, ged_st,
> +  TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT);
> +sysbus_init_mmio(sbd, _st->regs);
>  }
>  
>  static void acpi_ged_class_init(ObjectClass *class, void *data)




Re: [PATCH v1] configure: fix malloc check

2020-07-10 Thread Richard Henderson
On 7/7/20 10:13 AM, Olaf Hering wrote:
> Avoid random return value.
> 
> Fixes commit f2dfe54c74f768a5bf78c9e5918918727f9d9459
> 
> Signed-off-by: Olaf Hering 
> ---
>  configure | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Richard Henderson 

r~



Re: [PATCH v5 5/5] vhost-user-blk: default num_queues to -smp N

2020-07-10 Thread Raphael Norwitz
On Fri, Jul 10, 2020 at 5:53 AM Stefan Hajnoczi  wrote:
>
> On Thu, Jul 09, 2020 at 11:02:24AM -0700, Raphael Norwitz wrote:
> > On Mon, Jul 6, 2020 at 7:00 AM Stefan Hajnoczi  wrote:
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index a00b854736..39aec42dae 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -420,6 +420,9 @@ static void vhost_user_blk_device_realize(DeviceState 
> > > *dev, Error **errp)
> > >  return;
> > >  }
> > >
> > > +if (s->num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> > > +s->num_queues = 1;
> > > +}
> >
> > What is this check for? Is it just a backstop to ensure that
> > num_queues is set to 1 if vhost-user-blk-pci doesn't update it?
>
> For the non-PCI VIRTIO transports that do not handle num_queues ==
> VHOST_USER_BLK_AUTO_NUM_QUEUES themselves.
>

Got it. Looks good then.

Reviewed-by: Raphael Norwitz 

> Stefan



Re: [PATCH v4 7/7] Makefile: Ship the generic platform bios images for RISC-V

2020-07-10 Thread Alistair Francis
On Thu, Jul 9, 2020 at 10:11 PM Bin Meng  wrote:
>
> From: Bin Meng 
>
> Update the install blob list to include the generic platform
> fw_dynamic bios images.
>
> Signed-off-by: Bin Meng 

You didn't address the comments in v3.

Thinking about this more though it looks like we currently don't
install anything, so this is an improvement.

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - change fw_jump to fw_dynamic in the Makefile
>
> Changes in v2:
> - new patch: Makefile: Ship the generic platform bios images for RISC-V
>
>  Makefile | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index f06b3ae..05e05bb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -840,7 +840,9 @@ palcode-clipper \
>  u-boot.e500 u-boot-sam460-20100605.bin \
>  qemu_vga.ndrv \
>  edk2-licenses.txt \
> -hppa-firmware.img
> +hppa-firmware.img \
> +opensbi-riscv32-generic-fw_dynamic.bin 
> opensbi-riscv32-generic-fw_dynamic.elf \
> +opensbi-riscv64-generic-fw_dynamic.bin opensbi-riscv64-generic-fw_dynamic.elf
>
>
>  DESCS=50-edk2-i386-secure.json 50-edk2-x86_64-secure.json \
> --
> 2.7.4
>
>



[Bug 1885332] Re: Error in user-mode calculation of ELF aux vector's AT_PHDR

2020-07-10 Thread Langston
@Dimitry To confirm that this is really the same issue (and not an
unrelated crash in the same function), could you post:

 1. the ELF headers ("readelf -h"),
 2. the program headers ("readelf -l"), and
 3. the output (the AUX VECTOR section) from this GDB script (suitably modified 
for your program), when connecting to QEMU's GDB server? 
https://gist.github.com/langston-barrett/5573d64ae0c9953e2fa0fe26847a5e1e

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

Title:
  Error in user-mode calculation of ELF aux vector's AT_PHDR

Status in QEMU:
  New

Bug description:
  
  I have an (admittedly strange) statically-linked ELF binary for Linux that 
runs just fine on top of the Linux kernel in QEMU full-system emulation, but 
crashes before main in user-mode emulation. Specifically, it crashes when 
initializing thread-local storage in glibc's _dl_aux_init, because it reads out 
a strange value from the AT_PHDR entry of the ELF aux vector.

  The binary has these program headers:

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  EXIDX  0x065874 0x00075874 0x00075874 0x00570 0x00570 R   0x4
  PHDR   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x00 0x0001 0x0001 0x65de8 0x65de8 R E 0x1
  LOAD   0x066b7c 0x00086b7c 0x00086b7c 0x02384 0x02384 RW  0x1
  NOTE   0x000114 0x00010114 0x00010114 0x00044 0x00044 R   0x4
  TLS0x066b7c 0x00086b7c 0x00086b7c 0x00010 0x00030 R   0x4
  GNU_STACK  0x00 0x 0x 0x0 0x0 RW  0x8
  GNU_RELRO  0x066b7c 0x00086b7c 0x00086b7c 0x00484 0x00484 R   0x1
  LOAD   0x07e000 0x00089000 0x00089000 0x03f44 0x03f44 R E 0x1000
  LOAD   0x098000 0x0003 0x0003 0x01000 0x01000 RW  0x1000

  If I build the Linux kernel with the following patch to the very end
  of create_elf_tables in fs/binfmt_elf.c

/* Put the elf_info on the stack in the right place.  */
elf_addr_t *my_auxv = (elf_addr_t *) mm->saved_auxv;
int i;
for (i = 0; i < 15; i++) {
  printk("0x%x = 0x%x", my_auxv[2*i], my_auxv[(2*i)+ 1]);
}
if (copy_to_user(sp, mm->saved_auxv, ei_index * sizeof(elf_addr_t)))
return -EFAULT;
return 0;

  and run it like this:

qemu-system-arm \
  -M versatilepb \
  -nographic \
  -dtb ./dts/versatile-pb.dtb \
  -kernel zImage \
  -M versatilepb \
  -m 128M \
  -append "earlyprintk=vga,keep" \
  -initrd initramfs

  after I've built the kernel initramfs like this (where "init" is the
  binary in question):

make ARCH=arm versatile_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all -j10
cp "$1" arch/arm/boot/init
cd arch/arm/boot
echo init | cpio -o --format=newc > initramfs

  then I get the following output. This is the kernel's view of the aux
  vector for this binary:

0x10 = 0x1d7
0x6 = 0x1000
0x11 = 0x64
0x3 = 0x90
0x4 = 0x20
0x5 = 0xb
0x7 = 0x0
0x8 = 0x0
0x9 = 0x101b8
0xb = 0x0
0xc = 0x0
0xd = 0x0
0xe = 0x0
0x17 = 0x0
0x19 = 0xbec62fb5

  However, if I run "qemu-arm -g 12345 binary" and use GDB to peek at
  the aux vector at the beginning of __libc_start_init (for example,
  using this Python GDB API script: https://gist.github.com/langston-
  barrett/5573d64ae0c9953e2fa0fe26847a5e1e), then I see the following
  values:

AT_PHDR = 0xae000
AT_PHENT = 0x20
AT_PHNUM = 0xb
AT_PAGESZ = 0x1000
AT_BASE = 0x0
AT_FLAGS = 0x0
AT_ENTRY = 0x10230
AT_UID = 0x3e9
AT_EUID = 0x3e9
AT_GID = 0x3e9
AT_EGID = 0x3e9
AT_HWCAP = 0x1fb8d7
AT_CLKTCK = 0x64
AT_RANDOM = -0x103c0
AT_HWCAP2 = 0x1f
AT_NULL = 0x0

  The crucial difference is in AT_PHDR (0x3), which is indeed the
  virtual address of the PHDR segment when the kernel calculates it, but
  is not when QEMU calculates it.

  qemu-arm --version
  qemu-arm version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.26)

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



Re: [RFC 01/65] target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion

2020-07-10 Thread Alistair Francis
On Fri, Jul 10, 2020 at 9:13 AM Richard Henderson
 wrote:
>
> On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> > From: Frank Chang 
> >
> > gvec should provide vecop_list to avoid:
> > "tcg_tcg_assert_listed_vecop: code should not be reached bug" assertion.
> >
> > Signed-off-by: Frank Chang 
> > ---
> >  target/riscv/insn_trans/trans_rvv.inc.c | 5 +
> >  1 file changed, 5 insertions(+)
>
> Reviewed-by: Richard Henderson 
>
> Alistair, this one should be queued for 5.1 as a bug fix.

Thanks for reviewing these. I have applied the first 4 to my PR for 5.1.

Alistair

>
>
> r~
>



Re: [PATCH] disas/riscv: Fix incorrect disassembly for `imm20` operand.

2020-07-10 Thread Richard Henderson
On 7/7/20 8:43 AM, Wei Wu wrote:
>  static int32_t operand_imm20(rv_inst inst)
>  {
> -return (((int64_t)inst << 32) >> 44) << 12;
> +return ((int64_t)inst << 32) >> 44;
>  }

There's no point in casting to int64_t, for one.  But it would be better to use
sextract32(inst, 12, 20).


r~



Re: [PULL 0/1] qemu-openbios queue 20200707

2020-07-10 Thread Peter Maydell
On Tue, 7 Jul 2020 at 22:08, Mark Cave-Ayland
 wrote:
>
> The following changes since commit eb2c66b10efd2b914b56b20ae90655914310c925:
>
>   Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-07-06' 
> into staging (2020-07-07 19:47:26 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/mcayland/qemu.git tags/qemu-openbios-20200707
>
> for you to fetch changes up to 1e04092feecfc8caaf314df2670bf9c645a0b122:
>
>   Update OpenBIOS images to 75fbb41d built from submodule. (2020-07-07 
> 21:54:37 +0100)
>
> 
> qemu-openbios queue
>
> 


Applied, thanks.

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

-- PMM



[Bug 1885332] Re: Error in user-mode calculation of ELF aux vector's AT_PHDR

2020-07-10 Thread Dmitry
> runs just fine on top of the Linux kernel in QEMU full-system
emulation, but crashes before main in user-mode emulation

So it seems system vs user-mode is not the issue here, probably it is
related to gdb mode in user-mode qemu.

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

Title:
  Error in user-mode calculation of ELF aux vector's AT_PHDR

Status in QEMU:
  New

Bug description:
  
  I have an (admittedly strange) statically-linked ELF binary for Linux that 
runs just fine on top of the Linux kernel in QEMU full-system emulation, but 
crashes before main in user-mode emulation. Specifically, it crashes when 
initializing thread-local storage in glibc's _dl_aux_init, because it reads out 
a strange value from the AT_PHDR entry of the ELF aux vector.

  The binary has these program headers:

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  EXIDX  0x065874 0x00075874 0x00075874 0x00570 0x00570 R   0x4
  PHDR   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x00 0x0001 0x0001 0x65de8 0x65de8 R E 0x1
  LOAD   0x066b7c 0x00086b7c 0x00086b7c 0x02384 0x02384 RW  0x1
  NOTE   0x000114 0x00010114 0x00010114 0x00044 0x00044 R   0x4
  TLS0x066b7c 0x00086b7c 0x00086b7c 0x00010 0x00030 R   0x4
  GNU_STACK  0x00 0x 0x 0x0 0x0 RW  0x8
  GNU_RELRO  0x066b7c 0x00086b7c 0x00086b7c 0x00484 0x00484 R   0x1
  LOAD   0x07e000 0x00089000 0x00089000 0x03f44 0x03f44 R E 0x1000
  LOAD   0x098000 0x0003 0x0003 0x01000 0x01000 RW  0x1000

  If I build the Linux kernel with the following patch to the very end
  of create_elf_tables in fs/binfmt_elf.c

/* Put the elf_info on the stack in the right place.  */
elf_addr_t *my_auxv = (elf_addr_t *) mm->saved_auxv;
int i;
for (i = 0; i < 15; i++) {
  printk("0x%x = 0x%x", my_auxv[2*i], my_auxv[(2*i)+ 1]);
}
if (copy_to_user(sp, mm->saved_auxv, ei_index * sizeof(elf_addr_t)))
return -EFAULT;
return 0;

  and run it like this:

qemu-system-arm \
  -M versatilepb \
  -nographic \
  -dtb ./dts/versatile-pb.dtb \
  -kernel zImage \
  -M versatilepb \
  -m 128M \
  -append "earlyprintk=vga,keep" \
  -initrd initramfs

  after I've built the kernel initramfs like this (where "init" is the
  binary in question):

make ARCH=arm versatile_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all -j10
cp "$1" arch/arm/boot/init
cd arch/arm/boot
echo init | cpio -o --format=newc > initramfs

  then I get the following output. This is the kernel's view of the aux
  vector for this binary:

0x10 = 0x1d7
0x6 = 0x1000
0x11 = 0x64
0x3 = 0x90
0x4 = 0x20
0x5 = 0xb
0x7 = 0x0
0x8 = 0x0
0x9 = 0x101b8
0xb = 0x0
0xc = 0x0
0xd = 0x0
0xe = 0x0
0x17 = 0x0
0x19 = 0xbec62fb5

  However, if I run "qemu-arm -g 12345 binary" and use GDB to peek at
  the aux vector at the beginning of __libc_start_init (for example,
  using this Python GDB API script: https://gist.github.com/langston-
  barrett/5573d64ae0c9953e2fa0fe26847a5e1e), then I see the following
  values:

AT_PHDR = 0xae000
AT_PHENT = 0x20
AT_PHNUM = 0xb
AT_PAGESZ = 0x1000
AT_BASE = 0x0
AT_FLAGS = 0x0
AT_ENTRY = 0x10230
AT_UID = 0x3e9
AT_EUID = 0x3e9
AT_GID = 0x3e9
AT_EGID = 0x3e9
AT_HWCAP = 0x1fb8d7
AT_CLKTCK = 0x64
AT_RANDOM = -0x103c0
AT_HWCAP2 = 0x1f
AT_NULL = 0x0

  The crucial difference is in AT_PHDR (0x3), which is indeed the
  virtual address of the PHDR segment when the kernel calculates it, but
  is not when QEMU calculates it.

  qemu-arm --version
  qemu-arm version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.26)

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



[Bug 1885332] Re: Error in user-mode calculation of ELF aux vector's AT_PHDR

2020-07-10 Thread Dmitry
@langston0 Thanks for detailed explanation, got the same problem for
qemu-s390.


The way to reproduce (linux kernel >= 4.8, for example: Ubuntu 18.04):
# Register qemu binfmt_misc handlers
$ docker run --rm --privileged multiarch/qemu-user-static --reset -p yes

$ cat Dockerfile.s390x 
FROM s390x/ubuntu
RUN apt-get update && \
apt-get install -y \
gcc make libpcre3-dev libreadline-dev

RUN cd /home && git clone https://github.com/nginx/njs

RUN cd /home/njs && ./configure --cc-opt='-O0 -static -lm -lrt -pthread
-Wl,--whole-archive -lpthread -ltinfo -Wl,--no-whole-archive' && make
njs

$ docker build -t njs/390x -f Dockerfile.s390x .

# check the binary (WORKS!)
# inside docker s390 binaries are executed using qemu-s390-static from the host
$ docker run  -t njs/390x /home/njs/build/njs -c 'console.log("hello")'
hello

# copy binary to host
$ docker run  -v `pwd`:/m -ti njs/390x cp /home/njs/build/njs /m/njs-s390

# deregister binfmt handler
$ sudo bash -c "echo -1 > /proc/sys/fs/binfmt_misc/qemu-s390x"

# run qemu gdb
$ qemu-s390x  -g 12345 ./njs-s390

# in a separate terminal
$ gdb-multiarch ./njs-s390 -ex 'target remote localhost:12345'
0x01000520 in _start ()
(gdb) si
0x01000524 in _start ()
(gdb) si
0x0100052a in _start ()
(gdb) c
Continuing.

Program received signal SIGILL, Illegal instruction.
0x011a418c in _dl_aux_init ()
(gdb) bt
#0  0x011a418c in _dl_aux_init ()
#1  0x011663f0 in __libc_start_main ()
#2  0x01000564 in _start ()

qemu-s390x --version
qemu-s390x version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.28)

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

Title:
  Error in user-mode calculation of ELF aux vector's AT_PHDR

Status in QEMU:
  New

Bug description:
  
  I have an (admittedly strange) statically-linked ELF binary for Linux that 
runs just fine on top of the Linux kernel in QEMU full-system emulation, but 
crashes before main in user-mode emulation. Specifically, it crashes when 
initializing thread-local storage in glibc's _dl_aux_init, because it reads out 
a strange value from the AT_PHDR entry of the ELF aux vector.

  The binary has these program headers:

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  EXIDX  0x065874 0x00075874 0x00075874 0x00570 0x00570 R   0x4
  PHDR   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x00 0x0001 0x0001 0x65de8 0x65de8 R E 0x1
  LOAD   0x066b7c 0x00086b7c 0x00086b7c 0x02384 0x02384 RW  0x1
  NOTE   0x000114 0x00010114 0x00010114 0x00044 0x00044 R   0x4
  TLS0x066b7c 0x00086b7c 0x00086b7c 0x00010 0x00030 R   0x4
  GNU_STACK  0x00 0x 0x 0x0 0x0 RW  0x8
  GNU_RELRO  0x066b7c 0x00086b7c 0x00086b7c 0x00484 0x00484 R   0x1
  LOAD   0x07e000 0x00089000 0x00089000 0x03f44 0x03f44 R E 0x1000
  LOAD   0x098000 0x0003 0x0003 0x01000 0x01000 RW  0x1000

  If I build the Linux kernel with the following patch to the very end
  of create_elf_tables in fs/binfmt_elf.c

/* Put the elf_info on the stack in the right place.  */
elf_addr_t *my_auxv = (elf_addr_t *) mm->saved_auxv;
int i;
for (i = 0; i < 15; i++) {
  printk("0x%x = 0x%x", my_auxv[2*i], my_auxv[(2*i)+ 1]);
}
if (copy_to_user(sp, mm->saved_auxv, ei_index * sizeof(elf_addr_t)))
return -EFAULT;
return 0;

  and run it like this:

qemu-system-arm \
  -M versatilepb \
  -nographic \
  -dtb ./dts/versatile-pb.dtb \
  -kernel zImage \
  -M versatilepb \
  -m 128M \
  -append "earlyprintk=vga,keep" \
  -initrd initramfs

  after I've built the kernel initramfs like this (where "init" is the
  binary in question):

make ARCH=arm versatile_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all -j10
cp "$1" arch/arm/boot/init
cd arch/arm/boot
echo init | cpio -o --format=newc > initramfs

  then I get the following output. This is the kernel's view of the aux
  vector for this binary:

0x10 = 0x1d7
0x6 = 0x1000
0x11 = 0x64
0x3 = 0x90
0x4 = 0x20
0x5 = 0xb
0x7 = 0x0
0x8 = 0x0
0x9 = 0x101b8
0xb = 0x0
0xc = 0x0
0xd = 0x0
0xe = 0x0
0x17 = 0x0
0x19 = 0xbec62fb5

  However, if I run "qemu-arm -g 12345 binary" and use GDB to peek at
  the aux vector at the beginning of __libc_start_init (for example,
  using this Python GDB API script: https://gist.github.com/langston-
  barrett/5573d64ae0c9953e2fa0fe26847a5e1e), then I see the following
  values:

AT_PHDR = 0xae000
AT_PHENT = 0x20
AT_PHNUM = 0xb
AT_PAGESZ = 0x1000
AT_BASE = 0x0
AT_FLAGS = 0x0

[Bug 1885332] Re: Error in user-mode calculation of ELF aux vector's AT_PHDR

2020-07-10 Thread Dmitry
BTW, before "sudo bash -c "echo -1 > /proc/sys/fs/binfmt_misc/qemu-
s390x"

njs-s390 also works on the host:

$ ./njs-s390 -c 'console.log("hello")'
hello

$ file njs-s390
njs-s390: ELF 64-bit MSB executable, IBM S/390, version 1 (GNU/Linux), 
statically linked, BuildID[sha1]=e37618578fb0a8c60f426826167a800e4f314ef3, for 
GNU/Linux 3.2.0, with debug_info, not stripped

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

Title:
  Error in user-mode calculation of ELF aux vector's AT_PHDR

Status in QEMU:
  New

Bug description:
  
  I have an (admittedly strange) statically-linked ELF binary for Linux that 
runs just fine on top of the Linux kernel in QEMU full-system emulation, but 
crashes before main in user-mode emulation. Specifically, it crashes when 
initializing thread-local storage in glibc's _dl_aux_init, because it reads out 
a strange value from the AT_PHDR entry of the ELF aux vector.

  The binary has these program headers:

Program Headers:
  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  EXIDX  0x065874 0x00075874 0x00075874 0x00570 0x00570 R   0x4
  PHDR   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x0a3000 0x0090 0x0090 0x00160 0x00160 R   0x1000
  LOAD   0x00 0x0001 0x0001 0x65de8 0x65de8 R E 0x1
  LOAD   0x066b7c 0x00086b7c 0x00086b7c 0x02384 0x02384 RW  0x1
  NOTE   0x000114 0x00010114 0x00010114 0x00044 0x00044 R   0x4
  TLS0x066b7c 0x00086b7c 0x00086b7c 0x00010 0x00030 R   0x4
  GNU_STACK  0x00 0x 0x 0x0 0x0 RW  0x8
  GNU_RELRO  0x066b7c 0x00086b7c 0x00086b7c 0x00484 0x00484 R   0x1
  LOAD   0x07e000 0x00089000 0x00089000 0x03f44 0x03f44 R E 0x1000
  LOAD   0x098000 0x0003 0x0003 0x01000 0x01000 RW  0x1000

  If I build the Linux kernel with the following patch to the very end
  of create_elf_tables in fs/binfmt_elf.c

/* Put the elf_info on the stack in the right place.  */
elf_addr_t *my_auxv = (elf_addr_t *) mm->saved_auxv;
int i;
for (i = 0; i < 15; i++) {
  printk("0x%x = 0x%x", my_auxv[2*i], my_auxv[(2*i)+ 1]);
}
if (copy_to_user(sp, mm->saved_auxv, ei_index * sizeof(elf_addr_t)))
return -EFAULT;
return 0;

  and run it like this:

qemu-system-arm \
  -M versatilepb \
  -nographic \
  -dtb ./dts/versatile-pb.dtb \
  -kernel zImage \
  -M versatilepb \
  -m 128M \
  -append "earlyprintk=vga,keep" \
  -initrd initramfs

  after I've built the kernel initramfs like this (where "init" is the
  binary in question):

make ARCH=arm versatile_defconfig
make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- all -j10
cp "$1" arch/arm/boot/init
cd arch/arm/boot
echo init | cpio -o --format=newc > initramfs

  then I get the following output. This is the kernel's view of the aux
  vector for this binary:

0x10 = 0x1d7
0x6 = 0x1000
0x11 = 0x64
0x3 = 0x90
0x4 = 0x20
0x5 = 0xb
0x7 = 0x0
0x8 = 0x0
0x9 = 0x101b8
0xb = 0x0
0xc = 0x0
0xd = 0x0
0xe = 0x0
0x17 = 0x0
0x19 = 0xbec62fb5

  However, if I run "qemu-arm -g 12345 binary" and use GDB to peek at
  the aux vector at the beginning of __libc_start_init (for example,
  using this Python GDB API script: https://gist.github.com/langston-
  barrett/5573d64ae0c9953e2fa0fe26847a5e1e), then I see the following
  values:

AT_PHDR = 0xae000
AT_PHENT = 0x20
AT_PHNUM = 0xb
AT_PAGESZ = 0x1000
AT_BASE = 0x0
AT_FLAGS = 0x0
AT_ENTRY = 0x10230
AT_UID = 0x3e9
AT_EUID = 0x3e9
AT_GID = 0x3e9
AT_EGID = 0x3e9
AT_HWCAP = 0x1fb8d7
AT_CLKTCK = 0x64
AT_RANDOM = -0x103c0
AT_HWCAP2 = 0x1f
AT_NULL = 0x0

  The crucial difference is in AT_PHDR (0x3), which is indeed the
  virtual address of the PHDR segment when the kernel calculates it, but
  is not when QEMU calculates it.

  qemu-arm --version
  qemu-arm version 2.11.1(Debian 1:2.11+dfsg-1ubuntu7.26)

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



Re: [RFC PATCH v2 4/4] target/avr/translate: Fix SBRC/SBRS instructions

2020-07-10 Thread Richard Henderson
On 7/7/20 9:58 AM, Philippe Mathieu-Daudé wrote:
> I couldn't run Sarah's test suite on Fedora 30:
> 
> /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find
> crtatmega2560.o: No such file or directory
> /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lm
> /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -lc
> /usr/lib/gcc/avr/9.2.0/../../../../avr/bin/ld: cannot find -latmega2560
> collect2: error: ld returned 1 exit status
> 
> I'll try on some Debian based host.

I believe the debian avr-libc package will have those, and should be pulled in
by gcc-avr.



r~



[PATCH] .cirrus.yml: add bash to the brew packages

2020-07-10 Thread Alex Bennée
Like the sed we include earlier we want something more recent for
iotests to work.

Fixes: 57ee95ed
Cc: Max Reitz 
Signed-off-by: Alex Bennée 
---
 .cirrus.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 69342ae031bc..f287d23c5b9b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -20,7 +20,7 @@ macos_task:
   osx_instance:
 image: mojave-base
   install_script:
-- brew install pkg-config python gnu-sed glib pixman make sdl2
+- brew install pkg-config python gnu-sed glib pixman make sdl2 bash
   script:
 - mkdir build
 - cd build
@@ -33,7 +33,7 @@ macos_xcode_task:
 # this is an alias for the latest Xcode
 image: mojave-xcode
   install_script:
-- brew install pkg-config gnu-sed glib pixman make sdl2
+- brew install pkg-config gnu-sed glib pixman make sdl2 bash
   script:
 - mkdir build
 - cd build
-- 
2.20.1




Re: [RFC 57/65] target/riscv: rvv-0.9: floating-point min/max instructions

2020-07-10 Thread Alex Bennée


frank.ch...@sifive.com writes:

> From: Frank Chang 
>
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 42a48be5fd..d617d0dfbd 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -3831,28 +3831,28 @@ GEN_VEXT_V_ENV(vfsqrt_v_w, 4, 4, clearl)
>  GEN_VEXT_V_ENV(vfsqrt_v_d, 8, 8, clearq)
>  
>  /* Vector Floating-Point MIN/MAX Instructions */
> -RVVCALL(OPFVV2, vfmin_vv_h, OP_UUU_H, H2, H2, H2, float16_minnum)
> -RVVCALL(OPFVV2, vfmin_vv_w, OP_UUU_W, H4, H4, H4, float32_minnum)
> -RVVCALL(OPFVV2, vfmin_vv_d, OP_UUU_D, H8, H8, H8, float64_minnum)
> +RVVCALL(OPFVV2, vfmin_vv_h, OP_UUU_H, H2, H2, H2, float16_minnum_noprop)
> +RVVCALL(OPFVV2, vfmin_vv_w, OP_UUU_W, H4, H4, H4, float32_minnum_noprop)
> +RVVCALL(OPFVV2, vfmin_vv_d, OP_UUU_D, H8, H8, H8,
>  float64_minnum_noprop)

This patch breaks bisection because you don't introduce this into
softfloat until later. You should always ensure each step can build and
run - practically this means the softfloat changes should be at the
beginning of the series.

>  GEN_VEXT_VV_ENV(vfmin_vv_h, 2, 2, clearh)
>  GEN_VEXT_VV_ENV(vfmin_vv_w, 4, 4, clearl)
>  GEN_VEXT_VV_ENV(vfmin_vv_d, 8, 8, clearq)
> -RVVCALL(OPFVF2, vfmin_vf_h, OP_UUU_H, H2, H2, float16_minnum)
> -RVVCALL(OPFVF2, vfmin_vf_w, OP_UUU_W, H4, H4, float32_minnum)
> -RVVCALL(OPFVF2, vfmin_vf_d, OP_UUU_D, H8, H8, float64_minnum)
> +RVVCALL(OPFVF2, vfmin_vf_h, OP_UUU_H, H2, H2, float16_minnum_noprop)
> +RVVCALL(OPFVF2, vfmin_vf_w, OP_UUU_W, H4, H4, float32_minnum_noprop)
> +RVVCALL(OPFVF2, vfmin_vf_d, OP_UUU_D, H8, H8, float64_minnum_noprop)
>  GEN_VEXT_VF(vfmin_vf_h, 2, 2, clearh)
>  GEN_VEXT_VF(vfmin_vf_w, 4, 4, clearl)
>  GEN_VEXT_VF(vfmin_vf_d, 8, 8, clearq)
>  
> -RVVCALL(OPFVV2, vfmax_vv_h, OP_UUU_H, H2, H2, H2, float16_maxnum)
> -RVVCALL(OPFVV2, vfmax_vv_w, OP_UUU_W, H4, H4, H4, float32_maxnum)
> -RVVCALL(OPFVV2, vfmax_vv_d, OP_UUU_D, H8, H8, H8, float64_maxnum)
> +RVVCALL(OPFVV2, vfmax_vv_h, OP_UUU_H, H2, H2, H2, float16_maxnum_noprop)
> +RVVCALL(OPFVV2, vfmax_vv_w, OP_UUU_W, H4, H4, H4, float32_maxnum_noprop)
> +RVVCALL(OPFVV2, vfmax_vv_d, OP_UUU_D, H8, H8, H8, float64_maxnum_noprop)
>  GEN_VEXT_VV_ENV(vfmax_vv_h, 2, 2, clearh)
>  GEN_VEXT_VV_ENV(vfmax_vv_w, 4, 4, clearl)
>  GEN_VEXT_VV_ENV(vfmax_vv_d, 8, 8, clearq)
> -RVVCALL(OPFVF2, vfmax_vf_h, OP_UUU_H, H2, H2, float16_maxnum)
> -RVVCALL(OPFVF2, vfmax_vf_w, OP_UUU_W, H4, H4, float32_maxnum)
> -RVVCALL(OPFVF2, vfmax_vf_d, OP_UUU_D, H8, H8, float64_maxnum)
> +RVVCALL(OPFVF2, vfmax_vf_h, OP_UUU_H, H2, H2, float16_maxnum_noprop)
> +RVVCALL(OPFVF2, vfmax_vf_w, OP_UUU_W, H4, H4, float32_maxnum_noprop)
> +RVVCALL(OPFVF2, vfmax_vf_d, OP_UUU_D, H8, H8, float64_maxnum_noprop)
>  GEN_VEXT_VF(vfmax_vf_h, 2, 2, clearh)
>  GEN_VEXT_VF(vfmax_vf_w, 4, 4, clearl)
>  GEN_VEXT_VF(vfmax_vf_d, 8, 8, clearq)


-- 
Alex Bennée



Re: [PATCH v4 11/40] tests/vm: change scripts to use self._config

2020-07-10 Thread Alex Bennée


Alex Bennée  writes:

> From: Robert Foley 
>
> This change converts existing scripts to using for example self.ROOT_PASS,
> to self._config['root_pass'].
> We made similar changes for GUEST_USER, and GUEST_PASS.
> This allows us also to remove the change in basevm.py,
> which adds __getattr__ for backwards compatibility.
>
> Signed-off-by: Robert Foley 
> Reviewed-by: Peter Puhov 
> Signed-off-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Message-Id: <20200601211421.1277-8-robert.fo...@linaro.org>
> ---
>  tests/vm/basevm.py | 11 ++-
>  tests/vm/fedora| 17 +
>  tests/vm/freebsd   | 16 
>  tests/vm/netbsd| 19 ++-
>  tests/vm/openbsd   | 17 +
>  5 files changed, 38 insertions(+), 42 deletions(-)
>
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 5fd66f6b26a..f716798b405 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -183,13 +183,6 @@ class BaseVM(object):
>  self.console_init(timeout=timeout)
>  self.console_wait(wait_string)
>  
> -def __getattr__(self, name):
> -# Support direct access to config by key.
> -# for example, access self._config['cpu'] by self.cpu
> -if name.lower() in self._config.keys():
> -return self._config[name.lower()]
> -return object.__getattribute__(self, name)
> -
>  def _download_with_cache(self, url, sha256sum=None, sha512sum=None):
>  def check_sha256sum(fname):
>  if not sha256sum:
> @@ -239,13 +232,13 @@ class BaseVM(object):
>  return r
>  
>  def ssh(self, *cmd):
> -return self._ssh_do(self.GUEST_USER, cmd, False)
> +return self._ssh_do(self._config["guest_user"], cmd, False)
>  
>  def ssh_root(self, *cmd):
>  return self._ssh_do("root", cmd, False)
>  
>  def ssh_check(self, *cmd):
> -self._ssh_do(self.GUEST_USER, cmd, True)
> +self._ssh_do(self._config["guest_user"], cmd, True)
>  
>  def ssh_root_check(self, *cmd):
>  self._ssh_do("root", cmd, True)
> diff --git a/tests/vm/fedora b/tests/vm/fedora
> index a9195670f4b..b2b478fdbca 100755
> --- a/tests/vm/fedora
> +++ b/tests/vm/fedora
> @@ -108,20 +108,20 @@ class FedoraVM(basevm.BaseVM):
>  
>  self.console_wait_send("7) [!] Root password", "7\n")
>  self.console_wait("Password:")
> -self.console_send("%s\n" % self.ROOT_PASS)
> +self.console_send("%s\n" % self._config["root_pass"])
>  self.console_wait("Password (confirm):")
> -self.console_send("%s\n" % self.ROOT_PASS)
> +self.console_send("%s\n" % self._config["root_pass"])
>  
>  self.console_wait_send("8) [ ] User creation", "8\n")
>  self.console_wait_send("1) [ ] Create user",   "1\n")
>  self.console_wait_send("3) User name", "3\n")
> -self.console_wait_send("ENTER:", "%s\n" % self.GUEST_USER)
> +self.console_wait_send("ENTER:", "%s\n" % self._config["guest_user"])
>  self.console_wait_send("4) [ ] Use password",  "4\n")
>  self.console_wait_send("5) Password",  "5\n")
>  self.console_wait("Password:")
> -self.console_send("%s\n" % self.GUEST_PASS)
> +self.console_send("%s\n" % self._config["guest_pass"])
>  self.console_wait("Password (confirm):")
> -self.console_send("%s\n" % self.GUEST_PASS)
> +self.console_send("%s\n" % self._config["guest_pass"])
>  self.console_wait_send("7) Groups","c\n")
>  
>  while True:
> @@ -139,7 +139,7 @@ class FedoraVM(basevm.BaseVM):
>  if good:
>  break
>  time.sleep(10)
> -self.console_send("r\n" % self.GUEST_PASS)
> +self.console_send("r\n" % self._config["guest_pass"])
>  
>  self.console_wait_send("'b' to begin install", "b\n")
>  
> @@ -150,12 +150,13 @@ class FedoraVM(basevm.BaseVM):
>  
>  # setup qemu user
>  prompt = " ~]$"
> -self.console_ssh_init(prompt, self.GUEST_USER, self.GUEST_PASS)
> +self.console_ssh_init(prompt, self._config["guest_user"],
> +  self._config["guest_pass"])
>  self.console_wait_send(prompt, "exit\n")
>  
>  # setup root user
>  prompt = " ~]#"
> -self.console_ssh_init(prompt, "root", self.ROOT_PASS)
> +self.console_ssh_init(prompt, "root", self._config["root_pass"])
>  self.console_sshd_config(prompt)
>  
>  # setup virtio-blk #1 (tarfile)
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index f87db2b126e..29252fa4a64 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -113,9 +113,9 @@ class FreeBSDVM(basevm.BaseVM):
>  
>  # post-install configuration
>  self.console_wait("New Password:")
> -self.console_send("%s\n" % self.ROOT_PASS)
> +

Re: [RFC 14/65] target/riscv: rvv-0.9: stride load and store instructions

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
>  # *** Vector loads and stores are encoded within LOADFP/STORE-FP ***
> -vlb_v  ... 100 . 0 . 000 . 111 @r2_nfvm
> -vlh_v  ... 100 . 0 . 101 . 111 @r2_nfvm
> -vlw_v  ... 100 . 0 . 110 . 111 @r2_nfvm

Again, something you can't do until 0.7.1 is not supported.

If you don't want to simultaneously support 0.7.1 and 0.9/1.0, then you should
simply remove 0.7.1 in the first patch, so that there's no confusion.

Is the rest of it mostly renaming?  You should definitely expand on what you're
doing within each patch description.  A description of what has changed in the
spec since 0.7.1 will help the reviewer validate that you've gotten all of the
corner cases.

I am going to stop reviewing this patch series now, as I expect that most of
the remaining patches will have similar comments.


r~



Re: [RFC 13/65] target/riscv: rvv-0.9: configure instructions

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> -static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl *a)
> +static bool trans_vsetvl(DisasContext *s, arg_vsetvl *a)

Do not mix this change with anything else.

> +rd = tcg_const_i32(a->rd);
> +rs1 = tcg_const_i32(a->rs1);

Any time you put a register number into a tcg const, there's probably a better
way to do things.

> -/* Using x0 as the rs1 register specifier, encodes an infinite AVL */
> -if (a->rs1 == 0) {
> -/* As the mask is at least one bit, RV_VLEN_MAX is >= VLMAX */
> -s1 = tcg_const_tl(RV_VLEN_MAX);
> -} else {
> -s1 = tcg_temp_new();
> -gen_get_gpr(s1, a->rs1);
> -}

E.g. this code should be kept, and add

if (a->rd == 0 && a->rs1 == 0) {
s1 = tcg_temp_new();
tcg_gen_mov_tl(s1, cpu_vl);
} else ...


> +if ((sew > cpu->cfg.elen)
> +|| vill
> +|| vflmul < ((float)sew / cpu->cfg.elen)
> +|| (ediv != 0)
> +|| (reserved != 0)) {
>  /* only set vill bit. */
>  env->vtype = FIELD_DP64(0, VTYPE, VILL, 1);
> -env->vl = 0;
> -env->vstart = 0;
>  return 0;
>  }

You do need to check 0.7.1 so long as it's supported.


r~



Re: [RFC 12/65] target/riscv: rvv-0.9: update check functions

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> +#define REQUIRE_RVV do {\
> +if (s->mstatus_vs == 0) \
> +return false;   \
> +} while (0)

You've used this macro already back in patch 7.  I guess it should not have
been there?  Or this bit belongs there, one or the other.

I think this patch requires a description and justification.  I have no idea
why you are replacing

> -return (vext_check_isa_ill(s) &&
> -vext_check_overlap_mask(s, a->rd, a->vm, false) &&
> -vext_check_reg(s, a->rd, false) &&
> -vext_check_reg(s, a->rs2, false) &&
> -vext_check_reg(s, a->rs1, false));

with invisible returns

> +REQUIRE_RVV;
> +VEXT_CHECK_ISA_ILL(s);
> +VEXT_CHECK_SSS(s, a->rd, a->rs1, a->rs2, a->vm, true);
> +return true;


r~



Re: [RFC 11/65] target/riscv: rvv-0.9: add fractional LMUL, VTA and VMA

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> -static void vext_clear(void *tail, uint32_t cnt, uint32_t tot)
> +static void vext_clear(void *tail, uint32_t vta, uint32_t cnt, uint32_t tot)
>  {
> +/* tail element undisturbed */
> +if (vta == 0) {
> +return;
> +}
> +
>  memset(tail, 0, tot - cnt);
>  }

First, this patch is doing too much.  LMUL should definitely be split from
VTA/VMA; they are not functionally related.

Second, it would be in-spec to simply do nothing for VTA/VMA, except validate
their setting within VTYPE.

Third, the spec talks about setting "agnostic" values to 1s, not 0s as you are
doing here.  So there's definitely something wrong.


r~



Re: [PATCH v7 14/47] stream: Deal with filters

2020-07-10 Thread Andrey Shinkevich

On 10.07.2020 18:24, Max Reitz wrote:

On 09.07.20 16:52, Andrey Shinkevich wrote:

On 25.06.2020 18:21, Max Reitz wrote:

Because of the (not so recent anymore) changes that make the stream job
independent of the base node and instead track the node above it, we
have to split that "bottom" node into two cases: The bottom COW node,
and the node directly above the base node (which may be an R/W filter
or the bottom COW node).

Signed-off-by: Max Reitz 
---
   qapi/block-core.json |  4 +++
   block/stream.c   | 63 
   blockdev.c   |  4 ++-
   3 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index b20332e592..df87855429 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2486,6 +2486,10 @@
   # On successful completion the image file is updated to drop the
backing file
   # and the BLOCK_JOB_COMPLETED event is emitted.
   #
+# In case @device is a filter node, block-stream modifies the first
non-filter
+# overlay node below it to point to base's backing node (or NULL if
@base was
+# not specified) instead of modifying @device itself.
+#
   # @job-id: identifier for the newly-created block job. If
   #  omitted, the device name will be used. (Since 2.7)
   #
diff --git a/block/stream.c b/block/stream.c
index aa2e7af98e..b9c1141656 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -31,7 +31,8 @@ enum {
     typedef struct StreamBlockJob {
   BlockJob common;
-    BlockDriverState *bottom;
+    BlockDriverState *base_overlay; /* COW overlay (stream from this) */
+    BlockDriverState *above_base;   /* Node directly above the base */

Keeping the base_overlay is enough to complete the stream job.

Depends on the definition.  If we decide it isn’t enough, then it isn’t
enough.


The above_base may disappear during the job and we can't rely on it.

In this version of this series, it may not, because the chain is frozen.
  So the above_base cannot disappear.


Once we insert a filter above the top bs of the stream job, the parallel 
jobs in


the iotests #030 will fail with 'frozen link error'. It is because of the

independent parallel stream or commit jobs that insert/remove their filters

asynchroniously.



We can discuss whether we should allow it to disappear, but I think not.

The problem is, we need something to set as the backing file after
streaming.  How do we figure out what that should be?  My proposal is we
keep above_base and use its immediate child.


We can do the same with the base_overlay.

If the backing node turns out to be a filter, the proper backing child will

be set after the filter is removed. So, we shouldn't care.



If we don’t keep above_base, then we’re basically left guessing as to
what should be the backing file after the stream job.


   BlockdevOnError on_error;
   char *backing_file_str;
   bool bs_read_only;
@@ -53,7 +54,7 @@ static void stream_abort(Job *job)
     if (s->chain_frozen) {
   BlockJob *bjob = >common;
-    bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->bottom);
+    bdrv_unfreeze_backing_chain(blk_bs(bjob->blk), s->above_base);
   }
   }
   @@ -62,14 +63,15 @@ static int stream_prepare(Job *job)
   StreamBlockJob *s = container_of(job, StreamBlockJob, common.job);
   BlockJob *bjob = >common;
   BlockDriverState *bs = blk_bs(bjob->blk);
-    BlockDriverState *base = backing_bs(s->bottom);
+    BlockDriverState *unfiltered_bs = bdrv_skip_filters(bs);
+    BlockDriverState *base = bdrv_filter_or_cow_bs(s->above_base);

The initial base node may be a top node for a concurrent commit job and

may disappear.

Then it would just be replaced by another node, though, so above_base
keeps a child.  The @base here is not necessarily the initial @base, and
that’s intentional.


Not really. In my example, above_base becomes a dangling

pointer because after the commit job finishes, its filter that should 
belong to the


commit job frozen chain will be deleted. If we freeze the link to the 
above_base


for this job, the iotests #30 will not pass.


base = bdrv_filter_or_cow_bs(s->base_overlay) is more reliable.

But also wrong.  The point of keeping above_base around is to get its
child here to use that child as the new backing child of the top node.


   Error *local_err = NULL;
   int ret = 0;
   -    bdrv_unfreeze_backing_chain(bs, s->bottom);
+    bdrv_unfreeze_backing_chain(bs, s->above_base);
   s->chain_frozen = false;
   -    if (bs->backing) {
+    if (bdrv_cow_child(unfiltered_bs)) {
   const char *base_id = NULL, *base_fmt = NULL;
   if (base) {
   base_id = s->backing_file_str;
@@ -77,8 +79,8 @@ static int stream_prepare(Job *job)
   base_fmt = base->drv->format_name;
   }
   }
-    bdrv_set_backing_hd(bs, base, _err);
-    ret = bdrv_change_backing_file(bs, base_id, base_fmt);
+    

Re: [RFC PATCH 2/3] hw/i2c/smbus_eeprom: Add description based on child name

2020-07-10 Thread Eduardo Habkost
On Fri, Jul 10, 2020 at 11:05:29AM +0200, Philippe Mathieu-Daudé wrote:
> +Stefan for tracing PoV
> 
> On 7/9/20 9:48 PM, Eduardo Habkost wrote:
> > On Fri, Jun 26, 2020 at 04:26:33PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 6/26/20 1:00 PM, BALATON Zoltan wrote:
> >>> On Fri, 26 Jun 2020, Philippe Mathieu-Daudé wrote:
>  Suggested-by: Markus Armbruster 
>  Signed-off-by: Philippe Mathieu-Daudé 
>  ---
>  hw/i2c/smbus_eeprom.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
>  diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>  index 879fd7c416..22ba7b20d4 100644
>  --- a/hw/i2c/smbus_eeprom.c
>  +++ b/hw/i2c/smbus_eeprom.c
>  @@ -47,6 +47,7 @@ typedef struct SMBusEEPROMDevice {
>      uint8_t *init_data;
>      uint8_t offset;
>      bool accessed;
>  +    char *description;
>  } SMBusEEPROMDevice;
> 
>  static uint8_t eeprom_receive_byte(SMBusDevice *dev)
>  @@ -134,7 +135,9 @@ static void smbus_eeprom_realize(DeviceState *dev,
>  Error **errp)
>      smbus_eeprom_reset(dev);
>      if (eeprom->init_data == NULL) {
>          error_setg(errp, "init_data cannot be 
>  NULL");
>  +        return;
>      }
>  +    eeprom->description =
>  object_get_canonical_path_component(OBJECT(dev));
>  }
> 
>  static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data)
> >>>
> >>> What is this for? Shouldn't this description field be in some parent
> >>> object and whatever wants to print it could use the
> >>> object_get_canonical_path_component() as default value if it's not set
> >>> instead of adding more boiler plate to every object?
> >>
> >> You are right, if we want to use this field generically, it should be
> >> a static Object field. I'll defer that question to Eduardo/Markus.
> > 
> > I don't understand what's the question here.  What would be the
> > purpose of a static Object field, and why it would be better than
> > simply calling object_get_canonical_path_component() when you
> > need it?
> 
> Because when reading a 8KB EEPROM with tracing enabled we end
> up calling:
> 
> 8192 g_hash_table_iter_init()
> 8192 g_hash_table_iter_next()
> 8192 object_property_iter_init()
> 8192 object_property_iter_next()
> 8192 g_hash_table_add()
> 8192 g_strdup()
> 8192 g_free()
> 
> Which is why I added the SMBusEEPROMDeviceState::description
> field, to call it once and cache it. But Zoltan realized this
> could benefit all QOM objects, not this single one.
> 
> So the question is, is it OK to make this a cached field
> in object_get_canonical_path_component()?

That's what I was thinking of, but now I see that
object_get_canonical_path_component() is an inconvenient API
because it requires callers to g_free() the return value.

We could change object_get_canonical_path_component() to not
require callers to call g_free(), or create a new mechanism to
get the object name like you suggested (and then get rid of most
of the existing uses of object_get_canonical_path_component()).

> 
> Something like (incomplete):
> 
> -- >8 --
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -642,6 +642,7 @@ static void object_property_del_child(Object *obj,
> Object *child)
>  break;
>  }
>  }
> +g_free(child->canonical_path_component);
>  }
> 
>  void object_unparent(Object *obj)
> @@ -1666,6 +1667,7 @@ object_property_add_child(Object *obj, const char
> *name,
>  op->resolve = object_resolve_child_property;
>  object_ref(child);
>  child->parent = obj;
> +child->canonical_path_component =
> object_get_canonical_path_component(child);
>  return op;
>  }
> 
> @@ -1901,6 +1903,10 @@ char *object_get_canonical_path_component(const
> Object *obj)
>  return NULL;
>  }
> 
> +if (obj->canonical_path_component) {
> +return obj->canonical_path_component;
> +}
> +
>  g_hash_table_iter_init(, obj->parent->properties);
>  while (g_hash_table_iter_next(, NULL, (gpointer *))) {
>  if (!object_property_is_child(prop)) {
> @@ -1908,7 +1914,8 @@ char *object_get_canonical_path_component(const
> Object *obj)
>  }
> 
>  if (prop->opaque == obj) {
> -return g_strdup(prop->name);
> +obj->canonical_path_component_cached = g_strdup(prop->name);
> +return obj->canonical_path_component_cached;
>  }
>  }
> 
> ---
> 

-- 
Eduardo




Re: [RFC 10/65] target/riscv: rvv-0.9: remove MLEN calculations

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> As in RVV 0.9 design, MLEN is hardcoded with value 1 (Section 4.5).
> Thus, remove all MLEN related calculations.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c |  44 +
>  target/riscv/internals.h|   9 +-
>  target/riscv/translate.c|   2 -
>  target/riscv/vector_helper.c| 252 ++--
>  4 files changed, 116 insertions(+), 191 deletions(-)

You can't do this until you remove 0.7.1 support.


r~



Re: [PATCH v11 00/34] Add subcluster allocation to qcow2

2020-07-10 Thread no-reply
Patchew URL: https://patchew.org/QEMU/cover.1594396418.git.be...@igalia.com/



Hi,

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

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

  CC  block/blkverify.o
  CC  block/blkreplay.o
/tmp/qemu-test/src/block/qcow2-cluster.c: In function 'qcow2_get_host_offset':
/tmp/qemu-test/src/block/qcow2-cluster.c:473:19: error: 'expected_type' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
 } else if (type != expected_type) {
   ^
/tmp/qemu-test/src/block/qcow2-cluster.c:449:25: note: 'expected_type' was 
declared here
 QCow2SubclusterType expected_type, type;
 ^
cc1: all warnings being treated as errors
make: *** [block/qcow2-cluster.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in 
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=21da7e0fdd084eea8b00971953a3ff2a', '-u', 
'1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-yoa6xyt4/src/docker-src.2020-07-10-13.26.25.25621:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=21da7e0fdd084eea8b00971953a3ff2a
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-yoa6xyt4/src'
make: *** [docker-run-test-quick@centos7] Error 2

real2m27.189s
user0m8.691s


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

Re: [RFC 09/65] target/riscv: rvv-0.9: add vlenb register

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 228b9bdb5d..871c2ddfa1 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -317,6 +317,7 @@ static void riscv_cpu_reset(DeviceState *dev)
>  env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
>  env->mcause = 0;
>  env->pc = env->resetvec;
> +env->vlenb = cpu->cfg.vlen >> 3;
>  #endif
>  cs->exception_index = EXCP_NONE;
>  env->load_res = -1;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c02690ed0d..81c85bf4c2 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -111,6 +111,7 @@ struct CPURISCVState {
>  target_ulong vl;
>  target_ulong vstart;
>  target_ulong vtype;
> +target_ulong vlenb;

I don't see that you need this.  The field is read-only, so the read_vlenb
function can just return

  env_archcpu(env)->cfg.vlen >> 3

directly.


r~



[PATCH 0/2] block: add logging facility for long standing IO requests

2020-07-10 Thread Denis V. Lunev
There are severe delays with IO requests processing if QEMU is running in
virtual machine or over software defined storage. Such delays potentially
results in unpredictable guest behavior. For example, guests over IDE or
SATA drive could remount filesystem read-only if write is performed
longer than 10 seconds.

Such reports are very complex to process. Some good starting point for this
seems quite reasonable. This patch provides one. It adds logging of such
potentially dangerous long IO operations.

Signed-off-by: Denis V. Lunev 
CC: Vladimir Sementsov-Ogievskiy 
CC: Kevin Wolf 
CC: Max Reitz 





Re: [RFC 08/65] target/riscv: rvv-0.9: update mstatus_vs by tb_flags

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> From: LIU Zhiwei 
> 
> Signed-off-by: LIU Zhiwei 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.h   | 2 ++
>  target/riscv/translate.c | 1 +
>  2 files changed, 3 insertions(+)

This belongs with the second half of the previous patch, the translate portion.


r~

> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0cf3fe9456..c02690ed0d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -361,6 +361,7 @@ void riscv_cpu_set_fflags(CPURISCVState *env, 
> target_ulong);
>  
>  #define TB_FLAGS_MMU_MASK   3
>  #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
> +#define TB_FLAGS_MSTATUS_VS MSTATUS_VS
>  
>  typedef CPURISCVState CPUArchState;
>  typedef RISCVCPU ArchCPU;
> @@ -411,6 +412,7 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState 
> *env, target_ulong *pc,
>  
>  #ifdef CONFIG_USER_ONLY
>  flags |= TB_FLAGS_MSTATUS_FS;
> +flags |= TB_FLAGS_MSTATUS_VS;
>  #else
>  flags |= cpu_mmu_index(env, 0);
>  if (riscv_cpu_fp_enabled(env)) {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index a806e33301..02b4204584 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -796,6 +796,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
> *dcbase, CPUState *cs)
>  ctx->pc_succ_insn = ctx->base.pc_first;
>  ctx->mem_idx = tb_flags & TB_FLAGS_MMU_MASK;
>  ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
> +ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
>  ctx->priv_ver = env->priv_ver;
>  #if !defined(CONFIG_USER_ONLY)
>  if (riscv_has_ext(env, RVH)) {
> 




[PATCH 2/2] block: add logging facility for long standing IO requests

2020-07-10 Thread Denis V. Lunev
There are severe delays with IO requests processing if QEMU is running in
virtual machine or over software defined storage. Such delays potentially
results in unpredictable guest behavior. For example, guests over IDE or
SATA drive could remount filesystem read-only if write is performed
longer than 10 seconds.

Such reports are very complex to process. Some good starting point for this
seems quite reasonable. This patch provides one. It adds logging of such
potentially dangerous long IO operations.

Signed-off-by: Denis V. Lunev 
CC: Vladimir Sementsov-Ogievskiy 
CC: Kevin Wolf 
CC: Max Reitz 
---
 block/accounting.c | 59 +-
 blockdev.c |  7 -
 include/block/accounting.h |  5 +++-
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/block/accounting.c b/block/accounting.c
index 8d41c8a83a..3002444fa5 100644
--- a/block/accounting.c
+++ b/block/accounting.c
@@ -27,7 +27,9 @@
 #include "block/accounting.h"
 #include "block/block_int.h"
 #include "qemu/timer.h"
+#include "qemu/log.h"
 #include "sysemu/qtest.h"
+#include "sysemu/block-backend.h"
 
 static QEMUClockType clock_type = QEMU_CLOCK_REALTIME;
 static const int qtest_latency_ns = NANOSECONDS_PER_SECOND / 1000;
@@ -41,10 +43,11 @@ void block_acct_init(BlockAcctStats *stats)
 }
 
 void block_acct_setup(BlockAcctStats *stats, bool account_invalid,
-  bool account_failed)
+  bool account_failed, unsigned latency_log_threshold_ms)
 {
 stats->account_invalid = account_invalid;
 stats->account_failed = account_failed;
+stats->latency_log_threshold_ms = latency_log_threshold_ms;
 }
 
 void block_acct_cleanup(BlockAcctStats *stats)
@@ -182,6 +185,58 @@ void block_latency_histograms_clear(BlockAcctStats *stats)
 }
 }
 
+static const char *block_account_type(enum BlockAcctType type)
+{
+switch (type) {
+case BLOCK_ACCT_READ:
+return "READ";
+case BLOCK_ACCT_WRITE:
+return "WRITE";
+case BLOCK_ACCT_FLUSH:
+return "DISCARD";
+case BLOCK_ACCT_UNMAP:
+return "TRUNCATE";
+case BLOCK_ACCT_NONE:
+return "NONE";
+case BLOCK_MAX_IOTYPE:
+break;
+}
+return "UNKNOWN";
+}
+
+static void block_acct_report_long(BlockAcctStats *stats,
+   BlockAcctCookie *cookie, int64_t latency_ns)
+{
+unsigned latency_ms = latency_ns / 100;
+int64_t start_time_host_s;
+char buf[64];
+struct tm t;
+BlockDriverState *bs;
+
+if (cookie->type == BLOCK_ACCT_NONE) {
+return;
+}
+if (stats->latency_log_threshold_ms == 0) {
+return;
+}
+if (latency_ms < stats->latency_log_threshold_ms) {
+return;
+}
+
+start_time_host_s = cookie->start_time_ns / 10ull;
+strftime(buf, sizeof(buf), "%m-%d %H:%M:%S",
+ localtime_r(_time_host_s, ));
+
+bs = blk_bs(blk_stats2blk(stats));
+qemu_log("long %s[%ld] IO request: %d.03%d since %s.%03d bs: %s(%s, %s)\n",
+ block_account_type(cookie->type), cookie->bytes,
+ (int)(latency_ms / 1000), (int)(latency_ms % 1000), buf,
+ (int)((cookie->start_time_ns / 100) % 1000),
+ bs == NULL ? "unknown" : bdrv_get_node_name(bs),
+ bs == NULL ? "unknown" : bdrv_get_format_name(bs),
+ bs == NULL ? "unknown" : bs->filename);
+}
+
 static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie 
*cookie,
  bool failed)
 {
@@ -222,6 +277,8 @@ static void block_account_one_io(BlockAcctStats *stats, 
BlockAcctCookie *cookie,
 
 qemu_mutex_unlock(>lock);
 
+block_acct_report_long(stats, cookie, latency_ns);
+
 cookie->type = BLOCK_ACCT_NONE;
 }
 
diff --git a/blockdev.c b/blockdev.c
index 31d5eaf6bf..d87260958c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -625,7 +625,8 @@ static BlockBackend *blockdev_init(const char *file, QDict 
*bs_opts,
 
 bs->detect_zeroes = detect_zeroes;
 
-block_acct_setup(blk_get_stats(blk), account_invalid, account_failed);
+block_acct_setup(blk_get_stats(blk), account_invalid, account_failed,
+qemu_opt_get_number(opts, "latency-log-threshold", 0));
 
 if (!parse_stats_intervals(blk_get_stats(blk), interval_list, errp)) {
 blk_unref(blk);
@@ -3740,6 +3741,10 @@ QemuOptsList qemu_common_drive_opts = {
 .type = QEMU_OPT_BOOL,
 .help = "whether to account for failed I/O operations "
 "in the statistics",
+},{
+.name = "latency-log-threshold",
+.type = QEMU_OPT_STRING,
+.help = "threshold for long I/O report (disabled if <=0), in ms",
 },
 { /* end of list */ }
 },
diff --git a/include/block/accounting.h b/include/block/accounting.h
index 878b4c3581..c3ea25f9aa 100644
--- a/include/block/accounting.h
+++ 

Re: [RFC 07/65] target/riscv: rvv-0.9: add vector context status

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> From: LIU Zhiwei 
> 
> Signed-off-by: LIU Zhiwei 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.h  |  4 ++
>  target/riscv/cpu_bits.h |  1 +
>  target/riscv/cpu_helper.c   | 13 ++
>  target/riscv/csr.c  | 25 ++-
>  target/riscv/insn_trans/trans_rvv.inc.c | 57 +
>  target/riscv/translate.c| 32 ++
>  6 files changed, 123 insertions(+), 9 deletions(-)

BTW, I think this should be split.

One patch for the csr.c changes, another for the translate changes.


r~



[PATCH 1/2] block/block-backend: add converter from BlockAcctStats to BlockBackend

2020-07-10 Thread Denis V. Lunev
Right now BlockAcctStats is always reside on BlockBackend. This structure
is not used in any other place. Thus we are able to create a converter
from one pointer to another.

Signed-off-by: Denis V. Lunev 
CC: Vladimir Sementsov-Ogievskiy 
CC: Kevin Wolf 
CC: Max Reitz 
---
 block/block-backend.c  | 5 +
 include/sysemu/block-backend.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/block/block-backend.c b/block/block-backend.c
index 6936b25c83..e77a7e468e 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2143,6 +2143,11 @@ BlockAcctStats *blk_get_stats(BlockBackend *blk)
 return >stats;
 }
 
+BlockBackend *blk_stats2blk(BlockAcctStats *s)
+{
+return container_of(s, BlockBackend, stats);
+}
+
 void *blk_aio_get(const AIOCBInfo *aiocb_info, BlockBackend *blk,
   BlockCompletionFunc *cb, void *opaque)
 {
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8203d7f6f9..bd4694e7bc 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -227,6 +227,7 @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier 
*notify);
 void blk_io_plug(BlockBackend *blk);
 void blk_io_unplug(BlockBackend *blk);
 BlockAcctStats *blk_get_stats(BlockBackend *blk);
+BlockBackend *blk_stats2blk(BlockAcctStats *stats);
 BlockBackendRootState *blk_get_root_state(BlockBackend *blk);
 void blk_update_root_state(BlockBackend *blk);
 bool blk_get_detect_zeroes_from_root_state(BlockBackend *blk);
-- 
2.17.1




Re: [RFC 07/65] target/riscv: rvv-0.9: add vector context status

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -376,6 +376,7 @@
>  #define MSTATUS_SPP 0x0100
>  #define MSTATUS_MPP 0x1800
>  #define MSTATUS_FS  0x6000
> +#define MSTATUS_VS  0x0600
>  #define MSTATUS_XS  0x00018000

Please sort VS up below SPP, so that the bits are in order.

> @@ -180,6 +180,7 @@ static int write_fcsr(CPURISCVState *env, int csrno, 
> target_ulong val)
>  return -1;
>  }
>  env->mstatus |= MSTATUS_FS;
> +env->mstatus |= MSTATUS_VS;
>  #endif
>  env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
>  if (vs(env, csrno) >= 0) {

Does rvv 0.9 still have the vector fields in FCSR, or are they only present in
the new VCSR?

> @@ -420,7 +442,7 @@ static int write_mstatus(CPURISCVState *env, int csrno, 
> target_ulong val)
>  mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
>  MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
>  MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> -MSTATUS_TW;
> +MSTATUS_TW | MSTATUS_VS;

Since 0.7.1 does not have the VS field, you might want to force VS to dirty in
the written mstatus.  Correspondingly, you should remove VS from the returned
mstatus in read_mstatus.

You appear to be missing a change to riscv_cpu_swap_hypervisor_regs.

That makes all of the riscv_cpu_vector_enabled checks return true for 0.7.1,
which afaik is correct.

> @@ -245,7 +250,9 @@ static bool ld_us_op(DisasContext *s, arg_r2nfvm *a, 
> uint8_t seq)
>  data = FIELD_DP32(data, VDATA, VM, a->vm);
>  data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
>  data = FIELD_DP32(data, VDATA, NF, a->nf);
> -return ldst_us_trans(a->rd, a->rs1, data, fn, s);
> +ret = ldst_us_trans(a->rd, a->rs1, data, fn, s);
> +mark_vs_dirty(s);
> +return ret;

Just push the mark_vs_dirty call into ldst_us_trans.

> @@ -382,7 +390,9 @@ static bool ld_stride_op(DisasContext *s, arg_rnfvm *a, 
> uint8_t seq)
>  data = FIELD_DP32(data, VDATA, VM, a->vm);
>  data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
>  data = FIELD_DP32(data, VDATA, NF, a->nf);
> -return ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +ret  = ldst_stride_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +mark_vs_dirty(s);
> +return ret;

Likewise.

> @@ -510,7 +521,9 @@ static bool ld_index_op(DisasContext *s, arg_rnfvm *a, 
> uint8_t seq)
>  data = FIELD_DP32(data, VDATA, VM, a->vm);
>  data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
>  data = FIELD_DP32(data, VDATA, NF, a->nf);
> -return ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +ret = ldst_index_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +mark_vs_dirty(s);
> +return ret;

Likewise.

> @@ -632,7 +646,9 @@ static bool ldff_op(DisasContext *s, arg_r2nfvm *a, 
> uint8_t seq)
>  data = FIELD_DP32(data, VDATA, VM, a->vm);
>  data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
>  data = FIELD_DP32(data, VDATA, NF, a->nf);
> -return ldff_trans(a->rd, a->rs1, data, fn, s);
> +ret = ldff_trans(a->rd, a->rs1, data, fn, s);
> +mark_vs_dirty(s);

Likewise.

> @@ -741,7 +758,9 @@ static bool amo_op(DisasContext *s, arg_rwdvm *a, uint8_t 
> seq)
>  data = FIELD_DP32(data, VDATA, VM, a->vm);
>  data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
>  data = FIELD_DP32(data, VDATA, WD, a->wd);
> -return amo_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +ret = amo_trans(a->rd, a->rs1, a->rs2, data, fn, s);
> +mark_vs_dirty(s);
> +return ret;

Likewise.

> @@ -911,9 +932,12 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn 
> *gvec_fn,
>  
>  tcg_temp_free_i64(src1);
>  tcg_temp_free(tmp);
> +mark_vs_dirty(s);
>  return true;
>  }
> -return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
> +ret = opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
> +mark_vs_dirty(s);
> +return ret;

Likewise.  And more.

> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 9632e79cf3..a806e33301 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -47,6 +47,7 @@ typedef struct DisasContext {
>  bool virt_enabled;
>  uint32_t opcode;
>  uint32_t mstatus_fs;
> +uint32_t mstatus_vs;

Missing a change to riscv_tr_init_disas_context to initialize this.


r~



Re: [PATCH 0/2] iotests: More _filter_img_create fixes

2020-07-10 Thread John Snow



On 7/10/20 12:32 PM, Max Reitz wrote:
> Hi,
> 
> I’m sorry.
> 
> John, could I ask you to test whether this series fixes the problems
> you’re seeing?
> 

This is based on kwolf/block, I see.

By the time you return to reading work email, this link will have
information for you:

https://travis-ci.org/github/jnsnow/qemu/jobs/706960907

> 
> Max Reitz (2):
>   iotests: Drop readarray from _do_filter_img_create
>   iotests: Set LC_ALL=C for sort
> 
>  tests/qemu-iotests/common.filter | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

-- 
—js




[PULL v2 00/32] AVR port

2020-07-10 Thread Philippe Mathieu-Daudé
Since v1:

Fixed issue on big-endian host reported by Peter Maydell.

Possible false-positives from checkpatch:

  WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?

The following changes since commit f2a1cf9180f63e88bb38ff21c169da97c3f2bad5:

  Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2020-07-07-v2'=
 into staging (2020-07-10 14:41:23 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/avr-port-20200710

for you to fetch changes up to 23623ca1f27427d76cc111eb567cac6ce18dab3a:

  target/avr/disas: Fix store instructions display order (2020-07-10 18:13:39=
 +0200)


8bit AVR port from Michael Rolnik.

Michael started to work on the AVR port few years ago [*] and kept
improving the code over various series.

List of people who help him (in chronological order):
- Richard Henderson
- Sarah Harris and Edward Robbins
- Philippe Mathieu-Daud=C3=A9 and Aleksandar Markovic
- Pavel Dovgalyuk
- Thomas Huth

[*] The oldest contribution I could find on the list is from 2016:
https://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg02985.html



Michael Rolnik (25):
  target/avr: Add basic parameters of the new platform
  target/avr: Introduce basic CPU class object
  target/avr: CPU class: Add interrupt handling support
  target/avr: CPU class: Add memory management support
  target/avr: CPU class: Add migration support
  target/avr: CPU class: Add GDB support
  target/avr: Introduce enumeration AVRFeature
  target/avr: Add definitions of AVR core types
  target/avr: Add instruction helpers
  target/avr: Add instruction translation - Register definitions
  target/avr: Add instruction translation - Arithmetic and Logic
Instructions
  target/avr: Add instruction translation - Branch Instructions
  target/avr: Add instruction translation - Data Transfer Instructions
  target/avr: Add instruction translation - Bit and Bit-test
Instructions
  target/avr: Add instruction translation - MCU Control Instructions
  target/avr: Add instruction translation - CPU main translation
function
  target/avr: Initialize TCG register variables
  target/avr: Add support for disassembling via option '-d in_asm'
  target/avr: Register AVR support with the rest of QEMU
  tests/machine-none: Add AVR support
  hw/char: avr: Add limited support for USART peripheral
  hw/timer: avr: Add limited support for 16-bit timer peripheral
  hw/misc: avr: Add limited support for power reduction device
  tests/boot-serial: Test some Arduino boards (AVR based)
  tests/acceptance: Test the Arduino MEGA2560 board

Philippe Mathieu-Daud=C3=A9 (6):
  hw/avr: Add support for loading ELF/raw binaries
  hw/avr: Add some ATmega microcontrollers
  hw/avr: Add limited support for some Arduino boards
  target/avr/cpu: Drop tlb_flush() in avr_cpu_reset()
  target/avr/cpu: Fix $PC displayed address
  target/avr/disas: Fix store instructions display order

Thomas Huth (1):
  target/avr: Add section into QEMU documentation

 docs/system/target-avr.rst   |   37 +
 docs/system/targets.rst  |1 +
 configure|7 +
 default-configs/avr-softmmu.mak  |5 +
 qapi/machine.json|3 +-
 hw/avr/atmega.h  |   48 +
 hw/avr/boot.h|   33 +
 include/disas/dis-asm.h  |   19 +
 include/elf.h|4 +
 include/hw/char/avr_usart.h  |   93 +
 include/hw/misc/avr_power.h  |   46 +
 include/hw/timer/avr_timer16.h   |   94 +
 include/sysemu/arch_init.h   |1 +
 target/avr/cpu-param.h   |   36 +
 target/avr/cpu-qom.h |   53 +
 target/avr/cpu.h |  256 +++
 target/avr/helper.h  |   29 +
 target/avr/insn.decode   |  187 ++
 arch_init.c  |2 +
 hw/avr/arduino.c |  149 ++
 hw/avr/atmega.c  |  458 +
 hw/avr/boot.c|  115 ++
 hw/char/avr_usart.c  |  320 
 hw/misc/avr_power.c  |  113 ++
 hw/timer/avr_timer16.c   |  621 ++
 target/avr/cpu.c |  366 
 target/avr/disas.c   |  245 +++
 target/avr/gdbstub.c |   84 +
 target/avr/helper.c  |  348 
 target/avr/machine.c |  119 ++
 target/avr/translate.c   | 3061 ++
 tests/qtest/boot-serial-test.c   |   11 +
 tests/qtest/machine-none-test.c  |1 +
 MAINTAINERS  |   30 +
 gdb-xml/avr-cpu.xml  |   49 +
 hw/Kconfig   |1 +
 hw/avr/Kconfig   |9 +
 hw/avr/Makefile.objs |3 +
 hw/char/Kconfig  |3 +
 hw/char/Makefile.objs|1 +
 hw/misc/Kconfig  |3 +
 hw/misc/Makefile.objs|2

[PULL v2 09/32] target/avr: Add instruction helpers

2020-07-10 Thread Philippe Mathieu-Daudé
From: Michael Rolnik 

Add helpers for instructions that need to interact with QEMU. Also,
add stubs for unimplemented instructions. Instructions SPM and WDR
are left unimplemented because they require emulation of complex
peripherals. The implementation of instruction SLEEP is very limited
due to the lack of peripherals to generate wake interrupts. Memory
access instructions are implemented here because some address ranges
actually refer to CPU registers.

Signed-off-by: Michael Rolnik 
Signed-off-by: Richard Henderson 
Signed-off-by: Aleksandar Markovic 
Tested-by: Philippe Mathieu-Daudé 
Reviewed-by: Aleksandar Markovic 
Signed-off-by: Thomas Huth 
Message-Id: <20200705140315.260514-10-h...@tuxfamily.org>
[PMD: Replace cpu_physical_memory() API by address_space_ldst()
  API to fix running on big-endian host,
  reported and suggested by Peter Maydell]
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/avr/helper.h |  29 ++
 target/avr/helper.c | 209 
 2 files changed, 238 insertions(+)
 create mode 100644 target/avr/helper.h

diff --git a/target/avr/helper.h b/target/avr/helper.h
new file mode 100644
index 00..8e1ae7fda0
--- /dev/null
+++ b/target/avr/helper.h
@@ -0,0 +1,29 @@
+/*
+ * QEMU AVR CPU helpers
+ *
+ * Copyright (c) 2016-2020 Michael Rolnik
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+ * 
+ */
+
+DEF_HELPER_1(wdr, void, env)
+DEF_HELPER_1(debug, void, env)
+DEF_HELPER_1(break, void, env)
+DEF_HELPER_1(sleep, void, env)
+DEF_HELPER_1(unsupported, void, env)
+DEF_HELPER_3(outb, void, env, i32, i32)
+DEF_HELPER_2(inb, tl, env, i32)
+DEF_HELPER_3(fullwr, void, env, i32, i32)
+DEF_HELPER_2(fullrd, tl, env, i32)
diff --git a/target/avr/helper.c b/target/avr/helper.c
index d6985ff3f4..77bd9bc050 100644
--- a/target/avr/helper.c
+++ b/target/avr/helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "exec/address-spaces.h"
 #include "exec/helper-proto.h"
 
 bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
@@ -137,3 +138,211 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 
 return true;
 }
+
+/*
+ *  helpers
+ */
+
+void helper_sleep(CPUAVRState *env)
+{
+CPUState *cs = env_cpu(env);
+
+cs->exception_index = EXCP_HLT;
+cpu_loop_exit(cs);
+}
+
+void helper_unsupported(CPUAVRState *env)
+{
+CPUState *cs = env_cpu(env);
+
+/*
+ *  I count not find what happens on the real platform, so
+ *  it's EXCP_DEBUG for meanwhile
+ */
+cs->exception_index = EXCP_DEBUG;
+if (qemu_loglevel_mask(LOG_UNIMP)) {
+qemu_log("UNSUPPORTED\n");
+cpu_dump_state(cs, stderr, 0);
+}
+cpu_loop_exit(cs);
+}
+
+void helper_debug(CPUAVRState *env)
+{
+CPUState *cs = env_cpu(env);
+
+cs->exception_index = EXCP_DEBUG;
+cpu_loop_exit(cs);
+}
+
+void helper_break(CPUAVRState *env)
+{
+CPUState *cs = env_cpu(env);
+
+cs->exception_index = EXCP_DEBUG;
+cpu_loop_exit(cs);
+}
+
+void helper_wdr(CPUAVRState *env)
+{
+CPUState *cs = env_cpu(env);
+
+/* WD is not implemented yet, placeholder */
+cs->exception_index = EXCP_DEBUG;
+cpu_loop_exit(cs);
+}
+
+/*
+ * This function implements IN instruction
+ *
+ * It does the following
+ * a.  if an IO register belongs to CPU, its value is read and returned
+ * b.  otherwise io address is translated to mem address and physical memory
+ * is read.
+ * c.  it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
+ *
+ */
+target_ulong helper_inb(CPUAVRState *env, uint32_t port)
+{
+target_ulong data = 0;
+
+switch (port) {
+case 0x38: /* RAMPD */
+data = 0xff & (env->rampD >> 16);
+break;
+case 0x39: /* RAMPX */
+data = 0xff & (env->rampX >> 16);
+break;
+case 0x3a: /* RAMPY */
+data = 0xff & (env->rampY >> 16);
+break;
+case 0x3b: /* RAMPZ */
+data = 0xff & (env->rampZ >> 16);
+break;
+case 0x3c: /* EIND */
+data = 0xff & (env->eind >> 16);
+break;
+case 0x3d: /* SPL */
+data = env->sp & 0x00ff;
+break;
+case 0x3e: /* SPH */
+data = env->sp >> 8;
+break;
+case 0x3f: /* SREG */
+data = cpu_get_sreg(env);
+break;
+default:
+   

Re: [RFC 06/65] target/riscv: rvv-0.9: add vcsr register

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> +[CSR_VCSR] ={ vs,   read_vcsr,write_vcsr
> },

As long as you have the vext_spec argument, you need a separate vs_0_9
predicate function, so that this csr is not available to VEXT_VERSION_0_07_1.


r~



Re: [PULL 00/10] Modules 20200707 patches

2020-07-10 Thread Philippe Mathieu-Daudé
On 7/7/20 3:42 PM, Gerd Hoffmann wrote:
> The following changes since commit 7623b5ba017f61de5d7c2bba12c6feb3d55091b1:
> 
>   Merge remote-tracking branch 
> 'remotes/vivier2/tags/linux-user-for-5.1-pull-request' into staging 
> (2020-07-06 11:40:10 +0100)
> 
> are available in the Git repository at:
> 
>   git://git.kraxel.org/qemu tags/modules-20200707-pull-request
> 
> for you to fetch changes up to ef138c77249771081d8c2d09b8e729f7e92cdf28:
> 
>   chardev: enable modules, use for braille (2020-07-07 15:33:59 +0200)
> 
> 
> qom: add support for qom objects in modules.
> build some devices (qxl, virtio-gpu, ccid, usb-redir) as modules.
> build braille chardev as module.
> 
> v2: more verbose comment for "build: fix device module builds" patch.
> 
> note: qemu doesn't rebuild objects on cflags changes (specifically
>   -fPIC being added when code is switched from builtin to module).
>   Workaround for resulting build errors: "make clean", rebuild.
> 
> 
> 
> Gerd Hoffmann (10):
>   module: qom module support
>   object: qom module support
>   qdev: device module support
>   build: fix device module builds
>   ccid: build smartcard as module
>   usb: build usb-redir as module
>   vga: build qxl as module
>   vga: build virtio-gpu only once
>   vga: build virtio-gpu as module
>   chardev: enable modules, use for braille
> 
>  Makefile.objs|  2 ++
>  Makefile.target  | 14 +
>  include/qemu/module.h|  2 ++
>  include/qom/object.h | 12 +++
>  chardev/char.c   |  2 +-
>  hw/core/qdev.c   |  6 ++--
>  qdev-monitor.c   |  5 +--
>  qom/object.c | 14 +
>  qom/qom-qmp-cmds.c   |  3 +-
>  softmmu/vl.c |  4 +--
>  util/module.c| 67 
>  chardev/Makefile.objs|  5 ++-
>  hw/Makefile.objs |  2 ++
>  hw/display/Makefile.objs | 28 ++---
>  hw/usb/Makefile.objs | 13 +---
>  15 files changed, 155 insertions(+), 24 deletions(-)
> 

This is making Travis-CI fail:

https://travis-ci.org/github/philmd/qemu/jobs/706941688

MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=s390x-softmmu/qemu-system-s390x
QTEST_QEMU_IMG=qemu-img tests/qtest/device-introspect-test -m=quick -k
--tap < /dev/null | ./scripts/tap-driver.pl
--test-name="device-introspect-test"
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-qxl.so:
undefined symbol: vga_ioport_read
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-virtio-gpu.so:
undefined symbol: pci_std_vga_mmio_region_init
PASS 1 device-introspect-test /s390x/device/introspect/list
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-qxl.so:
undefined symbol: vga_ioport_read
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-virtio-gpu.so:
undefined symbol: pci_std_vga_mmio_region_init
PASS 2 device-introspect-test /s390x/device/introspect/list-fields
PASS 3 device-introspect-test /s390x/device/introspect/none
PASS 4 device-introspect-test /s390x/device/introspect/abstract
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-qxl.so:
undefined symbol: vga_ioport_read
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-virtio-gpu.so:
undefined symbol: pci_std_vga_mmio_region_init
PASS 5 device-introspect-test /s390x/device/introspect/abstract-interfaces
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-qxl.so:
undefined symbol: vga_ioport_read
Failed to open module:
/home/travis/build/philmd/qemu/build/s390x-softmmu/../hw-display-virtio-gpu.so:
undefined symbol: pci_std_vga_mmio_region_init
missing object type 'virtio-gpu-device'
Broken pipe
/home/travis/build/philmd/qemu/tests/qtest/libqtest.c:175: kill_qemu()
detected QEMU death from signal 6 (Aborted) (core dumped)
Aborted (core dumped)
ERROR - too few tests run (expected 6, got 5)
/home/travis/build/philmd/qemu/tests/Makefile.include:648: recipe for
target 'check-qtest-s390x' failed
make: *** [check-qtest-s390x] Error 1




Re: [PATCH v4 36/40] gitlab: split build-disabled into two phases

2020-07-10 Thread Philippe Mathieu-Daudé
On 7/10/20 6:26 PM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé  writes:
> 
>> On 7/10/20 4:58 PM, Alex Bennée wrote:
>>>
>>> Thomas Huth  writes:
>>>
 On 01/07/2020 15.56, Alex Bennée wrote:
> As we run check-qtest in "SLOW" mode this can timeout so split into
> two jobs.
>
> Signed-off-by: Alex Bennée 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Thomas Huth 

  Hi Alex,

 I think you can drop this patch and use "[PATCH v2] tests: improve
 performance of device-introspect-test" instead.
>>>
>>> As I'm re-rolling the PR sure...
>>
>> Also maybe:
>>
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721458.html
> 
> I don't think it's directly related

I thought it was.

> - can we just avoid pilling a bunch
> of stuff in on a re-roll please.

Understood, sorry.



Re: [PATCH 2/2] x86/cpu: Handle GUEST_MAXPHYADDR < HOST_MAXPHYADDR for hosts that don't support it

2020-07-10 Thread Paolo Bonzini
On 10/07/20 18:02, Eduardo Habkost wrote:
> On Fri, Jul 10, 2020 at 09:22:42AM +0200, Paolo Bonzini wrote:
>> On 09/07/20 21:13, Eduardo Habkost wrote:
 Doesn't this require intercepting MOV-to-CR3 when the guest is in PAE
 mode, so that the hypervisor can validate the high bits in the PDPTEs?
>>> If the fix has additional overhead, is the additional overhead
>>> bad enough to warrant making it optional?  Most existing
>>> GUEST_MAXPHYADDR < HOST_MAXPHYADDR guests already work today
>>> without the fix.
>>
>> The problematic case is when host maxphyaddr is 52.  That case wouldn't
>> work at all without the fix.
> 
> What can QEMU do to do differentiate "can't work at all without
> the fix" from "not the best idea, but will probably work"?

Blocking guest_maxphyaddr < host_maxphyaddr if maxphyaddr==52 would be a
good start.  However it would block the default configuration on IceLake
processors (which is why Mohammed looked at this thing in the first place).

Paolo




Re: [PATCH v5 1/4] target/i386: add missing vmx features for several CPU models

2020-07-10 Thread Eduardo Habkost
On Fri, Jul 10, 2020 at 09:45:49AM +0800, Chenyi Qiang wrote:
> 
> 
> On 7/10/2020 6:12 AM, Eduardo Habkost wrote:
> > 
> > I'm very sorry for taking so long to review this.  Question
> > below:
> > 
> > On Fri, Jun 19, 2020 at 03:31:11PM +0800, Chenyi Qiang wrote:
> > > Add some missing VMX features in Skylake-Server, Cascadelake-Server and
> > > Icelake-Server CPU models based on the output of Paolo's script.
> > > 
> > > Signed-off-by: Chenyi Qiang 
> > 
> > Why are you changing the v1 definition instead adding those new
> > features in a new version of the CPU model, just like you did in
> > patch 3/4?
> > 
> 
> I suppose these missing vmx features are not quite necessary for customers.
> Just post it here to see if they are worth being added.
> Adding a new version is reasonable. Is it appropriate to put all the missing
> features in patch 1/4, 3/4, 4/4 in a same version?

Yes, it would be OK to add only one new version with all the new
features.

-- 
Eduardo




Re: [PATCH v11 00/34] Add subcluster allocation to qcow2

2020-07-10 Thread no-reply
Patchew URL: https://patchew.org/QEMU/cover.1594396418.git.be...@igalia.com/



Hi,

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

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

  CC  block/snapshot.o
  CC  block/qapi.o
/tmp/qemu-test/src/block/qcow2-cluster.c: In function 'qcow2_get_host_offset':
/tmp/qemu-test/src/block/qcow2-cluster.c:473:19: error: 'expected_type' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
 } else if (type != expected_type) {
   ^
/tmp/qemu-test/src/block/qcow2-cluster.c:449:25: note: 'expected_type' was 
declared here
 QCow2SubclusterType expected_type, type;
 ^
cc1: all warnings being treated as errors
make: *** [block/qcow2-cluster.o] Error 1
make: *** Waiting for unfinished jobs
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 669, in 
---
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', 
'--label', 'com.qemu.instance.uuid=10623143eef0416b9c3b987efa6621a1', '-u', 
'1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', 
'-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 
'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', 
'/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', 
'/var/tmp/patchew-tester-tmp-zd6p1e8g/src/docker-src.2020-07-10-12.40.45.27056:/var/tmp/qemu:z,ro',
 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit 
status 2.
filter=--filter=label=com.qemu.instance.uuid=10623143eef0416b9c3b987efa6621a1
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-zd6p1e8g/src'
make: *** [docker-run-test-quick@centos7] Error 2

real3m13.395s
user0m8.778s


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

Re: [PATCH 2/2] iotests: Set LC_ALL=C for sort

2020-07-10 Thread Eric Blake

On 7/10/20 11:32 AM, Max Reitz wrote:

Otherwise the result is basically unpredictable.

(Note that the precise environment variable to control sorting order is
LC_COLLATE, but LC_ALL overrides LC_COLLATE, and we do not want the
sorting order to be messed up if LC_ALL is set in the environment.)


Yep, that logic is correct.



Reported-by: John Snow 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/common.filter | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Eric Blake 



diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 345c3ca03e..4fd5c29b2a 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -177,7 +177,7 @@ _do_filter_img_create()
  -e 's/^\(data_file\)/3-\1/' \
  -e 's/^\(encryption\)/4-\1/' \
  -e 's/^\(preallocation\)/8-\1/' \
-| sort \
+| LC_ALL=C sort \
  | $SED -e 's/^[0-9]-//' \
  | tr '\n\0' ' \n' \
  | $SED -e 's/^ *$//' -e 's/ *$//'



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




[PULL 1/1] 9p: null terminate fs driver options list

2020-07-10 Thread Greg Kurz
From: Prasad J Pandit 

NULL terminate fs driver options' list, validate_opt() looks for
a null entry to terminate the loop.

Fixes: aee7f3ecd8b7 ("fsdev: Error out when unsupported option is passed")
Signed-off-by: Prasad J Pandit 
Reviewed-by: Li Qiang 
Message-Id: <20200709175848.650400-1-ppan...@redhat.com>
Signed-off-by: Greg Kurz 
---
 fsdev/qemu-fsdev.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index a9e069c0c78d..3da64e9f72b4 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -78,6 +78,7 @@ static FsDriverTable FsDrivers[] = {
 "throttling.iops-read-max-length",
 "throttling.iops-write-max-length",
 "throttling.iops-size",
+NULL
 },
 },
 {
@@ -85,6 +86,7 @@ static FsDriverTable FsDrivers[] = {
 .ops = _ops,
 .opts = (const char * []) {
 COMMON_FS_DRIVER_OPTIONS,
+NULL
 },
 },
 {
@@ -95,6 +97,7 @@ static FsDriverTable FsDrivers[] = {
 "socket",
 "sock_fd",
 "writeout",
+NULL
 },
 },
 };
-- 
2.26.2




[PATCH] fixup! qemu-img: Deprecate use of -b without -F

2020-07-10 Thread Eric Blake
Port to BSD truncate.

Signed-off-by: Eric Blake 
---
 tests/qemu-iotests/114 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
index 143683381334..d0609c499388 100755
--- a/tests/qemu-iotests/114
+++ b/tests/qemu-iotests/114
@@ -45,7 +45,7 @@ _unsupported_imgopts data_file
 # Intentionally specify backing file without backing format; demonstrate
 # the difference in warning messages when backing file could be probed.
 # Note that only a non-raw probe result will affect the resulting image.
-truncate --size=64M "$TEST_IMG.orig"
+truncate -s $((64 * 1024 * 1024)) "$TEST_IMG.orig"
 _make_test_img -b "$TEST_IMG.orig" 64M

 TEST_IMG="$TEST_IMG.base" _make_test_img 64M
-- 
2.27.0




Re: [RFC 05/65] target/riscv: remove vsll.vi, vsrl.vi, vsra.vi insns from using gvec

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> vsll.vi, vsrl.vi, vsra.vi cannot use shli gvec as it requires the
> shift immediate value to be within the range: [0.. SEW bits].
> Otherwise, it will hit the assertion:
> tcg_debug_assert(shift >= 0 && shift < (8 << vece));
> 
> However, RVV spec does not have such constraint, therefore we have to
> use helper functions instead.

Why do you say that?  It does have such a constraint:

# Only the low lg2(SEW) bits are read to obtain the shift amount from a
register value.

While that only talks about the register value, I sincerely doubt that the same
truncation does not actually apply to immediates.

And if the entire immediate value does apply, the manual should certainly
specify what should happen in that case.  And at present it doesn't.

It seems to me the bug is the bare use of GEN_OPIVI_GVEC_TRANS and thence
do_opivi_gvec.  The ZX parameter should be extended to more than just "zero vs
sign-extend", it should have an option for truncating the immediate to s->sew.


r~



Re: [PATCH 1/2] iotests: Drop readarray from _do_filter_img_create

2020-07-10 Thread Eric Blake

On 7/10/20 11:32 AM, Max Reitz wrote:

Some systems where we run tests on do not have a 4.x bash, so they do
not have readarray.  While it looked a bit nicer than messing with
`head` and `tail`, we do not really need it, so we might as well not use
it.

Reported-by: Claudio Fontana 
Signed-off-by: Max Reitz 
---
  tests/qemu-iotests/common.filter | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)



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




[PULL 0/1] 9p fixes for 5.1 2020-07-10

2020-07-10 Thread Greg Kurz
The following changes since commit b6d7e9b66f59ca6ebc6e9b830cd5e7bf849d31cf:

  Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' 
into staging (2020-07-10 09:01:28 +0100)

are available in the Git repository at:

  https://github.com/gkurz/qemu.git tags/9p-fix-2020-07-10

for you to fetch changes up to 353b5a91ccf2789b85967d19a8795816b8865562:

  9p: null terminate fs driver options list (2020-07-10 12:48:06 +0200)


Add missing NULL terminating element in fsdev option lists. Never
crashed QEMU by pure luck.


Prasad J Pandit (1):
  9p: null terminate fs driver options list

 fsdev/qemu-fsdev.c | 3 +++
 1 file changed, 3 insertions(+)
-- 
2.26.2




[PATCH 0/2] iotests: More _filter_img_create fixes

2020-07-10 Thread Max Reitz
Hi,

I’m sorry.

John, could I ask you to test whether this series fixes the problems
you’re seeing?


Max Reitz (2):
  iotests: Drop readarray from _do_filter_img_create
  iotests: Set LC_ALL=C for sort

 tests/qemu-iotests/common.filter | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.26.2




[RFC 3/3] x68: acpi: trigger SMI before scanning for hotplugged CPUs

2020-07-10 Thread Igor Mammedov
In case firmware has negotiated CPU hotplug SMI feature, generate
AML to describe SMI IO port region and send SMI to firmware
on each CPU hotplug SCI.

It might be not really usable, but should serve as a starting point to
discuss how better to deal with split hotplug sequence during hot-add
(
ex scenario where it will break is:
   hot-add
  -> (QEMU) add CPU in hotplug regs
  -> (QEMU) SCI
   -1-> (OS) scan
   -1-> (OS) SMI
   -1-> (FW) pull in CPU1 ***
   -1-> (OS) start iterating hotplug regs
   hot-add
  -> (QEMU) add CPU in hotplug regs
  -> (QEMU) SCI
-2-> (OS) scan (blocked on mutex till previous scan is finished)
   -1-> (OS) 1st added CPU1 send device check event -> INIT/SIPI
   -1-> (OS) 1st added CPU2 send device check event -> INIT/SIPI
   that's where it explodes, since FW didn't see CPU2
   when SMI was called
)

hot remove will throw in yet another set of problems, so lets discuss
both here and see if we can  really share hotplug registers block between
FW and AML or we should do something else with it.

Signed-off-by: Igor Mammedov 
---
v0:
 - s/aml_string/aml_eisaid/
   /fixes Windows BSOD, on nonsense value/ (Laszlo Ersek )
 - put SMI device under PCI0 like the rest of hotplug IO port
 - do not generate SMI AML if CPU hotplug SMI feature hasn't been negotiated
---
 include/hw/acpi/cpu.h |  1 +
 hw/acpi/cpu.c |  6 ++
 hw/i386/acpi-build.c  | 33 -
 hw/isa/lpc_ich9.c |  3 +++
 4 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index 62f0278ba2..0eeedaa491 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -50,6 +50,7 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
 typedef struct CPUHotplugFeatures {
 bool acpi_1_compatible;
 bool has_legacy_cphp;
+const char *smi_path;
 } CPUHotplugFeatures;
 
 void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 3d6a500fb7..6539bc23f6 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -14,6 +14,8 @@
 #define ACPI_CPU_CMD_DATA_OFFSET_RW 8
 #define ACPI_CPU_CMD_DATA2_OFFSET_R 0
 
+#define OVMF_CPUHP_SMI_CMD 4
+
 enum {
 CPHP_GET_NEXT_CPU_WITH_EVENT_CMD = 0,
 CPHP_OST_EVENT_CMD = 1,
@@ -473,6 +475,10 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
CPUHotplugFeatures opts,
 Aml *next_cpu_cmd = aml_int(CPHP_GET_NEXT_CPU_WITH_EVENT_CMD);
 
 aml_append(method, aml_acquire(ctrl_lock, 0x));
+if (opts.smi_path) {
+aml_append(method, aml_store(aml_int(OVMF_CPUHP_SMI_CMD),
+aml_name("%s", opts.smi_path)));
+}
 aml_append(method, aml_store(one, has_event));
 while_ctx = aml_while(aml_equal(has_event, one));
 {
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a..1e21386f0c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -95,6 +95,7 @@ typedef struct AcpiPmInfo {
 bool s3_disabled;
 bool s4_disabled;
 bool pcihp_bridge_en;
+bool smi_on_cpuhp;
 uint8_t s4_val;
 AcpiFadtData fadt;
 uint16_t cpu_hp_io_base;
@@ -194,6 +195,7 @@ static void acpi_get_pm_info(MachineState *machine, 
AcpiPmInfo *pm)
 pm->cpu_hp_io_base = 0;
 pm->pcihp_io_base = 0;
 pm->pcihp_io_len = 0;
+pm->smi_on_cpuhp = false;
 
 assert(obj);
 init_common_fadt_data(machine, obj, >fadt);
@@ -213,6 +215,9 @@ static void acpi_get_pm_info(MachineState *machine, 
AcpiPmInfo *pm)
 pm->fadt.reset_val = 0xf;
 pm->fadt.flags |= 1 << ACPI_FADT_F_RESET_REG_SUP;
 pm->cpu_hp_io_base = ICH9_CPU_HOTPLUG_IO_BASE;
+pm->smi_on_cpuhp =
+!!(object_property_get_uint(lpc, "x-smi-negotiated-features", NULL)
+   & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT));
 }
 
 /* The above need not be conditional on machine type because the reset port
@@ -1515,6 +1520,31 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 aml_append(dev, aml_name_decl("_UID", aml_int(1)));
 aml_append(dev, build_q35_osc_method());
 aml_append(sb_scope, dev);
+
+if (pm->smi_on_cpuhp) {
+/* reserve SMI block resources */
+dev = aml_device("PCI0.SMI0");
+aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A06")));
+aml_append(dev, aml_name_decl("_UID", aml_string("SMI 
resources")));
+crs = aml_resource_template();
+aml_append(crs,
+aml_io(
+   AML_DECODE16,
+   ACPI_PORT_SMI_CMD,
+   ACPI_PORT_SMI_CMD,
+   1,
+   1)
+);
+aml_append(dev, aml_name_decl("_CRS", crs));
+

Re: [PATCH v4 36/40] gitlab: split build-disabled into two phases

2020-07-10 Thread Alex Bennée


Philippe Mathieu-Daudé  writes:

> On 7/10/20 4:58 PM, Alex Bennée wrote:
>> 
>> Thomas Huth  writes:
>> 
>>> On 01/07/2020 15.56, Alex Bennée wrote:
 As we run check-qtest in "SLOW" mode this can timeout so split into
 two jobs.

 Signed-off-by: Alex Bennée 
 Reviewed-by: Philippe Mathieu-Daudé 
 Reviewed-by: Thomas Huth 
>>>
>>>  Hi Alex,
>>>
>>> I think you can drop this patch and use "[PATCH v2] tests: improve
>>> performance of device-introspect-test" instead.
>> 
>> As I'm re-rolling the PR sure...
>
> Also maybe:
>
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg721458.html

I don't think it's directly related - can we just avoid pilling a bunch
of stuff in on a re-roll please.

-- 
Alex Bennée



Re: [RFC 04/65] target/riscv: fix vill bit index in vtype register

2020-07-10 Thread Richard Henderson
On 7/10/20 3:48 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> vill bit is at vtype[XLEN-1].
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 

Alistair, this one should be queued for 5.1 as a bug fix.


r~




[RFC 1/3] x86: lpc9: let firmware negotiate CPU hotplug SMI feature

2020-07-10 Thread Igor Mammedov
It will allow firmware to notify QEMU that firmware requires SMI
being triggered on CPU hotplug, so that it would be able to account
for hotplugged CPU and relocate it to new SMM base.

Using the negotiated feature, follow up patches will insert SMI upcall
into AML code, to make sure that firmware processes hotplug before
guest OS would attempt to use new CPU.

Signed-off-by: Igor Mammedov 
---
 include/hw/i386/ich9.h | 1 +
 hw/i386/pc.c   | 4 +++-
 hw/isa/lpc_ich9.c  | 7 +++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/hw/i386/ich9.h b/include/hw/i386/ich9.h
index a98d10b252..422891a152 100644
--- a/include/hw/i386/ich9.h
+++ b/include/hw/i386/ich9.h
@@ -247,5 +247,6 @@ typedef struct ICH9LPCState {
 
 /* bit positions used in fw_cfg SMI feature negotiation */
 #define ICH9_LPC_SMI_F_BROADCAST_BIT0
+#define ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT  1
 
 #endif /* HW_ICH9_H */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d7f27bc16b..6fe80c84d7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -97,7 +97,9 @@
 #include "fw_cfg.h"
 #include "trace.h"
 
-GlobalProperty pc_compat_5_0[] = {};
+GlobalProperty pc_compat_5_0[] = {
+{ "ICH9-LPC", "x-smi-cpu-hotplug", "off" },
+};
 const size_t pc_compat_5_0_len = G_N_ELEMENTS(pc_compat_5_0);
 
 GlobalProperty pc_compat_4_2[] = {
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index cd6e169d47..83da7346ab 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -373,6 +373,11 @@ static void smi_features_ok_callback(void *opaque)
 /* guest requests invalid features, leave @features_ok at zero */
 return;
 }
+if (!(guest_features & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT)) &&
+guest_features & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT)) {
+/* cpu hotplug SMI requires SMI broadcast, leave @features_ok at zero 
*/
+return;
+}
 
 /* valid feature subset requested, lock it down, report success */
 lpc->smi_negotiated_features = guest_features;
@@ -747,6 +752,8 @@ static Property ich9_lpc_properties[] = {
 DEFINE_PROP_BOOL("noreboot", ICH9LPCState, pin_strap.spkr_hi, true),
 DEFINE_PROP_BIT64("x-smi-broadcast", ICH9LPCState, smi_host_features,
   ICH9_LPC_SMI_F_BROADCAST_BIT, true),
+DEFINE_PROP_BIT64("x-smi-cpu-hotplug", ICH9LPCState, smi_host_features,
+  ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.26.2




[PATCH v11 31/34] qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit

2020-07-10 Thread Alberto Garcia
Now that the implementation of subclusters is complete we can finally
add the necessary options to create and read images with this feature,
which we call "extended L2 entries".

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
---
 qapi/block-core.json |   7 +++
 block/qcow2.h|   8 ++-
 include/block/block_int.h|   1 +
 block/qcow2.c|  66 ++--
 tests/qemu-iotests/031.out   |   8 +--
 tests/qemu-iotests/036.out   |   4 +-
 tests/qemu-iotests/049.out   | 102 +++
 tests/qemu-iotests/060.out   |   1 +
 tests/qemu-iotests/061.out   |  20 +++---
 tests/qemu-iotests/065   |  12 ++--
 tests/qemu-iotests/082.out   |  39 +---
 tests/qemu-iotests/085.out   |  38 ++--
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +--
 tests/qemu-iotests/198   |   2 +
 tests/qemu-iotests/206.out   |   4 ++
 tests/qemu-iotests/242.out   |   5 ++
 tests/qemu-iotests/255.out   |   8 +--
 tests/qemu-iotests/274.out   |  49 ---
 tests/qemu-iotests/280.out   |   2 +-
 tests/qemu-iotests/291.out   |   2 +
 tests/qemu-iotests/common.filter |   1 +
 23 files changed, 253 insertions(+), 140 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index b20332e592..0d4231ee98 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -66,6 +66,9 @@
 # standalone (read-only) raw image without looking at qcow2
 # metadata (since: 4.0)
 #
+# @extended-l2: true if the image has extended L2 entries; only valid for
+#   compat >= 1.1 (since 5.1)
+#
 # @lazy-refcounts: on or off; only valid for compat >= 1.1
 #
 # @corrupt: true if the image has been marked corrupt; only valid for
@@ -87,6 +90,7 @@
   'compat': 'str',
   '*data-file': 'str',
   '*data-file-raw': 'bool',
+  '*extended-l2': 'bool',
   '*lazy-refcounts': 'bool',
   '*corrupt': 'bool',
   'refcount-bits': 'int',
@@ -4318,6 +4322,8 @@
 # @data-file-raw: True if the external data file must stay valid as a
 # standalone (read-only) raw image without looking at qcow2
 # metadata (default: false; since: 4.0)
+# @extended-l2  True to make the image have extended L2 entries
+#   (default: false; since 5.1)
 # @size: Size of the virtual disk in bytes
 # @version: Compatibility level (default: v3)
 # @backing-file: File name of the backing file if a backing file
@@ -4338,6 +4344,7 @@
   'data': { 'file': 'BlockdevRef',
 '*data-file':   'BlockdevRef',
 '*data-file-raw':   'bool',
+'*extended-l2': 'bool',
 'size': 'size',
 '*version': 'BlockdevQcow2Version',
 '*backing-file':'str',
diff --git a/block/qcow2.h b/block/qcow2.h
index f3499e53bf..065ec3df0b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -246,15 +246,18 @@ enum {
 QCOW2_INCOMPAT_CORRUPT_BITNR= 1,
 QCOW2_INCOMPAT_DATA_FILE_BITNR  = 2,
 QCOW2_INCOMPAT_COMPRESSION_BITNR = 3,
+QCOW2_INCOMPAT_EXTL2_BITNR  = 4,
 QCOW2_INCOMPAT_DIRTY= 1 << QCOW2_INCOMPAT_DIRTY_BITNR,
 QCOW2_INCOMPAT_CORRUPT  = 1 << QCOW2_INCOMPAT_CORRUPT_BITNR,
 QCOW2_INCOMPAT_DATA_FILE= 1 << QCOW2_INCOMPAT_DATA_FILE_BITNR,
 QCOW2_INCOMPAT_COMPRESSION  = 1 << QCOW2_INCOMPAT_COMPRESSION_BITNR,
+QCOW2_INCOMPAT_EXTL2= 1 << QCOW2_INCOMPAT_EXTL2_BITNR,
 
 QCOW2_INCOMPAT_MASK = QCOW2_INCOMPAT_DIRTY
 | QCOW2_INCOMPAT_CORRUPT
 | QCOW2_INCOMPAT_DATA_FILE
-| QCOW2_INCOMPAT_COMPRESSION,
+| QCOW2_INCOMPAT_COMPRESSION
+| QCOW2_INCOMPAT_EXTL2,
 };
 
 /* Compatible feature bits */
@@ -581,8 +584,7 @@ typedef enum QCow2MetadataOverlap {
 
 static inline bool has_subclusters(BDRVQcow2State *s)
 {
-/* FIXME: Return false until this feature is complete */
-return false;
+return s->incompatible_features & QCOW2_INCOMPAT_EXTL2;
 }
 
 static inline size_t l2_entry_size(BDRVQcow2State *s)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3d6cf88592..4c6545b950 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -58,6 +58,7 @@
 #define BLOCK_OPT_DATA_FILE "data_file"
 #define BLOCK_OPT_DATA_FILE_RAW "data_file_raw"
 #define BLOCK_OPT_COMPRESSION_TYPE  "compression_type"
+#define BLOCK_OPT_EXTL2 "extended_l2"
 
 #define BLOCK_PROBE_BUF_SIZE512
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 8ca0f22069..b7a1cae39c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1449,6 

[PATCH 2/2] iotests: Set LC_ALL=C for sort

2020-07-10 Thread Max Reitz
Otherwise the result is basically unpredictable.

(Note that the precise environment variable to control sorting order is
LC_COLLATE, but LC_ALL overrides LC_COLLATE, and we do not want the
sorting order to be messed up if LC_ALL is set in the environment.)

Reported-by: John Snow 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.filter | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 345c3ca03e..4fd5c29b2a 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -177,7 +177,7 @@ _do_filter_img_create()
 -e 's/^\(data_file\)/3-\1/' \
 -e 's/^\(encryption\)/4-\1/' \
 -e 's/^\(preallocation\)/8-\1/' \
-| sort \
+| LC_ALL=C sort \
 | $SED -e 's/^[0-9]-//' \
 | tr '\n\0' ' \n' \
 | $SED -e 's/^ *$//' -e 's/ *$//'
-- 
2.26.2




Re: [PULL v2] Block layer patches

2020-07-10 Thread Eric Blake

On 7/9/20 7:17 AM, Kevin Wolf wrote:

The following changes since commit 8796c64ecdfd34be394ea277a53df0c76996:

   Merge remote-tracking branch 
'remotes/kraxel/tags/audio-20200706-pull-request' into staging (2020-07-08 
16:33:59 +0100)

are available in the Git repository at:

   git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to d1c824e681e423a6224c6831a493e9175fa02dc1:

   qemu-img: Deprecate use of -b without -F (2020-07-09 14:14:55 +0200)


Block layer patches:

- file-posix: Mitigate file fragmentation with extent size hints
- Tighten qemu-img rules on missing backing format
- qemu-img map: Don't limit block status request size


Eric Blake (10):
   qemu-img: Flush stdout before before potential stderr messages
   block: Finish deprecation of 'qemu-img convert -n -o'
   sheepdog: Add trivial backing_fmt support
   vmdk: Add trivial backing_fmt support
   qcow: Tolerate backing_fmt=


I see you renumbered my test 293 to 301 for conflict resolution, but did 
not update the commit message to call out the updated number ;)


Since we may have to do a v3 because of the 'truncate -s' issue in 114, 
you could touch that up too.




  tests/qemu-iotests/301   | 88 
  tests/qemu-iotests/301.out   | 59 +++
  tests/qemu-iotests/common.filter | 62 
  tests/qemu-iotests/group |  1 +
  146 files changed, 906 insertions(+), 472 deletions(-)
  create mode 100755 tests/qemu-iotests/301
  create mode 100644 tests/qemu-iotests/301.out




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




[PATCH 1/2] iotests: Drop readarray from _do_filter_img_create

2020-07-10 Thread Max Reitz
Some systems where we run tests on do not have a 4.x bash, so they do
not have readarray.  While it looked a bit nicer than messing with
`head` and `tail`, we do not really need it, so we might as well not use
it.

Reported-by: Claudio Fontana 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.filter | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 3833206327..345c3ca03e 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -138,13 +138,13 @@ _do_filter_img_create()
 # Split the line into the pre-options part ($filename_part, which
 # precedes ", fmt=") and the options part ($options, which starts
 # with "fmt=")
-# (And just echo everything before the first "^Formatting")
-readarray formatting_line < <($SED -e 's/, fmt=/\n/')
+read formatting_line
 
-filename_part=${formatting_line[0]}
-unset formatting_line[0]
+# Split line at the first ", fmt="
+formatting_line=$(echo "$formatting_line" | $SED -e 's/, fmt=/\nfmt=/')
 
-options="fmt=${formatting_line[@]}"
+filename_part=$(echo "$formatting_line" | head -n 1)
+options=$(echo "$formatting_line" | tail -n +2)
 
 # Set grep_data_file to '\|data_file' to keep it; make it empty
 # to drop it.
-- 
2.26.2




[RFC 2/3] x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in use

2020-07-10 Thread Igor Mammedov
There were reports of guest crash on CPU hotplug, when using q35 machine
type and QVMF with Secure Boot, due to hotplugged CPU trying to process SMI
at default SMI handler location without it being relocated by firmware first.

Fix it by refusing hotplug if firmware hasn't negotiatiad CPU hotplug SMI
support while SMI broadcast is in use.

Signed-off-by: Igor Mammedov 
---
 hw/acpi/ich9.c | 12 +++-
 hw/i386/pc.c   | 11 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 2d204babc6..a22b434e0b 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -408,10 +408,20 @@ void ich9_pm_device_pre_plug_cb(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 ICH9LPCState *lpc = ICH9_LPC_DEVICE(hotplug_dev);
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
-!lpc->pm.acpi_memory_hotplug.is_enabled)
+!lpc->pm.acpi_memory_hotplug.is_enabled) {
 error_setg(errp,
"memory hotplug is not enabled: %s.memory-hotplug-support "
"is not set", object_get_typename(OBJECT(lpc)));
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+uint64_t negotiated = lpc->smi_negotiated_features;
+
+if (negotiated & BIT_ULL(ICH9_LPC_SMI_F_BROADCAST_BIT) &&
+!(negotiated & BIT_ULL(ICH9_LPC_SMI_F_CPU_HOTPLUG_BIT))) {
+error_setg(errp, "cpu hotplug SMI was not enabled by firmware");
+error_append_hint(errp, "update machine type to newer than 5.0 "
+"and firmware that suppors CPU hotplug in Secure Boot mode");
+}
+}
 }
 
 void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6fe80c84d7..dc1e9157d7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1508,6 +1508,17 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
 return;
 }
 
+if (pcms->acpi_dev) {
+Error *local_err = NULL;
+
+hotplug_handler_pre_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
+ _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+}
+
 init_topo_info(_info, x86ms);
 
 env->nr_dies = x86ms->smp_dies;
-- 
2.26.2




[RFC 0/3] x86: fix cpu hotplug with secure boot

2020-07-10 Thread Igor Mammedov
CPU hotplug with Secure Boot was not really supported and firmware wasn't aware
of hotplugged CPUs (which might lead to guest crashes). During 4.2 we introduced
locked SMI handler RAM arrea to make sure that guest OS wasn't able to inject
its own SMI handler and OVMF added initial CPU hotplug support.

This series is QEMU part of that support [1] which lets QMVF tell QEMU that
CPU hotplug with SMI broadcast enabled is supported so that QEMU would be able
to prevent hotplug in case it's not supported and trigger SMI on hotplug when
it's necessary. 

1) CPU hotplug negotiation part was introduced later so it might not be
in upstream OVMF yet or I might have missed the patch on edk2-devel
(Laszlo will point out to it/post formal patch)

Igor Mammedov (3):
  x86: lpc9: let firmware negotiate CPU hotplug SMI feature
  x86: cphp: prevent guest crash on CPU hotplug when broadcast SMI is in
use
  x68: acpi: trigger SMI before scanning for hotplugged CPUs

 include/hw/acpi/cpu.h  |  1 +
 include/hw/i386/ich9.h |  1 +
 hw/acpi/cpu.c  |  6 ++
 hw/acpi/ich9.c | 12 +++-
 hw/i386/acpi-build.c   | 33 -
 hw/i386/pc.c   | 15 ++-
 hw/isa/lpc_ich9.c  | 10 ++
 7 files changed, 75 insertions(+), 3 deletions(-)

-- 
2.26.2




  1   2   3   4   >