[PATCH] plugins/core: add missing break in cb_to_tcg_flags
Reported-by: Robert Henry Signed-off-by: Emilio G. Cota --- plugins/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/core.c b/plugins/core.c index 9e1b9e7a91..ed863011ba 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -286,6 +286,7 @@ static inline uint32_t cb_to_tcg_flags(enum qemu_plugin_cb_flags flags) switch (flags) { case QEMU_PLUGIN_CB_RW_REGS: ret = 0; +break; case QEMU_PLUGIN_CB_R_REGS: ret = TCG_CALL_NO_WG; break; -- 2.20.1
Re: [PATCH v2] Implement the Screamer sound chip for the mac99 machine type
I found the patch that breaks Screamer sound support for qemu-system-ppc. It is this: commit 2ceb8240fa4e4e30fb853565eb2bed3032d74f62 Author: Kővágó, Zoltán Date: Thu Sep 19 23:24:11 2019 +0200 coreaudio: port to the new audio backend api Signed-off-by: Kővágó, Zoltán Message-id: 586a1e66de5cbc6c5234f9ae556d24befb6afada.1568927990.git.dirty.ice...@gmail.com Signed-off-by: Gerd Hoffmann Reversing this patch should make the Screamer patch work with the current git version of QEMU.
Re: [Qemu-devel] What should a virtual board emulate?
Hi Paolo, On 3/20/19 11:35 AM, Paolo Bonzini wrote: On 20/03/19 11:03, Philippe Mathieu-Daudé wrote: -display/-vga options suffers same clarity problems than -net. Is it a card device or a cable linking to a network? Here is it a card device or a cable connecting a monitor display? -display is a cable, -vga is a card ("-nic none" is a card, "-nic anythingelse" is a card+cable; "-net nic" is a card, "-net anythingelse" is a cable). Mind, I'm not demanding mips-fulong2e should continue to ignore -vga; that's for its maintainer to decide. I don't demand, I ask: what should a virtual board emulate? What should -nodefaults do? IMHO -nodefaults contains soldered/mmio chipsets. Whether you plug a display or not is a different story. In principle you could also cut the copper tracks that connect the card to the PCI bus... But then you have a crippled machine... We are not trying to model that. I went back to continue a Fuloong Avocado test I started a year ago, and it was failing. I remember I had something working, so I bisected and reached this commit... 78c37d88f1b8b0b3ebcc632c458f0c3779fe2951 is the first bad commit commit 78c37d88f1b8b0b3ebcc632c458f0c3779fe2951 Author: Paolo Bonzini Date: Tue Mar 19 15:37:19 2019 +0100 mips-fulong2e: obey -vga none Do not create an ATI VGA if "-vga none" was passed on the command line. Booting PMON 1.1.2: console: PMON2000 MIPS Initializing. Standby... console: ERRORPC= CONFIG=00030932 console: PRID=6302 console: DIMM read console: 0080 console: read memory type console: read number of rows console: read memory size per side console: read blocks per ddrram console: read number of sides console: read width console: DIMM SIZE=1000 console: sdcfg=3d5043df console: msize=1000 console: Init SDRAM Done! console: Sizing caches... console: Init caches... console: godson2 caches found console: Init caches done, cfg = 00030932 console: Copy PMON to execute location... console: start = 0x8500 console: s0 = 0x3ac0 console: a500 console: a501 console: a502 console: a503 console: a504 console: copy text section done. console: Copy PMON to execute location done. console: sp=84ffc000Uncompressing BiosOK,Booting Bios console: FREQ console: FREI console: DONE console: TTYI console: TTYD console: ENVI console: MAPV console: Mfg 0, Id 60 console: STDV console: 8010: heap is already above this point console: SBDD console: 686I console: 0x3f8=ff console: PPCIH console: PCI bus 0 slot 5/0: reg 0x10 = 0x0 console: PCI bus 0 slot 5/0: reg 0x14 = 0x0 console: PCI bus 0 slot 5/0: reg 0x18 = 0x0 console: PCI bus 0 slot 5/0: reg 0x1c = 0x0 console: PCI bus 0 slot 5/0: reg 0x20 = 0x0 console: PCI bus 0 slot 5/0: reg 0x24 = 0x0 console: PCI bus 0 slot 5/1: reg 0x10 = 0xfff9 console: PCI bus 0 slot 5/1: reg 0x14 = 0xfffd console: PCI bus 0 slot 5/1: reg 0x18 = 0xfff9 console: PCI bus 0 slot 5/1: reg 0x1c = 0xfffd console: PCI bus 0 slot 5/1: reg 0x20 = 0xfff1 console: PCI bus 0 slot 5/1: reg 0x24 = 0x0 console: PCI bus 0 slot 5/2: reg 0x10 = 0x0 console: PCI bus 0 slot 5/2: reg 0x14 = 0x0 console: PCI bus 0 slot 5/2: reg 0x18 = 0x0 console: PCI bus 0 slot 5/2: reg 0x1c = 0x0 console: PCI bus 0 slot 5/2: reg 0x20 = 0xffe1 console: PCI bus 0 slot 5/2: reg 0x24 = 0x0 console: PCI bus 0 slot 5/3: reg 0x10 = 0x0 console: PCI bus 0 slot 5/3: reg 0x14 = 0x0 console: PCI bus 0 slot 5/3: reg 0x18 = 0x0 console: PCI bus 0 slot 5/3: reg 0x1c = 0x0 console: PCI bus 0 slot 5/3: reg 0x20 = 0xffe1 console: PCI bus 0 slot 5/3: reg 0x24 = 0x0 console: PCI bus 0 slot 5/4: reg 0x10 = 0x0 console: PCI bus 0 slot 5/4: reg 0x14 = 0x0 console: PCI bus 0 slot 5/4: reg 0x18 = 0x0 console: PCI bus 0 slot 5/4: reg 0x1c = 0x0 console: PCI bus 0 slot 5/4: reg 0x20 = 0x0 console: PCI bus 0 slot 5/4: reg 0x24 = 0x0 console: PCI bus 0 slot 5/5: reg 0x10 = 0x0 console: PCI bus 0 slot 5/5: reg 0x14 = 0x0 console: PCI bus 0 slot 5/5: reg 0x18 = 0x0 console: PCI bus 0 slot 5/5: reg 0x1c = 0x0 console: PCI bus 0 slot 5/5: reg 0x20 = 0x0 console: PCI bus 0 slot 5/5: reg 0x24 = 0x0 console: PCI bus 0 slot 5/6: reg 0x10 = 0x0 console: PCI bus 0 slot 5/6: reg 0x14 = 0x0 console: PCI bus 0 slot 5/6: reg 0x18 = 0x0 console: PCI bus 0 slot 5/6: reg 0x1c = 0x0 console: PCI bus 0 slot 5/6: reg 0x20 = 0x0 console: PCI bus 0 slot 5/6: reg 0x24 = 0x0 console: PCIS console: PCIR console: PCIW console: NETI console: RTCL console: PCID console: VGAI console: Default MODE_ID 2 console: starting radeon init... ^ Current QEMU is stuck here. Before it would continue: console: iobase=bfd0a100,mmbase=b505 console: mc_status=5 console: mc_status=5 console: mc_status=5 console: mc_status=5 console: ppll_div_3 = 301f4 console: Wrote: 0x0043 0x000301f4 0x (0x) console: Wrote: rd=67, fd=500, pd=3 console: VCLK_ECP_CNTL = 00C3 console: radeon init done console: FRBI console: cfb_c
Re: [RFC] Implementing vhost-user-blk device backend
Hi Stefan, Thank you for reviewing my work! All the improvements have been applied except for a small issue regarding object_add. > (qemu) object_add vhost-user-server,id=ID,chardev=CHARDEV,writable=on|off Currently I implement object_add feature in the following syntax which use unix_socket directly instead of chardev, (qemu) object_add vhost-user-server,id=id=disk,unix_socket=/tmp/vhost-user-blk_vhost.socket,name=disk,writable=off I know in QEMU we can create a socket server using chardev-add, (qemu) chardev-add socket,id=char1,path=/tmp/vhost-user-blk_vhost.socket But it seems it's a bit cumbersome to utilize chardev. Take QMP over socket as an example, $ x86_64-softmmu/qemu-system-x86_64 -drive file=dpdk.img,format=raw,if=none,id=disk -device ide-hd,drive=disk,bootindex=0 -m 128 -enable-kvm -chardev socket,id=mon1,path=/tmp/mon.sock,server,nowait -mon chardev=mon1,mode=control,pretty=on It doesn't support multiple concurrent client connections because of the limitation of chardev/char-socket.c. On Thu, Dec 19, 2019 at 10:31 PM Stefan Hajnoczi wrote: > On Mon, Nov 18, 2019 at 10:27:28PM +0800, Coiby Xu wrote: > > Hi all, > > > > This is an implementation of vhost-user-blk device backend by > > following > https://wiki.qemu.org/Google_Summer_of_Code_2019#vhost-user-blk_device_backend > . > > raw/qcow2 disk images can now be shared via vhost user protocol. In > > this way, it could provide better performance than QEMU's existing NBD > > support. > > Thank you for working on this feature! > > > +static size_t vub_iov_to_buf(const struct iovec *iov, > > + const unsigned int iov_cnt, void *buf) > > Please take a look at utils/iov.c. iov_to_buf_full() can be used > instead of defining this function. > > > +{ > > +size_t len; > > +unsigned int i; > > + > > +len = 0; > > +for (i = 0; i < iov_cnt; i++) { > > +memcpy(buf + len, iov[i].iov_base, iov[i].iov_len); > > +len += iov[i].iov_len; > > +} > > +return len; > > +} > > + > > +static VubDev *vub_device; > > If you switch to -object (see below) then this global pointer will go > away so I won't comment on it throughout this patch. > > > +static void vub_accept(QIONetListener *listener, QIOChannelSocket *sioc, > > + gpointer opaque) > > +{ > > +/* only one connection */ > > +if (vub_device->sioc) { > > +return; > > +} > > + > > +vub_device->sioc = sioc; > > +vub_device->listener = listener; > > +/* > > + * increase the object reference, so cioc will not freeed by > > + * qio_net_listener_channel_func which will call > object_unref(OBJECT(sioc)) > > + */ > > +object_ref(OBJECT(sioc)); > > + > > +qio_channel_set_name(QIO_CHANNEL(sioc), "vhost-server"); > > +if (!vug_init(&vub_device->parent, VHOST_USER_BLK_MAX_QUEUES, > sioc->fd, > > + vub_panic_cb, &vub_iface)) { > > +fprintf(stderr, "Failed to initialized libvhost-user-glib\n"); > > +} > > vug_init() uses the default GMainContext, which is bad for performance > when there are many devices because it cannot take advantage of > multi-core CPUs. vhost-user-server should support IOThread so that > devices can be run in dedicated threads. > > The nbd/server.c:NBDExport->ctx field serves this purpose in the NBD > server. It's a little trickier with libvhost-user-glib because the API > currently doesn't allow passing in a GMainContext and will need to be > extended. > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index cfcc044ce4..d8de179747 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -1614,6 +1614,33 @@ STEXI > > @findex acl_reset > > Remove all matches from the access control list, and set the default > > policy back to @code{deny}. > > +ETEXI > > + > > +{ > > +.name = "vhost_user_server_stop", > > +.args_type = "", > > +.params = "vhost_user_server_stop", > > +.help = "stop vhost-user-blk device backend", > > +.cmd= hmp_vhost_user_server_stop, > > +}, > > +STEXI > > +@item vhost_user_server_stop > > +@findex vhost_user_server_stop > > +Stop the QEMU embedded vhost-user-blk device backend server. > > +ETEXI > > The NBD server supports multiple client connections and exports > (drives). A vhost-user socket only supports one connection and one > device. I think it will be necessary to assign a unique identifier to > every vhost-user server. > > By the way, I think the server should be a UserCreatable Object so the > following syntax works: > > $ qemu -object vhost-user-server,id=ID,chardev=CHARDEV,writable=on|off > > And existing HMP/QMP commands can be used: > > (qemu) object_add vhost-user-server,id=ID,chardev=CHARDEV,writable=on|off > (qemu) object_del ID > > This way we don't need to define new HMP/QMP/command-line syntax for > vhost-user-server. > > If you grep for UserCreatable you'll find examples like "i
Re: [PATCH v2] hppa: allow max ram size upto 4Gb
On 1/3/20 10:54 AM, Igor Mammedov wrote: On Thu, 2 Jan 2020 21:22:12 +0100 Helge Deller wrote: On 02.01.20 18:46, Igor Mammedov wrote: Previous patch drops silent ram_size fixup and makes QEMU error out with: "RAM size more than 3840m is not supported" when user specified -m X more than supported value. User shouldn't be bothered with starting QEMU with valid CLI, so for the sake of user convenience allow using -m 4G vs -m 3840M. Requested-by: Helge Deller Signed-off-by: Igor Mammedov --- v2: - make main ram -1 prio, so it wouldn't conflict with other regions starting from 0xf900 I dislike it but if you feel it's really necessary feel free to ack it. Hard to find the v2 buried in the other series with my email client. should be applied on top of: "hppa: drop RAM size fixup" Hello Igor, I appreciate that you are trying to make it more cleaner. But, can't you merge both of your patches to one patch? Right now you have one patch "hppa: drop RAM size fixup", which is what I think is wrong. Then you add another one which somehow fixes it up again and adds other stuff. 1st patch bring it in line with other boards adding proper error check but without changing RAM size. While 2nd is changing device model (mapped RAM size) and clearly documents that it's a hack for user convenience, Hence I'd prefer to keep both separated. Having everything in one single patch makes your full change more understandable. Is it necessary to introduce clamped_ram_size and not continue with ram_size (even if you would add it as static local variable)? it's necessary since ram_size is global which should be kept == MachineState::ram_size. Later on I plan to remove the former altogether and maybe MachineState::ram_size aa well, since it could be read with memory_region_size(MachineState::ram). Why insist on clamping the ram? We recommend to model what the hardware does, and the hardware uses a full DIMM of DRAM, so 4GB, not less. What are the new problem introduced by using 4GB? I only see advantages doing so. This doesn't break your series. This doesn't break the CLI. Am I missing something? --- hw/hppa/machine.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c index ebbf44f..0302983 100644 --- a/hw/hppa/machine.c +++ b/hw/hppa/machine.c @@ -54,6 +54,7 @@ static uint64_t cpu_hppa_to_phys(void *opaque, uint64_t addr) static HPPACPU *cpu[HPPA_MAX_CPUS]; static uint64_t firmware_entry; +static ram_addr_t clamped_ram_size; static void machine_hppa_init(MachineState *machine) { @@ -74,8 +75,6 @@ static void machine_hppa_init(MachineState *machine) long i; unsigned int smp_cpus = machine->smp.cpus; -ram_size = machine->ram_size; - /* Create CPUs. */ for (i = 0; i < smp_cpus; i++) { char *name = g_strdup_printf("cpu%ld-io-eir", i); @@ -90,12 +89,14 @@ static void machine_hppa_init(MachineState *machine) } /* Limit main memory. */ -if (ram_size > FIRMWARE_START) { -error_report("RAM size more than %d is not supported", FIRMWARE_START); +if (machine->ram_size > 4 * GiB) { +error_report("RAM size more than 4Gb is not supported"); exit(EXIT_FAILURE); } +clamped_ram_size = machine->ram_size > FIRMWARE_START ? +FIRMWARE_START : machine->ram_size; -memory_region_add_subregion(addr_space, 0, machine->ram); +memory_region_add_subregion_overlap(addr_space, 0, machine->ram, -1); /* Init Dino (PCI host bus chip). */ pci_bus = dino_init(addr_space, &rtc_irq, &serial_irq); @@ -151,7 +152,7 @@ static void machine_hppa_init(MachineState *machine) qemu_log_mask(CPU_LOG_PAGE, "Firmware loaded at 0x%08" PRIx64 "-0x%08" PRIx64 ", entry at 0x%08" PRIx64 ".\n", firmware_low, firmware_high, firmware_entry); -if (firmware_low < ram_size || firmware_high >= FIRMWARE_END) { +if (firmware_low < clamped_ram_size || firmware_high >= FIRMWARE_END) { error_report("Firmware overlaps with memory or IO space"); exit(1); } @@ -204,7 +205,7 @@ static void machine_hppa_init(MachineState *machine) (1) Due to sign-extension problems and PDC, put the initrd no higher than 1G. (2) Reserve 64k for stack. */ -initrd_base = MIN(ram_size, 1 * GiB); +initrd_base = MIN(clamped_ram_size, 1 * GiB); initrd_base = initrd_base - 64 * KiB; initrd_base = (initrd_base - initrd_size) & TARGET_PAGE_MASK; @@ -232,7 +233,7 @@ static void machine_hppa_init(MachineState *machine) * various parameters in registers. After firmware initialization, * firmware will start the Linux kernel with ramdisk and cmdline. */ -cpu[0]->env.gr[26] = ram_size; +cpu[0]->env.gr[26] = clamped_ram_size; Helge, is this the code using this regis
Re: [PATCH v3 00/29] cputlb: Remove support for MMU_MODE*_SUFFIX
On 12/29/19 12:10 AM, Richard Henderson wrote: This is part of a project to raise the limit on NB_MMU_MODES. One of those is in cpu_ldst.h, in support of MMU_MODE*_SUFFIX. While this could be extended, it's not the best interface for such things. Better is a single interface that allows a variable mmu_idx. The best exemplars of that is the usage in target/mips and target/ppc. In the process, I tried to clean up the implementation of these functions for softmmu and user-only. Aleksander asked about code size changes. They vary between a minor size increase (e.g. for qemu-system-alpha, where there are in fact no uses of the functions, which are now present as out-of-line functions rather than eliminated inline functions), to a minor size decrease (e.g. -79k/-1.6% for qemu-system-i386). See below for details. Changes for v3: * Add patch to avoid breaking build for --enable-plugins. * Rebase on master and resolve conflicts. Changes for v2: * Significantly revised docs/devel/loads-stores.rst. * m68k and s390x dropped #defines and use *_mmuidx_ra directly. Patches lacking review: 0009-plugins-Include-trace-mem.h-in-api.c.patch 0010-cputlb-Move-body-of-cpu_ldst_template.h-out-of-li.patch r~ Cc: Aleksandar Markovic Cc: Aleksandar Rikalo Cc: Aurelien Jarno Cc: Chris Wulff Cc: David Gibson Cc: David Hildenbrand Cc: Edgar E. Iglesias Cc: Eduardo Habkost Cc: Guan Xuetao Cc: Laurent Vivier Cc: Marek Vasut Cc: Max Filippov Cc: Paolo Bonzini Cc: Peter Maydell Richard Henderson (29): target/xtensa: Use probe_access for itlb_hit_test cputlb: Use trace_mem_get_info instead of trace_mem_build_info trace: Remove trace_mem_build_info_no_se_[bl]e target/s390x: Include tcg.h in mem_helper.c target/arm: Include tcg.h in sve_helper.c accel/tcg: Include tcg.h in tcg-runtime.c linux-user: Include tcg.h in syscall.c linux-user: Include trace-root.h in syscall-trace.h plugins: Include trace/mem.h in api.c cputlb: Move body of cpu_ldst_template.h out of line translator: Use cpu_ld*_code instead of open-coding cputlb: Rename helper_ret_ld*_cmmu to cpu_ld*_code cputlb: Provide cpu_(ld,st}*_mmuidx_ra for user-only target/i386: Use cpu_*_mmuidx_ra instead of templates cputlb: Expand cpu_ldst_useronly_template.h in user-exec.c target/nios2: Remove MMU_MODE{0,1}_SUFFIX target/alpha: Remove MMU_MODE{0,1}_SUFFIX target/cris: Remove MMU_MODE{0,1}_SUFFIX target/i386: Remove MMU_MODE{0,1,2}_SUFFIX target/microblaze: Remove MMU_MODE{0,1,2}_SUFFIX target/sh4: Remove MMU_MODE{0,1}_SUFFIX target/unicore32: Remove MMU_MODE{0,1}_SUFFIX target/xtensa: Remove MMU_MODE{0,1,2,3}_SUFFIX target/m68k: Use cpu_*_mmuidx_ra instead of MMU_MODE{0,1}_SUFFIX target/mips: Use cpu_*_mmuidx_ra instead of MMU_MODE*_SUFFIX target/s390x: Use cpu_*_mmuidx_ra instead of MMU_MODE*_SUFFIX target/ppc: Use cpu_*_mmuidx_ra instead of MMU_MODE*_SUFFIX cputlb: Remove support for MMU_MODE*_SUFFIX cputlb: Expand cpu_ldst_template.h in cputlb.c Series: Tested-by: Philippe Mathieu-Daudé
Re: [PATCH v3 09/29] plugins: Include trace/mem.h in api.c
On 1/3/20 10:59 PM, Richard Henderson wrote: On 1/3/20 5:22 PM, Philippe Mathieu-Daudé wrote: Hi Richard, On 12/29/19 12:11 AM, Richard Henderson wrote: Code movement in an upcoming patch will show that this file was implicitly depending on trace/mem.h being included beforehand. Ah, it uses the TRACE_MEM_* macros from "trace/mem-internal.h", which is include by "trace/mem.h". OK. Which part requires "trace-root.h"? Isn't it "trace/mem-internal.h" that should include "trace-root.h"? I don't know -- perhaps it's not required at all. I think I did a blind copy of the trace related includes that were being removed. I'v tested your series, removing "trace-root.h" from this patch and building next patches, and there is no problem, it is indeed not required at all. Removing "trace-root.h": Reviewed-by: Philippe Mathieu-Daudé Regardless: Tested-by: Philippe Mathieu-Daudé
Re: [PATCH 3/3] capstone: Add skipdata hook for s390x
On 1/3/20 10:25 PM, Richard Henderson wrote: Capstone assumes any s390x unknown instruction is 2 bytes. Instead, use the ilen field in the first two bits of the instruction to stay in sync with the insn stream. Reviewed-by: Thomas Huth Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- disas.c | 37 + 1 file changed, 37 insertions(+) diff --git a/disas.c b/disas.c index 845c40fca8..1095bad049 100644 --- a/disas.c +++ b/disas.c @@ -178,6 +178,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info) to share this across calls and across host vs target disassembly. */ static __thread cs_insn *cap_insn; +/* + * The capstone library always skips 2 bytes for S390X. + * This is less than ideal, since we can tell from the first two bits + * the size of the insn and thus stay in sync with the insn stream. + */ +static size_t CAPSTONE_API +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size, + size_t offset, void *user_data) +{ +size_t ilen; + +/* See get_ilen() in target/s390x/internal.h. */ +switch (code[offset] >> 6) { +case 0: +ilen = 2; +break; +case 1: +case 2: +ilen = 4; +break; +default: +ilen = 6; +break; +} + +return ilen; +} + +static const cs_opt_skipdata cap_skipdata_s390x = { +.mnemonic = ".byte", +.callback = cap_skipdata_s390x_cb +}; + /* Initialize the Capstone library. */ /* ??? It would be nice to cache this. We would need one handle for the host and one for the target. For most targets we can reset specific @@ -208,6 +241,10 @@ static cs_err cap_disas_start(disassemble_info *info, csh *handle) /* "Disassemble" unknown insns as ".byte W,X,Y,Z". */ cs_option(*handle, CS_OPT_SKIPDATA, CS_OPT_ON); +if (info->cap_arch == CS_ARCH_SYSZ) { +cs_option(*handle, CS_OPT_SKIPDATA_SETUP, + (uintptr_t)&cap_skipdata_s390x); +} /* Allocate temp space for cs_disasm_iter. */ if (cap_insn == NULL) {
Re: [PATCH 2/3] capstone: Enable disassembly for s390x
On 1/3/20 10:24 PM, Richard Henderson wrote: Enable s390x, aka SYSZ, in the git submodule build. Set the capstone parameters for both s390x host and guest. Signed-off-by: Richard Henderson I'm fine with this patch because I don't use the s390 disas often. For the S390 experts, my previous analysis is here: https://www.mail-archive.com/qemu-devel@nongnu.org/msg667954.html Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé --- Makefile | 1 + disas.c| 3 +++ target/s390x/cpu.c | 4 3 files changed, 8 insertions(+) diff --git a/Makefile b/Makefile index 12e129ac9d..df1c692ccd 100644 --- a/Makefile +++ b/Makefile @@ -504,6 +504,7 @@ CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM CAP_CFLAGS += -DCAPSTONE_HAS_ARM CAP_CFLAGS += -DCAPSTONE_HAS_ARM64 CAP_CFLAGS += -DCAPSTONE_HAS_POWERPC +CAP_CFLAGS += -DCAPSTONE_HAS_SYSZ CAP_CFLAGS += -DCAPSTONE_HAS_X86 .PHONY: capstone/all diff --git a/disas.c b/disas.c index 3937da6157..845c40fca8 100644 --- a/disas.c +++ b/disas.c @@ -660,6 +660,9 @@ void disas(FILE *out, void *code, unsigned long size) print_insn = print_insn_m68k; #elif defined(__s390__) print_insn = print_insn_s390; +s.info.cap_arch = CS_ARCH_SYSZ; +s.info.cap_insn_unit = 2; +s.info.cap_insn_split = 6; #elif defined(__hppa__) print_insn = print_insn_hppa; #endif diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 625daeedd1..1734ad9c3a 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -43,6 +43,7 @@ #include "sysemu/tcg.h" #endif #include "fpu/softfloat-helpers.h" +#include "disas/capstone.h" #define CR0_RESET 0xE0UL #define CR14_RESET 0xC200UL; @@ -162,6 +163,9 @@ static void s390_cpu_disas_set_info(CPUState *cpu, disassemble_info *info) { info->mach = bfd_mach_s390_64; info->print_insn = print_insn_s390; +info->cap_arch = CS_ARCH_SYSZ; +info->cap_insn_unit = 2; +info->cap_insn_split = 6; } static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
Re: [PATCH 1/3] capstone: Update to next
On 1/3/20 10:24 PM, Richard Henderson wrote: Update to aaffb38c44fa. Choose this over the "current" 4.0.1 tag because next now includes the s390x z13 vector opcodes, and also the insn tables are now read-only. As Thomas suggested, Fixes: https://bugs.launchpad.net/qemu/+bug/1826175 With GCC on Linux: Tested-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- Makefile | 1 + capstone | 2 +- configure | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6b5ad1121b..12e129ac9d 100644 --- a/Makefile +++ b/Makefile @@ -499,6 +499,7 @@ dtc/%: .git-submodule-status # Remove all the extra -Warning flags that QEMU uses that Capstone doesn't; # no need to annoy QEMU developers with such things. CAP_CFLAGS = $(patsubst -W%,,$(CFLAGS) $(QEMU_CFLAGS)) +CAP_CFLAGS += -I$(SRC_PATH)/capstone/include CAP_CFLAGS += -DCAPSTONE_USE_SYS_DYN_MEM CAP_CFLAGS += -DCAPSTONE_HAS_ARM CAP_CFLAGS += -DCAPSTONE_HAS_ARM64 diff --git a/capstone b/capstone index 22ead3e0bf..aaffb38c44 16 --- a/capstone +++ b/capstone @@ -1 +1 @@ -Subproject commit 22ead3e0bfdb87516656453336160e0a37b066bf +Subproject commit aaffb38c44fa58f510ba9b6264f7079bfbba4c8e Looking at https://github.com/aquynh/capstone/pull/1549, this is unfortunate the upstream project use the 'squash merge request' feature :( Since I already reviewed various of the 1589 patches in the earlier version of this patch (22ead3e0bf..418d36d695) I'll only focus on the 291 'squashed' commits added on top: $ git log --oneline 418d36d695..aaffb38c44|wc -l 291 Most of the commits contains a single line of description, and various of them have hundreds of lines of changes. Similarly to my previous review, I skipped the architecture we don't support and X86. Still to many ARM/Aarch64 patches to review, so I'm focusing on the buildsys and changes on the API we use: Reviewed-by: Philippe Mathieu-Daudé diff --git a/configure b/configure index 940bf9e87a..de96bfe017 100755 --- a/configure +++ b/configure @@ -5062,7 +5062,7 @@ case "$capstone" in git_submodules="${git_submodules} capstone" fi mkdir -p capstone -QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include" +QEMU_CFLAGS="$QEMU_CFLAGS -I\$(SRC_PATH)/capstone/include/capstone" if test "$mingw32" = "yes"; then LIBCAPSTONE=capstone.lib else
[PATCH v2 2/5] hda-codec: fix recording rate control
Apply previous commit to hda_audio_input_cb for the same reasons. Signed-off-by: Volker Rümelin --- hw/audio/hda-codec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c index 768ba31e79..e711a99a41 100644 --- a/hw/audio/hda-codec.c +++ b/hw/audio/hda-codec.c @@ -265,8 +265,6 @@ static void hda_audio_input_cb(void *opaque, int avail) int64_t to_transfer = MIN(B_SIZE - (wpos - rpos), avail); -hda_timer_sync_adjust(st, -((wpos - rpos) + to_transfer - (B_SIZE >> 1))); - while (to_transfer) { uint32_t start = (uint32_t) (wpos & B_MASK); uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer); @@ -278,6 +276,8 @@ static void hda_audio_input_cb(void *opaque, int avail) break; } } + +hda_timer_sync_adjust(st, -((wpos - rpos) - (B_SIZE >> 1))); } static void hda_audio_output_timer(void *opaque) -- 2.16.4
[PATCH v2 3/5] paaudio: drop recording stream in qpa_fini_in
Every call to pa_stream_peek which returns a data length > 0 should have a corresponding pa_stream_drop. A call to qpa_read does not necessarily call pa_stream_drop immediately after a call to pa_stream_peek. Test in qpa_fini_in if a last pa_stream_drop is needed. This prevents following messages in the libvirt log file after a recording stream gets closed and a new one opened. pulseaudio: pa_stream_drop failed pulseaudio: Reason: Bad state pulseaudio: pa_stream_drop failed pulseaudio: Reason: Bad state To reproduce start qemu with -audiodev pa,id=audio0,in.mixing-engine=off and in the guest start and stop Audacity several times. Signed-off-by: Volker Rümelin --- audio/paaudio.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index 55a91f8980..7db1dc15f0 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -536,7 +536,6 @@ static void qpa_simple_disconnect(PAConnection *c, pa_stream *stream) { int err; -pa_threaded_mainloop_lock(c->mainloop); /* * wait until actually connects. workaround pa bug #247 * https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/247 @@ -550,7 +549,6 @@ static void qpa_simple_disconnect(PAConnection *c, pa_stream *stream) dolog("Failed to disconnect! err=%d\n", err); } pa_stream_unref(stream); -pa_threaded_mainloop_unlock(c->mainloop); } static void qpa_fini_out (HWVoiceOut *hw) @@ -558,8 +556,12 @@ static void qpa_fini_out (HWVoiceOut *hw) PAVoiceOut *pa = (PAVoiceOut *) hw; if (pa->stream) { -qpa_simple_disconnect(pa->g->conn, pa->stream); +PAConnection *c = pa->g->conn; + +pa_threaded_mainloop_lock(c->mainloop); +qpa_simple_disconnect(c, pa->stream); pa->stream = NULL; +pa_threaded_mainloop_unlock(c->mainloop); } } @@ -568,8 +570,20 @@ static void qpa_fini_in (HWVoiceIn *hw) PAVoiceIn *pa = (PAVoiceIn *) hw; if (pa->stream) { -qpa_simple_disconnect(pa->g->conn, pa->stream); +PAConnection *c = pa->g->conn; + +pa_threaded_mainloop_lock(c->mainloop); +if (pa->read_length) { +int r = pa_stream_drop(pa->stream); +if (r) { +qpa_logerr(pa_context_errno(c->context), + "pa_stream_drop failed\n"); +} +pa->read_length = 0; +} +qpa_simple_disconnect(c, pa->stream); pa->stream = NULL; +pa_threaded_mainloop_unlock(c->mainloop); } } -- 2.16.4
[PATCH v2 5/5] paaudio: wait until the recording stream is ready
Don't call pa_stream_peek before the recording stream is ready. Information to reproduce the problem. Start and stop Audacity in the guest several times because the problem is racy. libvirt log file: -audiodev pa,id=audio0,server=localhost,out.latency=3, out.mixing-engine=off,in.mixing-engine=off \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny, resourcecontrol=deny \ -msg timestamp=on : Domain id=4 is tainted: custom-argv char device redirected to /dev/pts/1 (label charserial0) audio: Device pcspk: audiodev default parameter is deprecated, please specify audiodev=audio0 audio: Device hda: audiodev default parameter is deprecated, please specify audiodev=audio0 pulseaudio: pa_stream_peek failed pulseaudio: Reason: Bad state pulseaudio: pa_stream_peek failed pulseaudio: Reason: Bad state Signed-off-by: Volker Rümelin --- audio/paaudio.c | 5 + 1 file changed, 5 insertions(+) diff --git a/audio/paaudio.c b/audio/paaudio.c index b23274550e..dbfe48c03a 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -162,6 +162,10 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length) CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail, "pa_threaded_mainloop_lock failed\n"); +if (pa_stream_get_state(p->stream) != PA_STREAM_READY) { +/* wait for stream to become ready */ +goto unlock; +} while (total < length) { size_t l; @@ -191,6 +195,7 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length) } } +unlock: pa_threaded_mainloop_unlock(c->mainloop); return total; -- 2.16.4
[PATCH v2 1/5] hda-codec: fix playback rate control
Since commit 1930616b98 "audio: make mixeng optional" the function hda_audio_output_cb can no longer assume the function parameter avail contains the free buffer size. With the playback mixing-engine turned off this leads to a broken playback rate control and playback buffer drops in regular intervals. This patch moves down the rate calculation, so the correct buffer fill level is used for the calculation. Signed-off-by: Volker Rümelin --- hw/audio/hda-codec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c index f17e8d8dce..768ba31e79 100644 --- a/hw/audio/hda-codec.c +++ b/hw/audio/hda-codec.c @@ -338,8 +338,6 @@ static void hda_audio_output_cb(void *opaque, int avail) return; } -hda_timer_sync_adjust(st, (wpos - rpos) - to_transfer - (B_SIZE >> 1)); - while (to_transfer) { uint32_t start = (uint32_t) (rpos & B_MASK); uint32_t chunk = (uint32_t) MIN(B_SIZE - start, to_transfer); @@ -351,6 +349,8 @@ static void hda_audio_output_cb(void *opaque, int avail) break; } } + +hda_timer_sync_adjust(st, (wpos - rpos) - (B_SIZE >> 1)); } static void hda_audio_compat_input_cb(void *opaque, int avail) -- 2.16.4
[PATCH v2 4/5] paaudio: try to drain the recording stream
There is no guarantee a single call to pa_stream_peek every timer_period microseconds can read a recording stream faster than the data gets produced at the source. Let qpa_read try to drain the recording stream. To reproduce the problem: Start qemu with -audiodev pa,id=audio0,in.mixing-engine=off On the host connect the qemu recording stream to the monitor of a hardware output device. While the problem can also be seen with a hardware input device, it's obvious with the monitor of a hardware output device. In the guest start audio recording with audacity and notice the slow recording data rate. Signed-off-by: Volker Rümelin --- audio/paaudio.c | 41 + 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/audio/paaudio.c b/audio/paaudio.c index 7db1dc15f0..b23274550e 100644 --- a/audio/paaudio.c +++ b/audio/paaudio.c @@ -156,34 +156,43 @@ static size_t qpa_read(HWVoiceIn *hw, void *data, size_t length) { PAVoiceIn *p = (PAVoiceIn *) hw; PAConnection *c = p->g->conn; -size_t l; -int r; +size_t total = 0; pa_threaded_mainloop_lock(c->mainloop); CHECK_DEAD_GOTO(c, p->stream, unlock_and_fail, "pa_threaded_mainloop_lock failed\n"); -if (!p->read_length) { -r = pa_stream_peek(p->stream, &p->read_data, &p->read_length); -CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail, - "pa_stream_peek failed\n"); -} +while (total < length) { +size_t l; +int r; + +if (!p->read_length) { +r = pa_stream_peek(p->stream, &p->read_data, &p->read_length); +CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail, + "pa_stream_peek failed\n"); +if (!p->read_length) { +/* buffer is empty */ +break; +} +} -l = MIN(p->read_length, length); -memcpy(data, p->read_data, l); +l = MIN(p->read_length, length - total); +memcpy((char *)data + total, p->read_data, l); -p->read_data += l; -p->read_length -= l; +p->read_data += l; +p->read_length -= l; +total += l; -if (!p->read_length) { -r = pa_stream_drop(p->stream); -CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail, - "pa_stream_drop failed\n"); +if (!p->read_length) { +r = pa_stream_drop(p->stream); +CHECK_SUCCESS_GOTO(c, r == 0, unlock_and_fail, + "pa_stream_drop failed\n"); +} } pa_threaded_mainloop_unlock(c->mainloop); -return l; +return total; unlock_and_fail: pa_threaded_mainloop_unlock(c->mainloop); -- 2.16.4
[PATCH v2 0/5] audio fixes
Here are five patches to fix PulseAudio playback/recording with the mixing engine off. v2: - Patch 3/5: Corrected error log message. Volker Rümelin (5): hda-codec: fix playback rate control hda-codec: fix recording rate control paaudio: drop recording stream in qpa_fini_in paaudio: try to drain the recording stream paaudio: wait until the recording stream is ready audio/paaudio.c | 70 hw/audio/hda-codec.c | 8 +++--- 2 files changed, 53 insertions(+), 25 deletions(-) -- 2.16.4
Re: [PATCH 2/5] hda-codec: fix recording rate control
> This mail is multipart text+html and "git am" can't process it (the > others are text only). Can you please resend the patches, preferably > with "git send-email" to avoid them being sent as multipart? They all > look good to me (this series and the 6th patch sent as separate mail). > > Sorry, I made a mistake here. I will send a version 2 patch series with git send-email. With best regards, Volker