Re: [Qemu-devel] [PATCH for-2.12] sam460ex: Fix timer frequency and clock multipliers
On Fri, Apr 06, 2018 at 12:42:48AM +0200, BALATON Zoltan wrote: > We only emulate timer running at CPU frequency which is what most > guests expect so set the frequency to match real hardware. This also > allows setting clock multipliers which caused slowdown previously due > to wrong timer frequency. > > Signed-off-by: BALATON ZoltanApplied to ppc-for-2.12. > --- > hw/ppc/ppc440_uc.c | 3 +-- > hw/ppc/sam460ex.c | 7 --- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c > index 976ab2b..e312fdb 100644 > --- a/hw/ppc/ppc440_uc.c > +++ b/hw/ppc/ppc440_uc.c > @@ -392,8 +392,7 @@ static uint32_t dcr_read_sdr(void *opaque, int dcrn) > case SDR0_CFGDATA: > switch (sdr->addr) { > case SDR0_STRP0: > -/* FIXME: Is this correct? This breaks timing in U-Boot */ > -ret = 0; /*(0xb5 << 8) | (1 << 4) | 9 */ > +ret = (0xb5 << 8) | (1 << 4) | 9; > break; > case SDR0_STRP1: > ret = (5 << 29) | (2 << 26) | (1 << 24); > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c > index 70b8e76..dfff262 100644 > --- a/hw/ppc/sam460ex.c > +++ b/hw/ppc/sam460ex.c > @@ -67,6 +67,7 @@ > IRQ12 = SM502_INT > */ > > +#define CPU_FREQ 115000 > #define SDRAM_NR_BANKS 4 > > /* FIXME: See u-boot.git 8ac41e, also fix in ppc440_uc.c */ > @@ -253,8 +254,8 @@ static int sam460ex_load_device_tree(hwaddr addr, > char *filename; > int fdt_size; > void *fdt; > -uint32_t tb_freq = 5000; > -uint32_t clock_freq = 5000; > +uint32_t tb_freq = CPU_FREQ; > +uint32_t clock_freq = CPU_FREQ; > > filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE); > if (!filename) { > @@ -416,7 +417,7 @@ static void sam460ex_init(MachineState *machine) > boot_info = g_malloc0(sizeof(*boot_info)); > env->load_info = boot_info; > > -ppc_booke_timers_init(cpu, 5000, 0); > +ppc_booke_timers_init(cpu, CPU_FREQ, 0); > ppc_dcr_init(env, NULL, NULL); > > /* PLB arbitrer */ -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] some ROMs questions
On Fri, Apr 06, 2018 at 01:59:03PM +0200, BALATON Zoltan wrote: > On Fri, 6 Apr 2018, Michael Tokarev wrote: > > 02.04.2018 17:30, BALATON Zoltan wrote: > > > On Mon, 2 Apr 2018, Michael Tokarev wrote: > > > > roms/u-boot-sam460ex/tools/updater/stubs.c - > > > > it is some strange symlink pointing to a strange place, probably should > > > > be removed? > > > > > > This does not seem to matter for building the rom image but this can be > > > fixed by converting it to a relative path. I've done that and will send a > > > patch to update the submodule as well to use the QEMU repo now that we > > > have a mirror there. Thanks for finding this. > > > > Hmm, I've no idea how to pull a submodule change.. :) > > git submodule update? But it's not in master yet only in David's > ppc-for-2.12 branch I think. > > > But I've one more question about this rom. When I'm trying to build it, > > the build fails: > > > > roms/u-boot-sam460ex$ make Sam460ex_config > > CROSS_COMPILE=powerpc64-linux-gnu- > > Generating include/autoconf.mk > > cc1: error: -mcall not supported in this configuration > > cc1: error: -mrelocatable not supported in this configuration > > cc1: error: -meabi not supported in this configuration > > cc1: error: -m64 requires a PowerPC64 cpu > > Generating include/autoconf.mk.dep > > cc1: error: -mcall not supported in this configuration > > cc1: error: -mrelocatable not supported in this configuration > > cc1: error: -meabi not supported in this configuration > > cc1: error: -m64 requires a PowerPC64 cpu > > Configuring for Sam460ex board... > > > > roms/u-boot-sam460ex$ make CROSS_COMPILE=powerpc64-linux-gnu- > > ... > > make -C arch/powerpc/cpu/ppc4xx start.o > > make[1]: Entering directory > > '/build/qemu/debian-qemu/roms/u-boot-sam460ex/arch/powerpc/cpu/ppc4xx' > > powerpc64-linux-gnu-gcc -D__ASSEMBLY__ -g -Os -mrelocatable -fPIC > > -meabi -ffunction-sections -fdata-sections -D__KERNEL__ > > -DTEXT_BASE=0xFFF8 > > -I/build/qemu/debian-qemu/roms/u-boot-sam460ex/include -fno-builtin > > -ffreestanding -nostdinc -isystem > > /usr/lib/gcc-cross/powerpc64-linux-gnu/6/include -pipe -DCONFIG_PPC > > -D__powerpc__ -DCONFIG_4xx -ffixed-r2 -mstring -msoft-float -Wa,-m440 > > -mcpu=440 -DCONFIG_440=1 -I../bios_emulator/scitech/include > > -I../bios_emulator/scitech/src/x86emu -Dprintk=printf \ > >-o start.o start.S -c > > start.S:1:0: error: -mcall not supported in this configuration > > /* > > > > start.S:1:0: error: -mrelocatable not supported in this configuration > > start.S:1:0: error: -meabi not supported in this configuration > > start.S:1:0: error: -m64 requires a PowerPC64 cpu > > /build/qemu/debian-qemu/roms/u-boot-sam460ex/config.mk:249: recipe for > > target 'start.o' failed > > make[1]: *** [start.o] Error 1 > > make[1]: Leaving directory > > '/build/qemu/debian-qemu/roms/u-boot-sam460ex/arch/powerpc/cpu/ppc4xx' > > Makefile:353: recipe for target 'arch/powerpc/cpu/ppc4xx/start.o' failed > > make: *** [arch/powerpc/cpu/ppc4xx/start.o] Error 2 > > > > What I'm doing wrong? > > > > $ powerpc64-linux-gnu-gcc --version > > powerpc64-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516 > > Note sure, maybe building it with ppc64 compiler or a too new one? This > board is 32bit and the source is from 2011 so I'm not sure what it does with > recent compilers. I have powerpc-elf-gcc 4.9.3 which works. Yeah, if it's a 32-bit board, I'm pretty sure you'll need a 32-bit compiler. Or at least add -m32 if it's a biarch compiler. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
[Qemu-devel] [qemu RFC] qapi: add "firmware.json"
Add a schema that describes the properties of virtual machine firmware. Each firmware executable installed on a host system should come with a JSON file that conforms to this schema, and informs the management applications about the firmware's properties. In addition, a configuration directory with symlinks to the JSON files should exist, with the symlinks carefully named to reflect a priority order. Management applications can then search this directory in priority order for the first firmware executable that satisfies their search criteria. The found JSON file provides the management layer with domain configuration bits that are required to run the firmware binary. Cc: "Daniel P. Berrange"Cc: Alexander Graf Cc: Ard Biesheuvel Cc: David Gibson Cc: Eric Blake Cc: Gary Ching-Pang Lin Cc: Gerd Hoffmann Cc: Kashyap Chamarthy Cc: Markus Armbruster Cc: Michael Roth Cc: Michal Privoznik Cc: Peter Krempa Cc: Peter Maydell Cc: Thomas Huth Signed-off-by: Laszlo Ersek --- Notes: Folks on the CC list, please try to see if the suggested schema is flexible enough to describe the virtual firmware(s) that you are familiar with. Thanks! Makefile | 9 ++ Makefile.objs | 4 + qapi/firmware.json| 343 ++ qapi/qapi-schema.json | 1 + qmp.c | 5 + .gitignore| 4 + 6 files changed, 366 insertions(+) create mode 100644 qapi/firmware.json diff --git a/Makefile b/Makefile index 727ef118f3d9..e088e3c1b39f 100644 --- a/Makefile +++ b/Makefile @@ -97,6 +97,7 @@ GENERATED_FILES += qapi/qapi-types-block.h qapi/qapi-types-block.c GENERATED_FILES += qapi/qapi-types-char.h qapi/qapi-types-char.c GENERATED_FILES += qapi/qapi-types-common.h qapi/qapi-types-common.c GENERATED_FILES += qapi/qapi-types-crypto.h qapi/qapi-types-crypto.c +GENERATED_FILES += qapi/qapi-types-firmware.h qapi/qapi-types-firmware.c GENERATED_FILES += qapi/qapi-types-introspect.h qapi/qapi-types-introspect.c GENERATED_FILES += qapi/qapi-types-migration.h qapi/qapi-types-migration.c GENERATED_FILES += qapi/qapi-types-misc.h qapi/qapi-types-misc.c @@ -115,6 +116,7 @@ GENERATED_FILES += qapi/qapi-visit-block.h qapi/qapi-visit-block.c GENERATED_FILES += qapi/qapi-visit-char.h qapi/qapi-visit-char.c GENERATED_FILES += qapi/qapi-visit-common.h qapi/qapi-visit-common.c GENERATED_FILES += qapi/qapi-visit-crypto.h qapi/qapi-visit-crypto.c +GENERATED_FILES += qapi/qapi-visit-firmware.h qapi/qapi-visit-firmware.c GENERATED_FILES += qapi/qapi-visit-introspect.h qapi/qapi-visit-introspect.c GENERATED_FILES += qapi/qapi-visit-migration.h qapi/qapi-visit-migration.c GENERATED_FILES += qapi/qapi-visit-misc.h qapi/qapi-visit-misc.c @@ -132,6 +134,7 @@ GENERATED_FILES += qapi/qapi-commands-block.h qapi/qapi-commands-block.c GENERATED_FILES += qapi/qapi-commands-char.h qapi/qapi-commands-char.c GENERATED_FILES += qapi/qapi-commands-common.h qapi/qapi-commands-common.c GENERATED_FILES += qapi/qapi-commands-crypto.h qapi/qapi-commands-crypto.c +GENERATED_FILES += qapi/qapi-commands-firmware.h qapi/qapi-commands-firmware.c GENERATED_FILES += qapi/qapi-commands-introspect.h qapi/qapi-commands-introspect.c GENERATED_FILES += qapi/qapi-commands-migration.h qapi/qapi-commands-migration.c GENERATED_FILES += qapi/qapi-commands-misc.h qapi/qapi-commands-misc.c @@ -149,6 +152,7 @@ GENERATED_FILES += qapi/qapi-events-block.h qapi/qapi-events-block.c GENERATED_FILES += qapi/qapi-events-char.h qapi/qapi-events-char.c GENERATED_FILES += qapi/qapi-events-common.h qapi/qapi-events-common.c GENERATED_FILES += qapi/qapi-events-crypto.h qapi/qapi-events-crypto.c +GENERATED_FILES += qapi/qapi-events-firmware.h qapi/qapi-events-firmware.c GENERATED_FILES += qapi/qapi-events-introspect.h qapi/qapi-events-introspect.c GENERATED_FILES += qapi/qapi-events-migration.h qapi/qapi-events-migration.c GENERATED_FILES += qapi/qapi-events-misc.h qapi/qapi-events-misc.c @@ -581,6 +585,7 @@ qapi-modules = $(SRC_PATH)/qapi/qapi-schema.json $(SRC_PATH)/qapi/common.json \ $(SRC_PATH)/qapi/block.json $(SRC_PATH)/qapi/block-core.json \ $(SRC_PATH)/qapi/char.json \ $(SRC_PATH)/qapi/crypto.json \ + $(SRC_PATH)/qapi/firmware.json \ $(SRC_PATH)/qapi/introspect.json \ $(SRC_PATH)/qapi/migration.json \ $(SRC_PATH)/qapi/misc.json \ @@ -600,6 +605,7 @@ qapi/qapi-types-block.c qapi/qapi-types-block.h \ qapi/qapi-types-char.c qapi/qapi-types-char.h \ qapi/qapi-types-common.c qapi/qapi-types-common.h \ qapi/qapi-types-crypto.c
[Qemu-devel] block-stream/commit and mixing internal and external snapshots
Perhaps others have already known this, but I just realized that if you mix internal and external snapshots, you can set yourself up for massive failures when trying to use block-stream or block-commit to consolidate data across the external backing chain, without also thinking about the internal snapshots. Here's a quick demonstration: $ # create the backing file, with all 1's; 1M clusters for convenience $ qemu-img create -f qcow2 -o cluster_size=1m base.qcow2 4M Formatting 'base.qcow2', fmt=qcow2 size=4194304 cluster_size=1048576 lazy_refcounts=off refcount_bits=16 $ qemu-io -c 'w -P 1 0 4m' -f qcow2 base.qcow2 wrote 4194304/4194304 bytes at offset 0 4 MiB, 1 ops; 0.0050 sec (791.139 MiB/sec and 197.7848 ops/sec) $ # create the wrapper file, write 2 to the first 2 clusters $ qemu-img create -f qcow2 -o backing_file=base.qcow2,backing_fmt=qcow2 top.qcow2 Formatting 'top.qcow2', fmt=qcow2 size=4194304 backing_file=base.qcow2 backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 $ qemu-io -c 'w -P 2 0 2m' -f qcow2 top.qcow2 wrote 2097152/2097152 bytes at offset 0 2 MiB, 1 ops; 0.0009 sec (2.144 GiB/sec and 1097.6948 ops/sec) $ # create an internal snapshot, then write 3 to the middle 2 clusters $ qemu-img snapshot -c snap1 top.qcow2 $ qemu-io -c 'w -P 3 1m 2m' -f qcow2 top.qcow2 wrote 2097152/2097152 bytes at offset 1048576 2 MiB, 1 ops; 0.0009 sec (2.102 GiB/sec and 1076.4263 ops/sec) $ # we've mixed internal and external; let's shorten the chain now $ qemu-img info top.qcow2 image: top.qcow2 file format: qcow2 virtual size: 4.0M (4194304 bytes) disk size: 2.3M cluster_size: 65536 backing file: base.qcow2 backing file format: qcow2 Snapshot list: IDTAG VM SIZEDATE VM CLOCK 1 snap1 0 2018-04-06 16:44:54 00:00:00.000 Format specific information: compat: 1.1 lazy refcounts: false refcount bits: 16 corrupt: false $ qemu-img rebase -f qcow2 -b '' top.qcow2 $ # create second snapshot, then revert to the first $ qemu-img snapshot -c snap2 top.qcow2 $ qemu-img snapshot -a snap1 top.qcow2 $ # contents at the time we took snap1 were 2211, right? OOOPS! $ qemu-io -c 'r -P 2 0 2m' -c 'r -P 1 2m 2m' -f qcow2 top.qcow2 read 2097152/2097152 bytes at offset 0 2 MiB, 1 ops; 0.0004 sec (3.914 GiB/sec and 2004.0080 ops/sec) Pattern verification failed at offset 2097152, 2097152 bytes read 2097152/2097152 bytes at offset 2097152 2 MiB, 1 ops; 0. sec (24.723 GiB/sec and 12658.2278 ops/sec) $ # the last two clusters were rewritten from 1 to 0. :( $ qemu-io -c 'r -P 0 2m 2m' -f qcow2 top.qcow2 read 2097152/2097152 bytes at offset 2097152 2 MiB, 1 ops; 0.0001 sec (13.754 GiB/sec and 7042.2535 ops/sec) $ # repair the damage, for now, and write 4 into last cluster... $ qemu-img rebase -u -f qcow2 -b base.qcow2 -F qcow2 top.qcow2 $ qemu-io -c 'w -P 4 3m 1m' -f qcow2 top.qcow2 wrote 1048576/1048576 bytes at offset 3145728 1 MiB, 1 ops; 0.0005 sec (1.713 GiB/sec and 1754.3860 ops/sec) $ # now let's try committing instead $ qemu-img commit -f qcow2 -d top.qcow2 Image committed. $ # revert back to snap2, which had contents 2331, right? OOPS! $ qemu-img snapshot -a snap2 top.qcow2 $ qemu-io -c 'r -P 2 0 1m' -c 'r -P 3 1m 1m' -c 'r -P 3 2m 1m' -c 'r -P 1 3m 1m' -f qcow2 top.qcow2 read 1048576/1048576 bytes at offset 0 1 MiB, 1 ops; 0.0002 sec (3.860 GiB/sec and 3952.5692 ops/sec) read 1048576/1048576 bytes at offset 1048576 1 MiB, 1 ops; 0.0002 sec (3.577 GiB/sec and 3663.0037 ops/sec) read 1048576/1048576 bytes at offset 2097152 1 MiB, 1 ops; 0.0002 sec (4.628 GiB/sec and 4739.3365 ops/sec) Pattern verification failed at offset 3145728, 1048576 bytes read 1048576/1048576 bytes at offset 3145728 1 MiB, 1 ops; 0.0007 sec (1.345 GiB/sec and 1377.4105 ops/sec) $ # the last cluster was rewritten from 1 to 4. :( $ qemu-io -c 'r -P 4 3m 1m' -f qcow2 top.qcow2 read 1048576/1048576 bytes at offset 3145728 1 MiB, 1 ops; 0.0011 sec (878.735 MiB/sec and 878.7346 ops/sec) The root cause to all of this is that right now, ALL internal snapshots share the same backing file information in the file header; but block-stream operations only modify the active snapshot. The actions of changing the backing file or of rewriting the clusters in the backing file don't break the active snapshot, but DO bleed through to the internal snapshots, for any cluster where the internal snapshot was relying on the backing file. Does this mean we should make it harder to perform external block operations on a qcow2 file that has internal snapshots (either refuse outright, or at least require a 'force' flag to let the user acknowledge the risk)? Similarly, should it be harder to create an internal snapshot when an image already has an external backing file, and/or should we improve the qcow2 specification of internal snapshot descriptors to record a per-snapshot backing file rather than the current approach that all snapshots share the same backing file?
[Qemu-devel] [Bug 1191326] Re: QNX 4 doesn't boot on qemu >= 1.3
I wonder - would be a record using rr of any help? I can create record for QEMU 1.2.0 where it works and on current QEMU. Also, I did a bit of debugging myself around the DMA code as per comment #3 it was introduced in a commit that changed some of the DMA. What I did was that I added some debug printfs [1] to dma_memory_rw() to QEMU 1.2.0 and to QEMU 2.11.1. I noticed on thing - there is a big difference between writes between the two versions. Because this stuff is completely outside my knowledge, I don't know whether this is important or not, but better more information that not enough. For recent versions of QEMU I see a few 16 B writes from address 0x6d10 and addresses close to it which contain some data. Immediately after that there is a ton of 8B writes from addresses starting at 0x102004 which contain zeros only. On the other hand, the QEMU 1.2.0 is missing the initial 16B writes, but then there's even more 8B writes from addresses around 0x102004 which contain some data instead of zeros like in the current version. [1] the printf looks like this: printf("DEBUG: DMA %s at address %lx %lu bytes: ", ((dir == DMA_DIRECTION_FROM_DEVICE) ? "read" : "write"), addr, len); -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1191326 Title: QNX 4 doesn't boot on qemu >= 1.3 Status in QEMU: Confirmed Bug description: I am using virtual machine with QNX4 operating system installed on it. I updated my qemu from version to newer and QNX4 doesn't start any more. All is ok on version 1.2 but when I try to use any newer version (1.3, 1.4, 1.5) QNX4 doesn't boot. I tried on windows and linux ubuntu hosts - effects are the same. When virtual machine boots qnx bootloader loads and starts operating system. In the next step qnx starts its ide driver, which detects qemu harddisk and cdrom. Problem starts when operating system tries mount partition - an error occur and qnx stop booting procedure: mount -p "No bios signature in partition sector on /dev/hd0" I have tried install qnx from cdrom but it seems that there is the same problem. QNX installer boot from cdrom, detects hard disk and cdrom, but cdrom can't be mounted in the next step of installation procedure. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1191326/+subscriptions
[Qemu-devel] [PATCH] xen/pt: use address_space_memory object for memory region hooks
Commit 99605175c (xen-pt: Fix PCI devices re-attach failed) introduced a subtle bug. As soon as the guest switches off Bus Mastering on the device it immediately causes all the BARs be unmapped due to the DMA address space of the device being changed. This is undesired behavior because the guest may try to communicate with the device after that which triggers the following errors in the logs: [00:05.0] xen_pt_bar_read: Error: Should not read BAR through QEMU. @0x0200 [00:05.0] xen_pt_bar_write: Error: Should not write BAR through QEMU. @0x0200 The issue that the original patch tried to workaround (uneven number of region_add/del calls on device attach/detach) was fixed in later QEMU versions. Signed-off-by: Igor DruzhininReported-by: Ross Lagerwall --- hw/xen/xen_pt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index 9b7a960..e5a6eff 100644 --- a/hw/xen/xen_pt.c +++ b/hw/xen/xen_pt.c @@ -907,7 +907,7 @@ out: } } -memory_listener_register(>memory_listener, >dev.bus_master_as); +memory_listener_register(>memory_listener, _space_memory); memory_listener_register(>io_listener, _space_io); s->listener_set = true; XEN_PT_LOG(d, -- 2.7.4
Re: [Qemu-devel] [PATCH v2 00/17] Translation loop conversion for sh4/sparc/mips/s390x/openrisc/riscv targets
Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 1523038800-2494-1-git-send-email-c...@braap.org Subject: [Qemu-devel] [PATCH v2 00/17] Translation loop conversion for sh4/sparc/mips/s390x/openrisc/riscv targets === TEST SCRIPT BEGIN === #!/bin/bash BASE=base n=1 total=$(git log --oneline $BASE.. | wc -l) failed=0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram commits="$(git log --format=%H --reverse $BASE..)" for c in $commits; do echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..." if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then failed=1 echo fi n=$((n+1)) done exit $failed === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu d7fa7fb504..08e173f294 master -> master * [new tag] patchew/1523038800-2494-1-git-send-email-c...@braap.org -> patchew/1523038800-2494-1-git-send-email-c...@braap.org Switched to a new branch 'test' 3bf0c1c6d4 target/riscv: convert to TranslatorOps 8c8cf713ea target/riscv: convert to DisasContextBase cda65dc923 target/riscv: convert to DisasJumpType b959e20396 target/openrisc: convert to TranslatorOps 8a1f5ad2cd target/openrisc: convert to DisasContextBase f6ef3b753a target/s390x: convert to TranslatorOps 8dad6fc829 target/s390x: convert to DisasContextBase afc9aac550 target/s390x: convert to DisasJumpType f588ffbd0c target/mips: convert to TranslatorOps b8797aa5c2 target/mips: use *ctx for DisasContext bf4f835373 target/mips: convert to DisasContextBase a422870894 target/mips: convert to DisasJumpType fdc97e620f target/sparc: convert to TranslatorOps 880c214bd0 target/sparc: convert to DisasContextBase feec87def1 target/sparc: convert to DisasJumpType 5d0217ee29 target/sh4: convert to TranslatorOps c1fffdaba5 translator: merge max_insns into DisasContextBase === OUTPUT BEGIN === Checking PATCH 1/17: translator: merge max_insns into DisasContextBase... Checking PATCH 2/17: target/sh4: convert to TranslatorOps... Checking PATCH 3/17: target/sparc: convert to DisasJumpType... Checking PATCH 4/17: target/sparc: convert to DisasContextBase... Checking PATCH 5/17: target/sparc: convert to TranslatorOps... Checking PATCH 6/17: target/mips: convert to DisasJumpType... Checking PATCH 7/17: target/mips: convert to DisasContextBase... Checking PATCH 8/17: target/mips: use *ctx for DisasContext... ERROR: space prohibited after that '&' (ctx:WxW) #81: FILE: target/mips/translate.c:20220: +ctx->kscrexist = (env->CP0_Config4 >> CP0C4_KScrExist) & 0xff; ^ total: 1 errors, 0 warnings, 254 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 9/17: target/mips: convert to TranslatorOps... Checking PATCH 10/17: target/s390x: convert to DisasJumpType... ERROR: braces {} are necessary for all arms of this statement #3464: FILE: target/s390x/translate.c:6225: +} while (status == DISAS_NEXT); [...] total: 1 errors, 0 warnings, 3340 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 11/17: target/s390x: convert to DisasContextBase... Checking PATCH 12/17: target/s390x: convert to TranslatorOps... Checking PATCH 13/17: target/openrisc: convert to DisasContextBase... ERROR: braces {} are necessary for all arms of this statement #223: FILE: target/openrisc/translate.c:1605: +} while (!dc->base.is_jmp [...] total: 1 errors, 0 warnings, 246 lines checked Your patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. Checking PATCH 14/17: target/openrisc: convert to TranslatorOps... Checking PATCH 15/17: target/riscv: convert to DisasJumpType... Checking PATCH 16/17: target/riscv: convert to DisasContextBase... Checking PATCH 17/17: target/riscv: convert to TranslatorOps... === OUTPUT END === Test command exited with code: 1 --- Email generated automatically by Patchew [http://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [Qemu-devel] [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 03:36:03PM +0300, Oleksandr Andrushchenko wrote: > On 04/06/2018 02:57 PM, Gerd Hoffmann wrote: > > Hi, > > > >>>I fail to see any common ground for xen-zcopy and udmabuf ... > >>Does the above mean you can assume that xen-zcopy and udmabuf > >>can co-exist as two different solutions? > >Well, udmabuf route isn't fully clear yet, but yes. > > > >See also gvt (intel vgpu), where the hypervisor interface is abstracted > >away into a separate kernel modules even though most of the actual vgpu > >emulation code is common. > Thank you for your input, I'm just trying to figure out > which of the three z-copy solutions intersect and how much > >>And what about hyper-dmabuf? xen z-copy solution is pretty similar fundamentally to hyper_dmabuf in terms of these core sharing feature: 1. the sharing process - import prime/dmabuf from the producer -> extract underlying pages and get those shared -> return references for shared pages 2. the page sharing mechanism - it uses Xen-grant-table. And to give you a quick summary of differences as far as I understand between two implementations (please correct me if I am wrong, Oleksandr.) 1. xen-zcopy is DRM specific - can import only DRM prime buffer while hyper_dmabuf can export any dmabuf regardless of originator 2. xen-zcopy doesn't seem to have dma-buf synchronization between two VMs while (as danvet called it as remote dmabuf api sharing) hyper_dmabuf sends out synchronization message to the exporting VM for synchronization. 3. 1-level references - when using grant-table for sharing pages, there will be same # of refs (each 8 byte) as # of shared pages, which is passed to the userspace to be shared with importing VM in case of xen-zcopy. Compared to this, hyper_dmabuf does multiple level addressing to generate only one reference id that represents all shared pages. 4. inter VM messaging (hype_dmabuf only) - hyper_dmabuf has inter-vm msg communication defined for dmabuf synchronization and private data (meta info that Matt Roper mentioned) exchange. 5. driver-to-driver notification (hyper_dmabuf only) - importing VM gets notified when newdmabuf is exported from other VM - uevent can be optionally generated when this happens. 6. structure - hyper_dmabuf is targetting to provide a generic solution for inter-domain dmabuf sharing for most hypervisors, which is why it has two layers as mattrope mentioned, front-end that contains standard API and backend that is specific to hypervisor. > >No idea, didn't look at it in detail. > > > >Looks pretty complex from a distant view. Maybe because it tries to > >build a communication framework using dma-bufs instead of a simple > >dma-buf passing mechanism. we started with simple dma-buf sharing but realized there are many things we need to consider in real use-case, so we added communication , notification and dma-buf synchronization then re-structured it to front-end and back-end (this made things more compicated..) since Xen was not our only target. Also, we thought passing the reference for the buffer (hyper_dmabuf_id) is not secure so added uvent mechanism later. > Yes, I am looking at it now, trying to figure out the full story > and its implementation. BTW, Intel guys were about to share some > test application for hyper-dmabuf, maybe I have missed one. > It could probably better explain the use-cases and the complexity > they have in hyper-dmabuf. One example is actually in github. If you want take a look at it, please visit: https://github.com/downor/linux_hyper_dmabuf_test/tree/xen/simple_export > > > >Like xen-zcopy it seems to depend on the idea that the hypervisor > >manages all memory it is easy for guests to share pages with the help of > >the hypervisor. > So, for xen-zcopy we were not trying to make it generic, > it just solves display (dumb) zero-copying use-cases for Xen. > We implemented it as a DRM helper driver because we can't see any > other use-cases as of now. > For example, we also have Xen para-virtualized sound driver, but > its buffer memory usage is not comparable to what display wants > and it works somewhat differently (e.g. there is no "frame done" > event, so one can't tell when the sound buffer can be "flipped"). > At the same time, we do not use virtio-gpu, so this could probably > be one more candidate for shared dma-bufs some day. > > Which simply isn't the case on kvm. > > > >hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build > >on top of xen-zcopy. > Hm, I can imagine that: xen-zcopy could be a library code for hyper-dmabuf > in terms of implementing all that page sharing fun in multiple directions, > e.g. Host->Guest, Guest->Host, Guest<->Guest. > But I'll let Matt and Dongwon to comment on that. I think we can definitely collaborate. Especially, maybe we are using some outdated sharing mechanism/grant-table mechanism in our Xen backend (thanks for bringing that up Oleksandr). However, the question is once we collaborate
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
"Mark Cave-Ayland"wrote on 04/06/2018 09:14:14 AM: > From: "Mark Cave-Ayland" > To: alar...@ddci.com, "Peter Maydell" > Cc: "Stefan Weil" , "QEMU Developers" > Date: 04/06/2018 09:14 AM > Subject: Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11 > > On 06/04/18 14:28, alar...@ddci.com wrote: > > > I was not successful with the wiki instructions for "Native builds > > with MSYS2": > > > >./qemu-2.12.0-rc2/configure --python=/usr/bin/python2 \ > >'--with-pkgversion=DDCI QEMU 2.12.0-rc2' \ > >--prefix=/usr/local/qemu \ > >'--target-list=aarch64-softmmu ppc64-softmmu x86_64-softmmu' > >make > >... > >C:\msys2-x86_64_20161025\msys64\mingw64\bin\ar.exe: creating > > libfdt/libfdt.a > >make[1]: *** No rule to make target ...build/capstone/capstone.lib'. > > Stop. > >make: *** [Makefile:503: subdir-capstone] Error 2 > > Ah I believe those instructions were mine from when I had temporary > access to a Windows VM for testing last year. > > Since the instructions pre-dated the inclusion of capstone as a > dependency, is it as simple as adding: > > git submodule update --init capstone > > before running configure and make? I didn't do the "git" parts. I extracted from the source tarball. Is the tarball source supposed to work, or is it necessary to always use git?
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
"Peter Maydell"wrote on 04/06/2018 12:21:37 PM: > On 6 April 2018 at 18:06, wrote: > > > > FWIW, the compile had some anomalous behavior: > > > > .../qemu-2.12.0-rc2/scripts/feature_to_c.sh: line 71: /usr/bin/sed: > > Invalid argument > > > > line 71 is: > > arrayname=xml_feature_$(echo $input | sed 's,.*/,,; s/[-.]/_/g') > > > > Not sure what the problem is, but I always use "-e" before scripts. > > That's a bit odd. (POSIX allows both "-e script" and just "script".) > What version of sed is this, and what version of /bin/sh ? I don't know what the issue is. When I execute that command from my normal bash shell, it works just fine. This is current Cygwin, so: qemu$ sed --version sed (GNU sed) 4.4 Packaged by Cygwin (4.4-1) ... qemu$ /bin/sh --version GNU bash, version 4.4.12(3)-release (x86_64-unknown-cygwin) ... Perhaps I misread the sed manual, but I thought when you used a character other than "/", you had to preface the character with a backslash, e.g., sed 's\,.*,,;' But, the above is all just speculation. I don't know what the real problem is. FWIW, I rebuilt twice more and the error didn't show up again, but I did get some other spurious errors, one from GCC "invalid argument" and another: /bin/sh: /usr/bin/x86_64-w64-mingw32-gcc: Device or resource busy make: *** [/cygdrive/c/scm/Deos/products/qemu/branches/2.12/output/qemu-2.12.0-rc2/rules.mak:66: stubs/notify-event.o] Error 126 So perhaps it is just normal Cygwin "fork" errors, or my use of "make -j8". I think you can ignore this. Sorry for the spam.
[Qemu-devel] [PATCH 1/1] Make qemu-bridge-helper work in macOS and FreeBSD
Add macOS and FreeBSD support to the qemu-bridge-helper. Differences when compared to linux version. 1) Does no drop privileges at the start of the process and run as root throughout the lifetime of the process there by increasing the risk incase of security vulnerability. 2) Does not support --use-vnet flag. Signed-off-by: Nikhil Balachandra--- Makefile | 8 +++- Makefile.objs| 5 ++ configure| 5 ++ qemu-bridge-helper.c | 131 +-- 4 files changed, 102 insertions(+), 47 deletions(-) This patch makes qemu-bridge-helper work in macOS and FreeBSD platforms. The patch also contains changes in configure script and Makefile for building qemu-bridge-helper in these platforms. After applying the patch, running `$ make` or `$ make qemu-bridge-helper` should build the binary (see macOS caveat below). While the compilation process is straight-forward in FreeBSD, there is one caveat while building for the macOS. Unlike FreeBSD, macOS does not ship with net/if_bridgevar.h header file. This Header file however could be obtained from the Darwin/XNU repository[1] and can be copied somewhere in the include path before building. Eventhough macOS does not ship with the if_bridgevar.h header file[2], I expect the API to remain stable as this header file is similar to what is found in other BSDs. If this patch is decided to be included in the qemu, can experienced qemu developers please tell me how to go about having this header file in the include path such that it does not require manually downloading and copying the file[3]? This is also my first patch to qemu and opensource C codebase project. Please be critical while reviewing the code. [1] - https://github.com/apple/darwin-xnu/blob/master/bsd/net/if_bridgevar.h [2] - Adding to the complexity, this header file is also marked as private/internal. [3] - Couple of ideas: This could be automatically downloaded during the build or can be directly copied into the qemu source tree if license permits such a usage. Thanks and Regards, Nikhil diff --git a/Makefile b/Makefile index 727ef118f3..c581d91c6b 100644 --- a/Makefile +++ b/Makefile @@ -347,7 +347,10 @@ $(call set-vpath, $(SRC_PATH)) LIBS+=-lz $(LIBS_TOOLS) +# Build Helpers (currently only qemu-bridge-helper) for Linux, FreeBSD and macOS. HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF) +HELPERS-$(CONFIG_FREEBSD) = qemu-bridge-helper$(EXESUF) +HELPERS-$(CONFIG_DARWIN) = qemu-bridge-helper$(EXESUF) ifdef BUILD_DOCS DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8 @@ -429,7 +432,8 @@ dummy := $(call unnest-vars,, \ ui-obj-m \ audio-obj-y \ audio-obj-m \ -trace-obj-y) +trace-obj-y \ +tap-obj-y) include $(SRC_PATH)/tests/Makefile.include @@ -535,7 +539,7 @@ qemu-img$(EXESUF): qemu-img.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-o qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj-y) $(COMMON_LDADDS) -qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS) +qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(tap-obj-y) $(COMMON_LDADDS) qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS) diff --git a/Makefile.objs b/Makefile.objs index c6c9b8fc21..1f9ced33bf 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -86,6 +86,11 @@ qom-obj-y = qom/ io-obj-y = io/ +### +# tap-obj-y is code used by qemu-bridge-helper + +tap-obj-y = net/ + ## # Target independent part of system emulation. The long term path is to # suppress *all* target specific code in case of system emulation, i.e. a diff --git a/configure b/configure index a2301dd0dc..34b6c398c7 100755 --- a/configure +++ b/configure @@ -728,6 +728,7 @@ GNU/kFreeBSD) ;; FreeBSD) bsd="yes" + freebsd="yes" make="${MAKE-gmake}" audio_drv_list="oss" audio_possible_drivers="oss sdl pa" @@ -5985,6 +5986,10 @@ if test "$linux" = "yes" ; then echo "CONFIG_LINUX=y" >> $config_host_mak fi +if test "$freebsd" = "yes" ; then + echo "CONFIG_FREEBSD=y" >> $config_host_mak +fi + if test "$darwin" = "yes" ; then echo "CONFIG_DARWIN=y" >> $config_host_mak fi diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c index 5396fbfbb6..2ed1b5503f 100644 --- a/qemu-bridge-helper.c +++ b/qemu-bridge-helper.c @@ -15,23 +15,47 @@ #include "qemu/osdep.h" - #include #include #include -#include - #include +#if defined(__linux__) +#include #include +#include "net/tap-linux.h" +#elif defined(__FreeBSD__) || defined(__APPLE__) +#include +#endif -#ifndef SIOCBRADDIF +#if
[Qemu-devel] [PATCH v2 for-2.12] tap: set vhostfd passed from qemu cli to non-blocking
A guest boot hangs while probing the network interface when iommu_platform=on is used. The following qemu cli hangs without this patch: # $QEMU \ -netdev tap,fd=3,id=hostnet0,vhost=on,vhostfd=4 3<>/dev/tap67 4<>/dev/host-net \ -device virtio-net-pci,netdev=hostnet0,id=net0,iommu_platform=on,disable-legacy=on \ ... Commit: c471ad0e9bd46 (vhost_net: device IOTLB support) took care of setting vhostfd to non-blocking when QEMU opens /dev/host-net but if the fd is passed from qemu cli then we need to ensure that fd is set to non-blocking. Fixes: c471ad0e9bd46 "vhost_net: device IOTLB support" Cc: Michael S. TsirkinCc: Jason Wang Signed-off-by: Brijesh Singh --- Changes since v1: - use qemu_set_nonblock() instead of fcntl(..) net/tap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/tap.c b/net/tap.c index 2b3a36f9b50d..89c4e19162a2 100644 --- a/net/tap.c +++ b/net/tap.c @@ -40,6 +40,7 @@ #include "qemu-common.h" #include "qemu/cutils.h" #include "qemu/error-report.h" +#include "qemu/sockets.h" #include "net/tap.h" @@ -693,6 +694,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer, } return; } +qemu_set_nonblock(vhostfd); } else { vhostfd = open("/dev/vhost-net", O_RDWR); if (vhostfd < 0) { -- 2.14.3
Re: [Qemu-devel] [PATCH for-2.12] vfio-ccw: fix memory leak in vfio_ccw_realize()
On Fri, 6 Apr 2018 18:54:13 +0200 Cornelia Huckwrote: > On Fri, 06 Apr 2018 17:14:19 +0200 > Greg Kurz wrote: > > > The vfio_ccw_realize() function currently leaks vcdev->vdev.name if > > the subchannel is already attached or if vfio_get_device() fails. > > > > This happens because vcdev->vdev.name is expected to be freed in > > vfio_put_device() which isn't called in this case. > > > > Adding g_free(vcdev->vdev.name) on these two error paths would be > > enough to fix the leak, but this is unfortunate since we would then > > have three locations doing this. Also, this would be inconsistent > > with the label based rollback scheme of this function. > > This is all a bit awkward :( > > But I'd actually prefer adding the two g_free calls for now, as that > would be a nice, small patch that my brain can still process at this > point in the evening :) > > Your patch would be a nice cleanup for 2.13, though. > > Alternatively, I could be convinced to queue this if I get a R-b from > someone :) > I guess I should rather split the patch in two: a bug fix with the two g_free calls for 2.12 and stable, and a cleanup patch for 2.13. > > > > The root issue is that vcdev->vdev.name is set before vfio_get_device() > > is called, which theoretically prevents to call vfio_put_device() to > > do the freeing. Well actually, we could call it anyway because > > vfio_put_base_device() is a nop if the device isn't attached, but this > > would be confusing. > > > > This patch hence moves all the logic of attaching the device to a > > separate vfio_ccw_get_device() function, which is the counterpart > > of vfio_put_device(). While here, vfio_put_device() is renamed to > > vfio_ccw_put_device() for consistency. > > > > Signed-off-by: Greg Kurz > > --- > > hw/vfio/ccw.c | 54 -- > > 1 file changed, 36 insertions(+), 18 deletions(-) > > > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > > index 4e5855741a64..49ae986d288d 100644 > > --- a/hw/vfio/ccw.c > > +++ b/hw/vfio/ccw.c > > @@ -292,12 +292,44 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev) > > g_free(vcdev->io_region); > > } > > > > -static void vfio_put_device(VFIOCCWDevice *vcdev) > > +static void vfio_ccw_put_device(VFIOCCWDevice *vcdev) > > { > > g_free(vcdev->vdev.name); > > vfio_put_base_device(>vdev); > > } > > > > +static int vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev, > > + Error **errp) > > +{ > > +char *name = g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid, > > + vcdev->cdev.hostid.ssid, > > + vcdev->cdev.hostid.devid); > > +VFIODevice *vbasedev; > > + > > +QLIST_FOREACH(vbasedev, >device_list, next) { > > +if (strcmp(vbasedev->name, name) == 0) { > > +error_setg(errp, "vfio: subchannel %s has already been > > attached", > > + name); > > +goto out_err; > > +} > > +} > > + > > +if (vfio_get_device(group, vcdev->cdev.mdevid, >vdev, errp)) { > > +goto out_err; > > +} > > + > > +vcdev->vdev.ops = _ccw_ops; > > +vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; > > +vcdev->vdev.name = name; > > +vcdev->vdev.dev = >cdev.parent_obj.parent_obj; > > + > > +return 0; > > + > > +out_err: > > +g_free(name); > > +return -1; > > +} > > + > > static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) > > { > > char *tmp, group_path[PATH_MAX]; > > @@ -327,7 +359,6 @@ static VFIOGroup *vfio_ccw_get_group(S390CCWDevice > > *cdev, Error **errp) > > > > static void vfio_ccw_realize(DeviceState *dev, Error **errp) > > { > > -VFIODevice *vbasedev; > > VFIOGroup *group; > > CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev); > > S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev); > > @@ -348,20 +379,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error > > **errp) > > goto out_group_err; > > } > > > > -vcdev->vdev.ops = _ccw_ops; > > -vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; > > -vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid, > > - cdev->hostid.ssid, > > cdev->hostid.devid); > > -vcdev->vdev.dev = dev; > > -QLIST_FOREACH(vbasedev, >device_list, next) { > > -if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) { > > -error_setg(, "vfio: subchannel %s has already been > > attached", > > - vcdev->vdev.name); > > -goto out_device_err; > > -} > > -} > > - > > -if (vfio_get_device(group, cdev->mdevid, >vdev, )) { > > +if (vfio_ccw_get_device(group, vcdev, )) { > > goto out_device_err; > > } > > > > @@ -380,7 +398,7 @@ static void
[Qemu-devel] [PATCH v2 07/17] target/mips: convert to DisasContextBase
Reviewed-by: Philippe Mathieu-DaudéCc: Aurelien Jarno Cc: Yongbok Kim Signed-off-by: Emilio G. Cota --- target/mips/translate.c | 346 1 file changed, 175 insertions(+), 171 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index a133205..aefd729 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -1430,17 +1430,15 @@ static TCGv_i64 msa_wr_d[64]; } while(0) typedef struct DisasContext { -struct TranslationBlock *tb; -target_ulong pc, saved_pc; +DisasContextBase base; +target_ulong saved_pc; uint32_t opcode; -int singlestep_enabled; int insn_flags; int32_t CP0_Config1; /* Routine used to access memory */ int mem_idx; TCGMemOp default_tcg_memop_mask; uint32_t hflags, saved_hflags; -DisasJumpType is_jmp; target_ulong btarget; bool ulri; int kscrexist; @@ -1517,8 +1515,9 @@ static const char * const msaregnames[] = { if (MIPS_DEBUG_DISAS) { \ qemu_log_mask(CPU_LOG_TB_IN_ASM, \ TARGET_FMT_lx ": %08x Invalid %s %03x %03x %03x\n", \ - ctx->pc, ctx->opcode, op, ctx->opcode >> 26,\ - ctx->opcode & 0x3F, ((ctx->opcode >> 16) & 0x1F)); \ + ctx->base.pc_next, ctx->opcode, op, \ + ctx->opcode >> 26, ctx->opcode & 0x3F, \ + ((ctx->opcode >> 16) & 0x1F)); \ } \ } while (0) @@ -1594,9 +1593,9 @@ static inline void gen_save_pc(target_ulong pc) static inline void save_cpu_state(DisasContext *ctx, int do_save_pc) { LOG_DISAS("hflags %08x saved %08x\n", ctx->hflags, ctx->saved_hflags); -if (do_save_pc && ctx->pc != ctx->saved_pc) { -gen_save_pc(ctx->pc); -ctx->saved_pc = ctx->pc; +if (do_save_pc && ctx->base.pc_next != ctx->saved_pc) { +gen_save_pc(ctx->base.pc_next); +ctx->saved_pc = ctx->base.pc_next; } if (ctx->hflags != ctx->saved_hflags) { tcg_gen_movi_i32(hflags, ctx->hflags); @@ -1635,7 +1634,7 @@ static inline void generate_exception_err(DisasContext *ctx, int excp, int err) gen_helper_raise_exception_err(cpu_env, texcp, terr); tcg_temp_free_i32(terr); tcg_temp_free_i32(texcp); -ctx->is_jmp = DISAS_EXCP; +ctx->base.is_jmp = DISAS_EXCP; } static inline void generate_exception(DisasContext *ctx, int excp) @@ -2126,7 +2125,7 @@ static void gen_base_offset_addr (DisasContext *ctx, TCGv addr, static target_ulong pc_relative_pc (DisasContext *ctx) { -target_ulong pc = ctx->pc; +target_ulong pc = ctx->base.pc_next; if (ctx->hflags & MIPS_HFLAG_BMASK) { int branch_bytes = ctx->hflags & MIPS_HFLAG_BDS16 ? 2 : 4; @@ -4275,12 +4274,12 @@ static void gen_trap (DisasContext *ctx, uint32_t opc, static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest) { -if (unlikely(ctx->singlestep_enabled)) { +if (unlikely(ctx->base.singlestep_enabled)) { return false; } #ifndef CONFIG_USER_ONLY -return (ctx->tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); +return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); #else return true; #endif @@ -4291,10 +4290,10 @@ static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) if (use_goto_tb(ctx, dest)) { tcg_gen_goto_tb(n); gen_save_pc(dest); -tcg_gen_exit_tb((uintptr_t)ctx->tb + n); +tcg_gen_exit_tb((uintptr_t)ctx->base.tb + n); } else { gen_save_pc(dest); -if (ctx->singlestep_enabled) { +if (ctx->base.singlestep_enabled) { save_cpu_state(ctx, 0); gen_helper_raise_exception_debug(cpu_env); } @@ -4317,7 +4316,7 @@ static void gen_compute_branch (DisasContext *ctx, uint32_t opc, if (ctx->hflags & MIPS_HFLAG_BMASK) { #ifdef MIPS_DEBUG_DISAS LOG_DISAS("Branch in delay / forbidden slot at PC 0x" - TARGET_FMT_lx "\n", ctx->pc); + TARGET_FMT_lx "\n", ctx->base.pc_next); #endif generate_exception_end(ctx, EXCP_RI); goto out; @@ -4335,7 +4334,7 @@ static void gen_compute_branch (DisasContext *ctx, uint32_t opc, gen_load_gpr(t1, rt); bcond_compute = 1; } -btgt = ctx->pc + insn_bytes + offset; +btgt = ctx->base.pc_next + insn_bytes + offset; break; case OPC_BGEZ: case OPC_BGEZAL: @@ -4354,7 +4353,7 @@ static void gen_compute_branch (DisasContext *ctx, uint32_t opc, gen_load_gpr(t0, rs);
Re: [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file
On 04/06/18 20:10, Eric Blake wrote: > On 04/06/2018 12:28 PM, Laszlo Ersek wrote: > >> I've created an RFC-level "qapi/firmware.json" schema file, based on >> this discussion. It "builds", and the generated documentation looks >> acceptable, superficially speaking. >> >> Before I post "qapi/firmware.json" for getting comments, I'd like to >> write JSON text that (a) describes firmware that I use, and (b) >> conforms to the schema. IOW, I'd like to validate whether the schema >> is good enough for describing at least such firmware that I know. >> >> Is there a tool that generates example JSON objects from a given >> schema? > > I know the QMP shell (scripts/qmp/qmp-shell) lets you enter commands > with a lot less typing than full JSON, and has a mode where it will > then echo the full JSON command it constructed from what you typed. To > be able to quickly validate examples, it may be sufficient to > temporarily add a new QMP command 'check-firmware': > > { 'command': 'check-firmware', 'boxed': true, 'data': 'Firmware' } > > assuming 'Firmware' is your top-level 'struct' in the QAPI file, then > implement a trivial: > > qmp_check_firmware(Firmware *obj, Error **errp) { > return 0; > } > > so that you can then run QMP shell, and type: > > check-firmware arg1=foo arg2=bar ... > > which will then generate the corresponding JSON, then either > successfully do nothing (what you typed validated, AND you have the > JSON output printed), or print an error (what you typed failed QAPI > validation, perhaps because it had an unrecognized key, was missing a > required key, used a wrong type, etc). > >> I vaguely recall there used to be one. Otherwise, writing the >> examples manually looks arduous (and I wouldn't know how to verify >> them against the schema). > > Similarly, if you generate a command the produces a 'Firmware' as the > return value, then you can populate the generated C struct (since you > did manage to run the QAPI generator over your new file, you should be > able to look at the C struct it generated), then output that over QMP > to show the counterpart JSON that matches the struct as populated. > The top level structure is complex / nested, but that doesn't appear to be an issue. According to the script, # key=value pairs also support Python or JSON object literal subset notations, # without spaces. Dictionaries/objects {} are supported as are arrays []. # #example-command arg-name1={'key':'value','obj'={'prop':"value"}} # # Both JSON and Python formatting should work, including both styles of # string literal quotes. Both paradigms of literal values should work, # including null/true/false for JSON and None/True/False for Python. This looks awesome, because it should let me provide messy nested input (which I'll obviously compose in my $EDITOR and then paste it), and then the QMP shell will both validate and pretty print that. I'm going to try this. Thank you, Eric! Laszlo
[Qemu-devel] [PATCH v2 11/17] target/s390x: convert to DisasContextBase
Notes: - Did not convert {num,max}_insns and is_jmp, since the corresponding code will go away in the next patch. - Avoided a checkpatch error in use_exit_tb. - As suggested by David, (1) Drop ctx.pc and use ctx.base.pc_next instead, and (2) Rename ctx.next_pc to ctx.pc_tmp and add a comment about it. Acked-by: Cornelia HuckSuggested-by: David Hildenbrand Reviewed-by: David Hildenbrand Cc: David Hildenbrand Cc: Cornelia Huck Cc: Alexander Graf Cc: qemu-s3...@nongnu.org Signed-off-by: Emilio G. Cota --- target/s390x/translate.c | 148 --- 1 file changed, 76 insertions(+), 72 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 3156286..a65e9cd 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -52,14 +52,18 @@ typedef struct DisasInsn DisasInsn; typedef struct DisasFields DisasFields; struct DisasContext { -struct TranslationBlock *tb; +DisasContextBase base; const DisasInsn *insn; DisasFields *fields; uint64_t ex_value; -uint64_t pc, next_pc; +/* + * During translate_one(), pc_tmp is used to determine the instruction + * to be executed after base.pc_next - e.g. next sequential instruction + * or a branch target. + */ +uint64_t pc_tmp; uint32_t ilen; enum cc_op cc_op; -bool singlestep_enabled; }; /* Information carried about a condition to be evaluated. */ @@ -81,8 +85,8 @@ static uint64_t inline_branch_miss[CC_OP_MAX]; static uint64_t pc_to_link_info(DisasContext *s, uint64_t pc) { -if (!(s->tb->flags & FLAG_MASK_64)) { -if (s->tb->flags & FLAG_MASK_32) { +if (!(s->base.tb->flags & FLAG_MASK_64)) { +if (s->base.tb->flags & FLAG_MASK_32) { return pc | 0x8000; } } @@ -188,16 +192,16 @@ static void return_low128(TCGv_i64 dest) static void update_psw_addr(DisasContext *s) { /* psw.addr */ -tcg_gen_movi_i64(psw_addr, s->pc); +tcg_gen_movi_i64(psw_addr, s->base.pc_next); } static void per_branch(DisasContext *s, bool to_next) { #ifndef CONFIG_USER_ONLY -tcg_gen_movi_i64(gbea, s->pc); +tcg_gen_movi_i64(gbea, s->base.pc_next); -if (s->tb->flags & FLAG_MASK_PER) { -TCGv_i64 next_pc = to_next ? tcg_const_i64(s->next_pc) : psw_addr; +if (s->base.tb->flags & FLAG_MASK_PER) { +TCGv_i64 next_pc = to_next ? tcg_const_i64(s->pc_tmp) : psw_addr; gen_helper_per_branch(cpu_env, gbea, next_pc); if (to_next) { tcg_temp_free_i64(next_pc); @@ -210,16 +214,16 @@ static void per_branch_cond(DisasContext *s, TCGCond cond, TCGv_i64 arg1, TCGv_i64 arg2) { #ifndef CONFIG_USER_ONLY -if (s->tb->flags & FLAG_MASK_PER) { +if (s->base.tb->flags & FLAG_MASK_PER) { TCGLabel *lab = gen_new_label(); tcg_gen_brcond_i64(tcg_invert_cond(cond), arg1, arg2, lab); -tcg_gen_movi_i64(gbea, s->pc); +tcg_gen_movi_i64(gbea, s->base.pc_next); gen_helper_per_branch(cpu_env, gbea, psw_addr); gen_set_label(lab); } else { -TCGv_i64 pc = tcg_const_i64(s->pc); +TCGv_i64 pc = tcg_const_i64(s->base.pc_next); tcg_gen_movcond_i64(cond, gbea, arg1, arg2, gbea, pc); tcg_temp_free_i64(pc); } @@ -228,7 +232,7 @@ static void per_branch_cond(DisasContext *s, TCGCond cond, static void per_breaking_event(DisasContext *s) { -tcg_gen_movi_i64(gbea, s->pc); +tcg_gen_movi_i64(gbea, s->base.pc_next); } static void update_cc_op(DisasContext *s) @@ -250,11 +254,11 @@ static inline uint64_t ld_code4(CPUS390XState *env, uint64_t pc) static int get_mem_index(DisasContext *s) { -if (!(s->tb->flags & FLAG_MASK_DAT)) { +if (!(s->base.tb->flags & FLAG_MASK_DAT)) { return MMU_REAL_IDX; } -switch (s->tb->flags & FLAG_MASK_ASC) { +switch (s->base.tb->flags & FLAG_MASK_ASC) { case PSW_ASC_PRIMARY >> FLAG_MASK_PSW_SHIFT: return MMU_PRIMARY_IDX; case PSW_ASC_SECONDARY >> FLAG_MASK_PSW_SHIFT: @@ -319,7 +323,7 @@ static inline void gen_trap(DisasContext *s) #ifndef CONFIG_USER_ONLY static void check_privileged(DisasContext *s) { -if (s->tb->flags & FLAG_MASK_PSTATE) { +if (s->base.tb->flags & FLAG_MASK_PSTATE) { gen_program_exception(s, PGM_PRIVILEGED); } } @@ -328,7 +332,7 @@ static void check_privileged(DisasContext *s) static TCGv_i64 get_address(DisasContext *s, int x2, int b2, int d2) { TCGv_i64 tmp = tcg_temp_new_i64(); -bool need_31 = !(s->tb->flags & FLAG_MASK_64); +bool need_31 = !(s->base.tb->flags & FLAG_MASK_64); /* Note that d2 is limited to 20 bits, signed. If we crop negative displacements early we create larger immedate addends. */ @@ -541,9 +545,9 @@ static void
[Qemu-devel] [PATCH v2 10/17] target/s390x: convert to DisasJumpType
The only non-trivial modification is the use of DISAS_TOO_MANY in the same way is used by the generic translation loop. Acked-by: Cornelia HuckReviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Cc: David Hildenbrand Cc: Cornelia Huck Cc: Alexander Graf Cc: qemu-s3...@nongnu.org Signed-off-by: Emilio G. Cota --- target/s390x/translate.c | 1267 +++--- 1 file changed, 632 insertions(+), 635 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index 7d39ab3..3156286 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -42,6 +42,7 @@ #include "exec/helper-gen.h" #include "trace-tcg.h" +#include "exec/translator.h" #include "exec/log.h" @@ -73,9 +74,6 @@ typedef struct { } u; } DisasCompare; -/* is_jmp field values */ -#define DISAS_EXCP DISAS_TARGET_0 - #ifdef DEBUG_INLINE_BRANCHES static uint64_t inline_branch_hit[CC_OP_MAX]; static uint64_t inline_branch_miss[CC_OP_MAX]; @@ -1091,26 +1089,24 @@ typedef struct { #define SPEC_r2_f12816 /* Return values from translate_one, indicating the state of the TB. */ -typedef enum { -/* Continue the TB. */ -NO_EXIT, -/* We have emitted one or more goto_tb. No fixup required. */ -EXIT_GOTO_TB, -/* We are not using a goto_tb (for whatever reason), but have updated - the PC (for whatever reason), so there's no need to do it again on - exiting the TB. */ -EXIT_PC_UPDATED, -/* We have updated the PC and CC values. */ -EXIT_PC_CC_UPDATED, -/* We are exiting the TB, but have neither emitted a goto_tb, nor - updated the PC for the next instruction to be executed. */ -EXIT_PC_STALE, -/* We are exiting the TB to the main loop. */ -EXIT_PC_STALE_NOCHAIN, -/* We are ending the TB with a noreturn function call, e.g. longjmp. - No following code will be executed. */ -EXIT_NORETURN, -} ExitStatus; + +/* We are not using a goto_tb (for whatever reason), but have updated + the PC (for whatever reason), so there's no need to do it again on + exiting the TB. */ +#define DISAS_PC_UPDATEDDISAS_TARGET_0 + +/* We have emitted one or more goto_tb. No fixup required. */ +#define DISAS_GOTO_TB DISAS_TARGET_1 + +/* We have updated the PC and CC values. */ +#define DISAS_PC_CC_UPDATED DISAS_TARGET_2 + +/* We are exiting the TB, but have neither emitted a goto_tb, nor + updated the PC for the next instruction to be executed. */ +#define DISAS_PC_STALE DISAS_TARGET_3 + +/* We are exiting the TB to the main loop. */ +#define DISAS_PC_STALE_NOCHAIN DISAS_TARGET_4 struct DisasInsn { unsigned opc:16; @@ -1125,7 +1121,7 @@ struct DisasInsn { void (*help_prep)(DisasContext *, DisasFields *, DisasOps *); void (*help_wout)(DisasContext *, DisasFields *, DisasOps *); void (*help_cout)(DisasContext *, DisasOps *); -ExitStatus (*help_op)(DisasContext *, DisasOps *); +DisasJumpType (*help_op)(DisasContext *, DisasOps *); uint64_t data; }; @@ -1147,11 +1143,11 @@ static void help_l2_shift(DisasContext *s, DisasFields *f, } } -static ExitStatus help_goto_direct(DisasContext *s, uint64_t dest) +static DisasJumpType help_goto_direct(DisasContext *s, uint64_t dest) { if (dest == s->next_pc) { per_branch(s, true); -return NO_EXIT; +return DISAS_NEXT; } if (use_goto_tb(s, dest)) { update_cc_op(s); @@ -1159,31 +1155,31 @@ static ExitStatus help_goto_direct(DisasContext *s, uint64_t dest) tcg_gen_goto_tb(0); tcg_gen_movi_i64(psw_addr, dest); tcg_gen_exit_tb((uintptr_t)s->tb); -return EXIT_GOTO_TB; +return DISAS_GOTO_TB; } else { tcg_gen_movi_i64(psw_addr, dest); per_branch(s, false); -return EXIT_PC_UPDATED; +return DISAS_PC_UPDATED; } } -static ExitStatus help_branch(DisasContext *s, DisasCompare *c, - bool is_imm, int imm, TCGv_i64 cdest) +static DisasJumpType help_branch(DisasContext *s, DisasCompare *c, + bool is_imm, int imm, TCGv_i64 cdest) { -ExitStatus ret; +DisasJumpType ret; uint64_t dest = s->pc + 2 * imm; TCGLabel *lab; /* Take care of the special cases first. */ if (c->cond == TCG_COND_NEVER) { -ret = NO_EXIT; +ret = DISAS_NEXT; goto egress; } if (is_imm) { if (dest == s->next_pc) { /* Branch to next. */ per_branch(s, true); -ret = NO_EXIT; +ret = DISAS_NEXT; goto egress; } if (c->cond == TCG_COND_ALWAYS) { @@ -1193,13 +1189,13 @@ static ExitStatus help_branch(DisasContext *s, DisasCompare *c, } else {
[Qemu-devel] [PATCH v2 06/17] target/mips: convert to DisasJumpType
Reviewed-by: Philippe Mathieu-DaudéCc: Aurelien Jarno Cc: Yongbok Kim Signed-off-by: Emilio G. Cota --- target/mips/translate.c | 186 +++- 1 file changed, 91 insertions(+), 95 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index d05ee67..a133205 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -36,6 +36,7 @@ #include "target/mips/trace.h" #include "trace-tcg.h" +#include "exec/translator.h" #include "exec/log.h" #define MIPS_DEBUG_DISAS 0 @@ -1439,7 +1440,7 @@ typedef struct DisasContext { int mem_idx; TCGMemOp default_tcg_memop_mask; uint32_t hflags, saved_hflags; -int bstate; +DisasJumpType is_jmp; target_ulong btarget; bool ulri; int kscrexist; @@ -1460,13 +1461,8 @@ typedef struct DisasContext { bool abs2008; } DisasContext; -enum { -BS_NONE = 0, /* We go out of the TB without reaching a branch or an - * exception condition */ -BS_STOP = 1, /* We want to stop translation for any reason */ -BS_BRANCH = 2, /* We reached a branch condition */ -BS_EXCP = 3, /* We reached an exception condition */ -}; +#define DISAS_STOP DISAS_TARGET_0 +#define DISAS_EXCP DISAS_TARGET_1 static const char * const regnames[] = { "r0", "at", "v0", "v1", "a0", "a1", "a2", "a3", @@ -1639,7 +1635,7 @@ static inline void generate_exception_err(DisasContext *ctx, int excp, int err) gen_helper_raise_exception_err(cpu_env, texcp, terr); tcg_temp_free_i32(terr); tcg_temp_free_i32(texcp); -ctx->bstate = BS_EXCP; +ctx->is_jmp = DISAS_EXCP; } static inline void generate_exception(DisasContext *ctx, int excp) @@ -5334,10 +5330,10 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_io_end(); } /* Break the TB to be able to take timer interrupts immediately - after reading count. BS_STOP isn't sufficient, we need to ensure - we break completely out of translated code. */ + after reading count. DISAS_STOP isn't sufficient, we need to + ensure we break completely out of translated code. */ gen_save_pc(ctx->pc + 4); -ctx->bstate = BS_EXCP; +ctx->is_jmp = DISAS_EXCP; rn = "Count"; break; /* 6,7 are implementation dependent */ @@ -5905,7 +5901,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) check_insn(ctx, ISA_MIPS32R2); gen_helper_mtc0_pagegrain(cpu_env, arg); rn = "PageGrain"; -ctx->bstate = BS_STOP; +ctx->is_jmp = DISAS_STOP; break; case 2: CP0_CHECK(ctx->sc); @@ -5966,7 +5962,7 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) case 0: check_insn(ctx, ISA_MIPS32R2); gen_helper_mtc0_hwrena(cpu_env, arg); -ctx->bstate = BS_STOP; +ctx->is_jmp = DISAS_STOP; rn = "HWREna"; break; default: @@ -6028,30 +6024,30 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) case 0: save_cpu_state(ctx, 1); gen_helper_mtc0_status(cpu_env, arg); -/* BS_STOP isn't good enough here, hflags may have changed. */ +/* DISAS_STOP isn't good enough here, hflags may have changed. */ gen_save_pc(ctx->pc + 4); -ctx->bstate = BS_EXCP; +ctx->is_jmp = DISAS_EXCP; rn = "Status"; break; case 1: check_insn(ctx, ISA_MIPS32R2); gen_helper_mtc0_intctl(cpu_env, arg); /* Stop translation as we may have switched the execution mode */ -ctx->bstate = BS_STOP; +ctx->is_jmp = DISAS_STOP; rn = "IntCtl"; break; case 2: check_insn(ctx, ISA_MIPS32R2); gen_helper_mtc0_srsctl(cpu_env, arg); /* Stop translation as we may have switched the execution mode */ -ctx->bstate = BS_STOP; +ctx->is_jmp = DISAS_STOP; rn = "SRSCtl"; break; case 3: check_insn(ctx, ISA_MIPS32R2); gen_mtc0_store32(arg, offsetof(CPUMIPSState, CP0_SRSMap)); /* Stop translation as we may have switched the execution mode */ -ctx->bstate = BS_STOP; +ctx->is_jmp = DISAS_STOP; rn = "SRSMap"; break; default: @@ -6063,11 +6059,11 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) case 0: save_cpu_state(ctx, 1); gen_helper_mtc0_cause(cpu_env, arg); -/* Stop translation as we
[Qemu-devel] [PATCH v2 13/17] target/openrisc: convert to DisasContextBase
While at it, set is_jmp to DISAS_NORETURN when generating an exception. Cc: Stafford HorneSigned-off-by: Emilio G. Cota --- target/openrisc/translate.c | 93 ++--- 1 file changed, 46 insertions(+), 47 deletions(-) diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index 2747b24..b37414f 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -36,7 +36,8 @@ #include "exec/log.h" #define LOG_DIS(str, ...) \ -qemu_log_mask(CPU_LOG_TB_IN_ASM, "%08x: " str, dc->pc, ## __VA_ARGS__) +qemu_log_mask(CPU_LOG_TB_IN_ASM, "%08x: " str, dc->base.pc_next,\ + ## __VA_ARGS__) /* is_jmp field values */ #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */ @@ -44,13 +45,10 @@ #define DISAS_TB_JUMP DISAS_TARGET_2 /* only pc was modified statically */ typedef struct DisasContext { -TranslationBlock *tb; -target_ulong pc; -uint32_t is_jmp; +DisasContextBase base; uint32_t mem_idx; uint32_t tb_flags; uint32_t delayed_branch; -bool singlestep_enabled; } DisasContext; static TCGv cpu_sr; @@ -126,9 +124,9 @@ static void gen_exception(DisasContext *dc, unsigned int excp) static void gen_illegal_exception(DisasContext *dc) { -tcg_gen_movi_tl(cpu_pc, dc->pc); +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next); gen_exception(dc, EXCP_ILLEGAL); -dc->is_jmp = DISAS_UPDATE; +dc->base.is_jmp = DISAS_NORETURN; } /* not used yet, open it when we need or64. */ @@ -166,12 +164,12 @@ static void check_ov64s(DisasContext *dc) static inline bool use_goto_tb(DisasContext *dc, target_ulong dest) { -if (unlikely(dc->singlestep_enabled)) { +if (unlikely(dc->base.singlestep_enabled)) { return false; } #ifndef CONFIG_USER_ONLY -return (dc->tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); +return (dc->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); #else return true; #endif @@ -182,10 +180,10 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest) if (use_goto_tb(dc, dest)) { tcg_gen_movi_tl(cpu_pc, dest); tcg_gen_goto_tb(n); -tcg_gen_exit_tb((uintptr_t)dc->tb + n); +tcg_gen_exit_tb((uintptr_t)dc->base.tb + n); } else { tcg_gen_movi_tl(cpu_pc, dest); -if (dc->singlestep_enabled) { +if (dc->base.singlestep_enabled) { gen_exception(dc, EXCP_DEBUG); } tcg_gen_exit_tb(0); @@ -194,16 +192,16 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest) static void gen_jump(DisasContext *dc, int32_t n26, uint32_t reg, uint32_t op0) { -target_ulong tmp_pc = dc->pc + n26 * 4; +target_ulong tmp_pc = dc->base.pc_next + n26 * 4; switch (op0) { case 0x00: /* l.j */ tcg_gen_movi_tl(jmp_pc, tmp_pc); break; case 0x01: /* l.jal */ -tcg_gen_movi_tl(cpu_R[9], dc->pc + 8); +tcg_gen_movi_tl(cpu_R[9], dc->base.pc_next + 8); /* Optimize jal being used to load the PC for PIC. */ -if (tmp_pc == dc->pc + 8) { +if (tmp_pc == dc->base.pc_next + 8) { return; } tcg_gen_movi_tl(jmp_pc, tmp_pc); @@ -211,7 +209,7 @@ static void gen_jump(DisasContext *dc, int32_t n26, uint32_t reg, uint32_t op0) case 0x03: /* l.bnf */ case 0x04: /* l.bf */ { -TCGv t_next = tcg_const_tl(dc->pc + 8); +TCGv t_next = tcg_const_tl(dc->base.pc_next + 8); TCGv t_true = tcg_const_tl(tmp_pc); TCGv t_zero = tcg_const_tl(0); @@ -227,7 +225,7 @@ static void gen_jump(DisasContext *dc, int32_t n26, uint32_t reg, uint32_t op0) tcg_gen_mov_tl(jmp_pc, cpu_R[reg]); break; case 0x12: /* l.jalr */ -tcg_gen_movi_tl(cpu_R[9], (dc->pc + 8)); +tcg_gen_movi_tl(cpu_R[9], (dc->base.pc_next + 8)); tcg_gen_mov_tl(jmp_pc, cpu_R[reg]); break; default: @@ -795,7 +793,7 @@ static void dec_misc(DisasContext *dc, uint32_t insn) return; } gen_helper_rfe(cpu_env); -dc->is_jmp = DISAS_UPDATE; +dc->base.is_jmp = DISAS_UPDATE; #endif } break; @@ -1254,15 +1252,16 @@ static void dec_sys(DisasContext *dc, uint32_t insn) switch (op0) { case 0x000:/* l.sys */ LOG_DIS("l.sys %d\n", K16); -tcg_gen_movi_tl(cpu_pc, dc->pc); +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next); gen_exception(dc, EXCP_SYSCALL); -dc->is_jmp = DISAS_UPDATE; +dc->base.is_jmp = DISAS_NORETURN; break; case 0x100:/* l.trap */ LOG_DIS("l.trap %d\n", K16); -tcg_gen_movi_tl(cpu_pc, dc->pc); +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next); gen_exception(dc, EXCP_TRAP); +
[Qemu-devel] [PATCH v2 12/17] target/s390x: convert to TranslatorOps
Note: I looked into dropping dc->do_debug. However, I don't see an easy way to do it given that TOO_MANY is also valid when we just translate more than max_insns. Thus, the check for do_debug in "case DISAS_PC_CC_UPDATED" would still need additional state to know whether or not we came from breakpoint_check. Acked-by: Cornelia HuckReviewed-by: David Hildenbrand Tested-by: David Hildenbrand Cc: David Hildenbrand Cc: Cornelia Huck Cc: Alexander Graf Cc: qemu-s3...@nongnu.org Signed-off-by: Emilio G. Cota --- target/s390x/translate.c | 162 +++ 1 file changed, 80 insertions(+), 82 deletions(-) diff --git a/target/s390x/translate.c b/target/s390x/translate.c index a65e9cd..2cbd870 100644 --- a/target/s390x/translate.c +++ b/target/s390x/translate.c @@ -64,6 +64,7 @@ struct DisasContext { uint64_t pc_tmp; uint32_t ilen; enum cc_op cc_op; +bool do_debug; }; /* Information carried about a condition to be evaluated. */ @@ -6158,98 +6159,87 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) return ret; } -void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) +static void s390x_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) { -CPUS390XState *env = cs->env_ptr; -DisasContext dc; -uint64_t next_page_start; -int num_insns, max_insns; -DisasJumpType status; -bool do_debug; +DisasContext *dc = container_of(dcbase, DisasContext, base); -dc.base.pc_first = tb->pc; /* 31-bit mode */ -if (!(tb->flags & FLAG_MASK_64)) { -dc.base.pc_first &= 0x7fff; +if (!(dc->base.tb->flags & FLAG_MASK_64)) { +dc->base.pc_first &= 0x7fff; +dc->base.pc_next = dc->base.pc_first; } -dc.base.pc_next = dc.base.pc_first; -dc.base.tb = tb; -dc.base.singlestep_enabled = cs->singlestep_enabled; -dc.cc_op = CC_OP_DYNAMIC; -dc.ex_value = dc.base.tb->cs_base; -do_debug = cs->singlestep_enabled; +dc->cc_op = CC_OP_DYNAMIC; +dc->ex_value = dc->base.tb->cs_base; +dc->do_debug = dc->base.singlestep_enabled; +} -next_page_start = (dc.base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; +static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs) +{ +} -num_insns = 0; -max_insns = tb_cflags(tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} +static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); -gen_tb_start(tb); +tcg_gen_insn_start(dc->base.pc_next, dc->cc_op); +} -do { -tcg_gen_insn_start(dc.base.pc_next, dc.cc_op); -num_insns++; +static bool s390x_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, + const CPUBreakpoint *bp) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); -if (unlikely(cpu_breakpoint_test(cs, dc.base.pc_next, BP_ANY))) { -status = DISAS_PC_STALE; -do_debug = true; -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -dc.base.pc_next += 2; -break; -} +dc->base.is_jmp = DISAS_PC_STALE; +dc->do_debug = true; +/* The address covered by the breakpoint must be included in + [tb->pc, tb->pc + tb->size) in order to for it to be + properly cleared -- thus we increment the PC here so that + the logic setting tb->size does the right thing. */ +dc->base.pc_next += 2; +return true; +} -if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} +static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) +{ +CPUS390XState *env = cs->env_ptr; +DisasContext *dc = container_of(dcbase, DisasContext, base); -status = translate_one(env, ); - -/* If we reach a page boundary, are single stepping, - or exhaust instruction count, stop generation. */ -if (status == DISAS_NEXT -&& (dc.base.pc_next >= next_page_start -|| tcg_op_buf_full() -|| num_insns >= max_insns -|| singlestep -|| dc.base.singlestep_enabled -|| dc.ex_value)) { -status = DISAS_TOO_MANY; -} -} while (status == DISAS_NEXT); +dc->base.is_jmp = translate_one(env, dc); +if (dc->base.is_jmp == DISAS_NEXT) { +uint64_t
[Qemu-devel] [PATCH v2 08/17] target/mips: use *ctx for DisasContext
No changes to the logic here; this is just to make the diff that follows easier to read. While at it, remove the unnecessary 'struct' in 'struct TranslationBlock'. Note that checkpatch complains with a false positive: ERROR: space prohibited after that '&' (ctx:WxW) #75: FILE: target/mips/translate.c:20220: +ctx->kscrexist = (env->CP0_Config4 >> CP0C4_KScrExist) & 0xff; ^ Reviewed-by: Philippe Mathieu-DaudéCc: Aurelien Jarno Cc: Yongbok Kim Signed-off-by: Emilio G. Cota --- target/mips/translate.c | 166 1 file changed, 84 insertions(+), 82 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index aefd729..08bd140 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -20194,55 +20194,57 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } } -void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) +void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) { CPUMIPSState *env = cs->env_ptr; -DisasContext ctx; +DisasContext ctx1; +DisasContext *ctx = target_ulong next_page_start; int max_insns; int insn_bytes; int is_slot; -ctx.base.tb = tb; -ctx.base.pc_first = tb->pc; -ctx.base.pc_next = tb->pc; -ctx.base.is_jmp = DISAS_NEXT; -ctx.base.singlestep_enabled = cs->singlestep_enabled; -ctx.base.num_insns = 0; - -next_page_start = (ctx.base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; -ctx.saved_pc = -1; -ctx.insn_flags = env->insn_flags; -ctx.CP0_Config1 = env->CP0_Config1; -ctx.btarget = 0; -ctx.kscrexist = (env->CP0_Config4 >> CP0C4_KScrExist) & 0xff; -ctx.rxi = (env->CP0_Config3 >> CP0C3_RXI) & 1; -ctx.ie = (env->CP0_Config4 >> CP0C4_IE) & 3; -ctx.bi = (env->CP0_Config3 >> CP0C3_BI) & 1; -ctx.bp = (env->CP0_Config3 >> CP0C3_BP) & 1; -ctx.PAMask = env->PAMask; -ctx.mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1; -ctx.eva = (env->CP0_Config5 >> CP0C5_EVA) & 1; -ctx.sc = (env->CP0_Config3 >> CP0C3_SC) & 1; -ctx.CP0_LLAddr_shift = env->CP0_LLAddr_shift; -ctx.cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1; +ctx->base.tb = tb; +ctx->base.pc_first = tb->pc; +ctx->base.pc_next = tb->pc; +ctx->base.is_jmp = DISAS_NEXT; +ctx->base.singlestep_enabled = cs->singlestep_enabled; +ctx->base.num_insns = 0; + +next_page_start = (ctx->base.pc_first & TARGET_PAGE_MASK) + +TARGET_PAGE_SIZE; +ctx->saved_pc = -1; +ctx->insn_flags = env->insn_flags; +ctx->CP0_Config1 = env->CP0_Config1; +ctx->btarget = 0; +ctx->kscrexist = (env->CP0_Config4 >> CP0C4_KScrExist) & 0xff; +ctx->rxi = (env->CP0_Config3 >> CP0C3_RXI) & 1; +ctx->ie = (env->CP0_Config4 >> CP0C4_IE) & 3; +ctx->bi = (env->CP0_Config3 >> CP0C3_BI) & 1; +ctx->bp = (env->CP0_Config3 >> CP0C3_BP) & 1; +ctx->PAMask = env->PAMask; +ctx->mvh = (env->CP0_Config5 >> CP0C5_MVH) & 1; +ctx->eva = (env->CP0_Config5 >> CP0C5_EVA) & 1; +ctx->sc = (env->CP0_Config3 >> CP0C3_SC) & 1; +ctx->CP0_LLAddr_shift = env->CP0_LLAddr_shift; +ctx->cmgcr = (env->CP0_Config3 >> CP0C3_CMGCR) & 1; /* Restore delay slot state from the tb context. */ -ctx.hflags = (uint32_t)ctx.base.tb->flags; /* FIXME: maybe use 64 bits? */ -ctx.ulri = (env->CP0_Config3 >> CP0C3_ULRI) & 1; -ctx.ps = ((env->active_fpu.fcr0 >> FCR0_PS) & 1) || +ctx->hflags = (uint32_t)ctx->base.tb->flags; /* FIXME: maybe use 64 bits? */ +ctx->ulri = (env->CP0_Config3 >> CP0C3_ULRI) & 1; +ctx->ps = ((env->active_fpu.fcr0 >> FCR0_PS) & 1) || (env->insn_flags & (INSN_LOONGSON2E | INSN_LOONGSON2F)); -ctx.vp = (env->CP0_Config5 >> CP0C5_VP) & 1; -ctx.mrp = (env->CP0_Config5 >> CP0C5_MRP) & 1; -ctx.nan2008 = (env->active_fpu.fcr31 >> FCR31_NAN2008) & 1; -ctx.abs2008 = (env->active_fpu.fcr31 >> FCR31_ABS2008) & 1; -restore_cpu_state(env, ); +ctx->vp = (env->CP0_Config5 >> CP0C5_VP) & 1; +ctx->mrp = (env->CP0_Config5 >> CP0C5_MRP) & 1; +ctx->nan2008 = (env->active_fpu.fcr31 >> FCR31_NAN2008) & 1; +ctx->abs2008 = (env->active_fpu.fcr31 >> FCR31_ABS2008) & 1; +restore_cpu_state(env, ctx); #ifdef CONFIG_USER_ONLY -ctx.mem_idx = MIPS_HFLAG_UM; +ctx->mem_idx = MIPS_HFLAG_UM; #else -ctx.mem_idx = hflags_mmu_index(ctx.hflags); +ctx->mem_idx = hflags_mmu_index(ctx->hflags); #endif -ctx.default_tcg_memop_mask = (ctx.insn_flags & ISA_MIPS32R6) ? - MO_UNALN : MO_ALIGN; +ctx->default_tcg_memop_mask = (ctx->insn_flags & ISA_MIPS32R6) ? + MO_UNALN : MO_ALIGN; max_insns = tb_cflags(tb) & CF_COUNT_MASK; if (max_insns == 0) { max_insns = CF_COUNT_MASK; @@
[Qemu-devel] [PATCH v2 15/17] target/riscv: convert to DisasJumpType
Cc: Michael ClarkCc: Palmer Dabbelt Cc: Sagar Karandikar Cc: Bastian Koppelmann Signed-off-by: Emilio G. Cota --- target/riscv/translate.c | 72 +++- 1 file changed, 28 insertions(+), 44 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 808eab7..a5c25ab 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -26,6 +26,7 @@ #include "exec/helper-proto.h" #include "exec/helper-gen.h" +#include "exec/translator.h" #include "exec/log.h" #include "instmap.h" @@ -46,7 +47,7 @@ typedef struct DisasContext { uint32_t flags; uint32_t mem_idx; int singlestep_enabled; -int bstate; +DisasJumpType is_jmp; /* Remember the rounding mode encoded in the previous fp instruction, which we have already installed into env->fp_status. Or -1 for no previous fp instruction. Note that we exit the TB when writing @@ -55,13 +56,6 @@ typedef struct DisasContext { int frm; } DisasContext; -enum { -BS_NONE = 0, /* When seen outside of translation while loop, indicates - need to exit tb due to end of page. */ -BS_STOP = 1, /* Need to exit tb for syscall, sret, etc. */ -BS_BRANCH = 2, /* Need to exit tb for branch, jal, etc. */ -}; - /* convert riscv funct3 to qemu memop for load/store */ static const int tcg_memop_lookup[8] = { [0 ... 7] = -1, @@ -88,7 +82,7 @@ static void generate_exception(DisasContext *ctx, int excp) TCGv_i32 helper_tmp = tcg_const_i32(excp); gen_helper_raise_exception(cpu_env, helper_tmp); tcg_temp_free_i32(helper_tmp); -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; } static void generate_exception_mbadaddr(DisasContext *ctx, int excp) @@ -98,7 +92,7 @@ static void generate_exception_mbadaddr(DisasContext *ctx, int excp) TCGv_i32 helper_tmp = tcg_const_i32(excp); gen_helper_raise_exception(cpu_env, helper_tmp); tcg_temp_free_i32(helper_tmp); -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; } static void gen_exception_debug(void) @@ -532,7 +526,7 @@ static void gen_jal(CPURISCVState *env, DisasContext *ctx, int rd, } gen_goto_tb(ctx, 0, ctx->pc + imm); /* must use this for safety */ -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; } static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc, @@ -563,7 +557,7 @@ static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc, gen_set_label(misaligned); gen_exception_inst_addr_mis(ctx); } -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; break; default: @@ -617,7 +611,7 @@ static void gen_branch(CPURISCVState *env, DisasContext *ctx, uint32_t opc, } else { gen_goto_tb(ctx, 0, ctx->pc + bimm); } -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; } static void gen_load(DisasContext *ctx, uint32_t opc, int rd, int rs1, @@ -1345,12 +1339,12 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc, /* always generates U-level ECALL, fixed in do_interrupt handler */ generate_exception(ctx, RISCV_EXCP_U_ECALL); tcg_gen_exit_tb(0); /* no chaining */ -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; break; case 0x1: /* EBREAK */ generate_exception(ctx, RISCV_EXCP_BREAKPOINT); tcg_gen_exit_tb(0); /* no chaining */ -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; break; #ifndef CONFIG_USER_ONLY case 0x002: /* URET */ @@ -1360,7 +1354,7 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc, if (riscv_has_ext(env, RVS)) { gen_helper_sret(cpu_pc, cpu_env, cpu_pc); tcg_gen_exit_tb(0); /* no chaining */ -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; } else { gen_exception_illegal(ctx); } @@ -1371,7 +1365,7 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc, case 0x302: /* MRET */ gen_helper_mret(cpu_pc, cpu_env, cpu_pc); tcg_gen_exit_tb(0); /* no chaining */ -ctx->bstate = BS_BRANCH; +ctx->is_jmp = DISAS_NORETURN; break; case 0x7b2: /* DRET */ gen_exception_illegal(ctx); @@ -1418,7 +1412,7 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc, /* end tb since we may be changing priv modes, to get mmu_index right */ tcg_gen_movi_tl(cpu_pc, ctx->next_pc); tcg_gen_exit_tb(0); /* no chaining */ -ctx->bstate
[Qemu-devel] [PATCH v2 16/17] target/riscv: convert to DisasContextBase
Notes: - Did not convert {num,max}_insns, since the corresponding code will go away in the next patch. - ctx->pc becomes ctx->base.pc_next, and ctx->next_pc becomes ctx->pc_tmp. While at it, convert the remaining tb->cflags readers to tb_cflags(). Cc: Michael ClarkCc: Palmer Dabbelt Cc: Sagar Karandikar Cc: Bastian Koppelmann Signed-off-by: Emilio G. Cota --- target/riscv/translate.c | 129 +++ 1 file changed, 64 insertions(+), 65 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index a5c25ab..c619a14 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -40,14 +40,12 @@ static TCGv load_val; #include "exec/gen-icount.h" typedef struct DisasContext { -struct TranslationBlock *tb; -target_ulong pc; -target_ulong next_pc; +DisasContextBase base; +/* pc_tmp points to the pc of the instruction following base.pc_next */ +target_ulong pc_tmp; uint32_t opcode; uint32_t flags; uint32_t mem_idx; -int singlestep_enabled; -DisasJumpType is_jmp; /* Remember the rounding mode encoded in the previous fp instruction, which we have already installed into env->fp_status. Or -1 for no previous fp instruction. Note that we exit the TB when writing @@ -78,21 +76,21 @@ static const int tcg_memop_lookup[8] = { static void generate_exception(DisasContext *ctx, int excp) { -tcg_gen_movi_tl(cpu_pc, ctx->pc); +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); TCGv_i32 helper_tmp = tcg_const_i32(excp); gen_helper_raise_exception(cpu_env, helper_tmp); tcg_temp_free_i32(helper_tmp); -ctx->is_jmp = DISAS_NORETURN; +ctx->base.is_jmp = DISAS_NORETURN; } static void generate_exception_mbadaddr(DisasContext *ctx, int excp) { -tcg_gen_movi_tl(cpu_pc, ctx->pc); +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); tcg_gen_st_tl(cpu_pc, cpu_env, offsetof(CPURISCVState, badaddr)); TCGv_i32 helper_tmp = tcg_const_i32(excp); gen_helper_raise_exception(cpu_env, helper_tmp); tcg_temp_free_i32(helper_tmp); -ctx->is_jmp = DISAS_NORETURN; +ctx->base.is_jmp = DISAS_NORETURN; } static void gen_exception_debug(void) @@ -114,12 +112,12 @@ static void gen_exception_inst_addr_mis(DisasContext *ctx) static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest) { -if (unlikely(ctx->singlestep_enabled)) { +if (unlikely(ctx->base.singlestep_enabled)) { return false; } #ifndef CONFIG_USER_ONLY -return (ctx->tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); +return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK); #else return true; #endif @@ -131,10 +129,10 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) /* chaining is only allowed when the jump is to the same page */ tcg_gen_goto_tb(n); tcg_gen_movi_tl(cpu_pc, dest); -tcg_gen_exit_tb((uintptr_t)ctx->tb + n); +tcg_gen_exit_tb((uintptr_t)ctx->base.tb + n); } else { tcg_gen_movi_tl(cpu_pc, dest); -if (ctx->singlestep_enabled) { +if (ctx->base.singlestep_enabled) { gen_exception_debug(); } else { tcg_gen_exit_tb(0); @@ -514,7 +512,7 @@ static void gen_jal(CPURISCVState *env, DisasContext *ctx, int rd, target_ulong next_pc; /* check misaligned: */ -next_pc = ctx->pc + imm; +next_pc = ctx->base.pc_next + imm; if (!riscv_has_ext(env, RVC)) { if ((next_pc & 0x3) != 0) { gen_exception_inst_addr_mis(ctx); @@ -522,11 +520,11 @@ static void gen_jal(CPURISCVState *env, DisasContext *ctx, int rd, } } if (rd != 0) { -tcg_gen_movi_tl(cpu_gpr[rd], ctx->next_pc); +tcg_gen_movi_tl(cpu_gpr[rd], ctx->pc_tmp); } -gen_goto_tb(ctx, 0, ctx->pc + imm); /* must use this for safety */ -ctx->is_jmp = DISAS_NORETURN; +gen_goto_tb(ctx, 0, ctx->base.pc_next + imm); /* must use this for safety */ +ctx->base.is_jmp = DISAS_NORETURN; } static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc, @@ -549,7 +547,7 @@ static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc, } if (rd != 0) { -tcg_gen_movi_tl(cpu_gpr[rd], ctx->next_pc); +tcg_gen_movi_tl(cpu_gpr[rd], ctx->pc_tmp); } tcg_gen_exit_tb(0); @@ -557,7 +555,7 @@ static void gen_jalr(CPURISCVState *env, DisasContext *ctx, uint32_t opc, gen_set_label(misaligned); gen_exception_inst_addr_mis(ctx); } -ctx->is_jmp = DISAS_NORETURN; +ctx->base.is_jmp = DISAS_NORETURN; break; default: @@ -603,15 +601,15 @@ static void gen_branch(CPURISCVState *env, DisasContext *ctx,
[Qemu-devel] [PATCH v2 14/17] target/openrisc: convert to TranslatorOps
Notes: - Changed the num_insns test in insn_start to check for dc->base.num_insns > 1, since when tb_start is first called in a TB, base.num_insns is already set to 1. - Removed DISAS_NEXT from the switch in tb_stop; use DISAS_TOO_MANY instead. - Added an assert_not_reached on tb_stop for DISAS_NEXT and the default case. - Merged the two separate log_target_disas calls into the disas_log op. Cc: Stafford HorneSigned-off-by: Emilio G. Cota --- target/openrisc/translate.c | 163 +--- 1 file changed, 79 insertions(+), 84 deletions(-) diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c index b37414f..7cf29cd 100644 --- a/target/openrisc/translate.c +++ b/target/openrisc/translate.c @@ -1520,46 +1520,22 @@ static void disas_openrisc_insn(DisasContext *dc, OpenRISCCPU *cpu) } } -void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) +static void openrisc_tr_init_disas_context(DisasContextBase *dcb, CPUState *cs) { +DisasContext *dc = container_of(dcb, DisasContext, base); CPUOpenRISCState *env = cs->env_ptr; -OpenRISCCPU *cpu = openrisc_env_get_cpu(env); -struct DisasContext ctx, *dc = -uint32_t pc_start; -uint32_t next_page_start; -int num_insns; -int max_insns; - -pc_start = tb->pc; - -dc->base.tb = tb; -dc->base.singlestep_enabled = cs->singlestep_enabled; -dc->base.pc_next = pc_start; -dc->base.is_jmp = DISAS_NEXT; +int bound; -dc->mem_idx = cpu_mmu_index(>env, false); +dc->mem_idx = cpu_mmu_index(env, false); dc->tb_flags = dc->base.tb->flags; dc->delayed_branch = (dc->tb_flags & TB_FLAGS_DFLAG) != 0; +bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; +dc->base.max_insns = MIN(dc->base.max_insns, bound); +} -next_page_start = (pc_start & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; -num_insns = 0; -max_insns = tb_cflags(tb) & CF_COUNT_MASK; - -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} - -if (qemu_loglevel_mask(CPU_LOG_TB_IN_ASM) -&& qemu_log_in_addr_range(pc_start)) { -qemu_log_lock(); -qemu_log("\n"); -qemu_log("IN: %s\n", lookup_symbol(pc_start)); -} - -gen_tb_start(tb); +static void openrisc_tr_tb_start(DisasContextBase *db, CPUState *cs) +{ +DisasContext *dc = container_of(db, DisasContext, base); /* Allow the TCG optimizer to see that R0 == 0, when it's true, which is the common case. */ @@ -1568,50 +1544,55 @@ void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) } else { cpu_R[0] = cpu_R0; } +} -do { -tcg_gen_insn_start(dc->base.pc_next, (dc->delayed_branch ? 1 : 0) - | (num_insns ? 2 : 0)); -num_insns++; +static void openrisc_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); -if (unlikely(cpu_breakpoint_test(cs, dc->base.pc_next, BP_ANY))) { -tcg_gen_movi_tl(cpu_pc, dc->base.pc_next); -gen_exception(dc, EXCP_DEBUG); -dc->base.is_jmp = DISAS_NORETURN; -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -dc->base.pc_next += 4; -break; -} +tcg_gen_insn_start(dc->base.pc_next, (dc->delayed_branch ? 1 : 0) + | (dc->base.num_insns > 1 ? 2 : 0)); +} -if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} -disas_openrisc_insn(dc, cpu); -dc->base.pc_next += 4; - -/* delay slot */ -if (dc->delayed_branch) { -dc->delayed_branch--; -if (!dc->delayed_branch) { -tcg_gen_mov_tl(cpu_pc, jmp_pc); -tcg_gen_discard_tl(jmp_pc); -dc->base.is_jmp = DISAS_UPDATE; -break; -} +static bool openrisc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, + const CPUBreakpoint *bp) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); + +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next); +gen_exception(dc, EXCP_DEBUG); +dc->base.is_jmp = DISAS_NORETURN; +/* The address covered by the breakpoint must be included in + [tb->pc, tb->pc + tb->size) in order to for it to be + properly cleared -- thus we increment the PC here so that + the logic setting tb->size below does the right thing. */ +dc->base.pc_next += 4; +return true; +} + +static void
[Qemu-devel] [PATCH v2 17/17] target/riscv: convert to TranslatorOps
Cc: Michael ClarkCc: Palmer Dabbelt Cc: Sagar Karandikar Cc: Bastian Koppelmann Signed-off-by: Emilio G. Cota --- target/riscv/translate.c | 158 --- 1 file changed, 80 insertions(+), 78 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index c619a14..a2024a2 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -1836,78 +1836,71 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx) } } -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) { -CPURISCVState *env = cs->env_ptr; -DisasContext ctx; -target_ulong next_page_start; -int num_insns; -int max_insns; - -ctx.base.pc_first = tb->pc; -ctx.base.pc_next = ctx.base.pc_first; -/* once we have GDB, the rest of the translate.c implementation should be - ready for singlestep */ -ctx.base.singlestep_enabled = cs->singlestep_enabled; -ctx.base.tb = tb; -ctx.base.is_jmp = DISAS_NEXT; - -next_page_start = (ctx.base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; -ctx.pc_tmp = ctx.base.pc_first; -ctx.flags = tb->flags; -ctx.mem_idx = tb->flags & TB_FLAGS_MMU_MASK; -ctx.frm = -1; /* unknown rounding mode */ - -num_insns = 0; -max_insns = tb_cflags(ctx.base.tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} -gen_tb_start(tb); +DisasContext *ctx = container_of(dcbase, DisasContext, base); -while (ctx.base.is_jmp == DISAS_NEXT) { -tcg_gen_insn_start(ctx.base.pc_next); -num_insns++; +ctx->pc_tmp = ctx->base.pc_first; +ctx->flags = ctx->base.tb->flags; +ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK; +ctx->frm = -1; /* unknown rounding mode */ +} -if (unlikely(cpu_breakpoint_test(cs, ctx.base.pc_next, BP_ANY))) { -tcg_gen_movi_tl(cpu_pc, ctx.base.pc_next); -ctx.base.is_jmp = DISAS_NORETURN; -gen_exception_debug(); -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -ctx.base.pc_next += 4; -goto done_generating; -} +static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) +{ +} -if (num_insns == max_insns && (tb_cflags(ctx.base.tb) & CF_LAST_IO)) { -gen_io_start(); -} +static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); + +tcg_gen_insn_start(ctx->base.pc_next); +} + +static bool riscv_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu, + const CPUBreakpoint *bp) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); + +tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next); +ctx->base.is_jmp = DISAS_NORETURN; +gen_exception_debug(); +/* The address covered by the breakpoint must be included in + [tb->pc, tb->pc + tb->size) in order to for it to be + properly cleared -- thus we increment the PC here so that + the logic setting tb->size below does the right thing. */ +ctx->base.pc_next += 4; +return true; +} + + +static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); +CPURISCVState *env = cpu->env_ptr; + +ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next); +decode_opc(env, ctx); +ctx->base.pc_next = ctx->pc_tmp; + +if (ctx->base.is_jmp == DISAS_NEXT) { +target_ulong next_page; -ctx.opcode = cpu_ldl_code(env, ctx.base.pc_next); -decode_opc(env, ); -ctx.base.pc_next = ctx.pc_tmp; - -if (ctx.base.is_jmp == DISAS_NEXT && -(cs->singlestep_enabled || - ctx.base.pc_next >= next_page_start || - tcg_op_buf_full() || - num_insns >= max_insns || - singlestep)) { -ctx.base.is_jmp = DISAS_TOO_MANY; +next_page = (ctx->base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; +if (ctx->base.pc_next >= next_page) { +ctx->base.is_jmp = DISAS_TOO_MANY; } } -if (tb_cflags(ctx.base.tb) & CF_LAST_IO) { -gen_io_end(); -} -switch (ctx.base.is_jmp) { +} + +static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); + +switch
[Qemu-devel] [PATCH v2 01/17] translator: merge max_insns into DisasContextBase
While at it, use int for both num_insns and max_insns to make sure we have same-type comparisons. Reviewed-by: Richard HendersonSigned-off-by: Emilio G. Cota --- include/exec/translator.h | 8 accel/tcg/translator.c | 21 ++--- target/alpha/translate.c | 6 ++ target/arm/translate-a64.c | 8 +++- target/arm/translate.c | 9 +++-- target/hppa/translate.c| 7 ++- target/i386/translate.c| 5 + target/ppc/translate.c | 5 ++--- 8 files changed, 27 insertions(+), 42 deletions(-) diff --git a/include/exec/translator.h b/include/exec/translator.h index e2dc2a0..71e7b2c 100644 --- a/include/exec/translator.h +++ b/include/exec/translator.h @@ -58,6 +58,7 @@ typedef enum DisasJumpType { * disassembly). * @is_jmp: What instruction to disassemble next. * @num_insns: Number of translated instructions (including current). + * @max_insns: Maximum number of instructions to be translated in this TB. * @singlestep_enabled: "Hardware" single stepping enabled. * * Architecture-agnostic disassembly context. @@ -67,7 +68,8 @@ typedef struct DisasContextBase { target_ulong pc_first; target_ulong pc_next; DisasJumpType is_jmp; -unsigned int num_insns; +int num_insns; +int max_insns; bool singlestep_enabled; } DisasContextBase; @@ -76,7 +78,6 @@ typedef struct DisasContextBase { * @init_disas_context: * Initialize the target-specific portions of DisasContext struct. * The generic DisasContextBase has already been initialized. - * Return max_insns, modified as necessary by db->tb->flags. * * @tb_start: * Emit any code required before the start of the main loop, @@ -106,8 +107,7 @@ typedef struct DisasContextBase { * Print instruction disassembly to log. */ typedef struct TranslatorOps { -int (*init_disas_context)(DisasContextBase *db, CPUState *cpu, - int max_insns); +void (*init_disas_context)(DisasContextBase *db, CPUState *cpu); void (*tb_start)(DisasContextBase *db, CPUState *cpu); void (*insn_start)(DisasContextBase *db, CPUState *cpu); bool (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index 23c6602..0f9dca9 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -34,8 +34,6 @@ void translator_loop_temp_check(DisasContextBase *db) void translator_loop(const TranslatorOps *ops, DisasContextBase *db, CPUState *cpu, TranslationBlock *tb) { -int max_insns; - /* Initialize DisasContext */ db->tb = tb; db->pc_first = tb->pc; @@ -45,18 +43,18 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, db->singlestep_enabled = cpu->singlestep_enabled; /* Instruction counting */ -max_insns = tb_cflags(db->tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; +db->max_insns = tb_cflags(db->tb) & CF_COUNT_MASK; +if (db->max_insns == 0) { +db->max_insns = CF_COUNT_MASK; } -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; +if (db->max_insns > TCG_MAX_INSNS) { +db->max_insns = TCG_MAX_INSNS; } if (db->singlestep_enabled || singlestep) { -max_insns = 1; +db->max_insns = 1; } -max_insns = ops->init_disas_context(db, cpu, max_insns); +ops->init_disas_context(db, cpu); tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ /* Reset the temp count so that we can identify leaks */ @@ -95,7 +93,8 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, update db->pc_next and db->is_jmp to indicate what should be done next -- either exiting this loop or locate the start of the next instruction. */ -if (db->num_insns == max_insns && (tb_cflags(db->tb) & CF_LAST_IO)) { +if (db->num_insns == db->max_insns +&& (tb_cflags(db->tb) & CF_LAST_IO)) { /* Accept I/O on the last instruction. */ gen_io_start(); ops->translate_insn(db, cpu); @@ -111,7 +110,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, /* Stop translation if the output buffer is full, or we have executed all of the allowed instructions. */ -if (tcg_op_buf_full() || db->num_insns >= max_insns) { +if (tcg_op_buf_full() || db->num_insns >= db->max_insns) { db->is_jmp = DISAS_TOO_MANY; break; } diff --git a/target/alpha/translate.c b/target/alpha/translate.c index 73a1b5e..15eca71 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -2919,8 +2919,7 @@ static DisasJumpType translate_one(DisasContext *ctx, uint32_t insn) return ret; } -static int
[Qemu-devel] [PATCH v2 04/17] target/sparc: convert to DisasContextBase
Notes: - pc and npc are left unmodified, since they can point to out-of-TB jump targets. - Got rid of last_pc in gen_intermediate_code(), using base.pc_next instead. Only update pc_next (1) on a breakpoint (so that tb->size includes the insn), and (2) after reading the current instruction from memory. This allows us to use base.pc_next in the BP check, which is what the translator loop does. Cc: Mark Cave-AylandCc: Artyom Tarasenko Signed-off-by: Emilio G. Cota --- target/sparc/translate.c | 92 +++- 1 file changed, 45 insertions(+), 47 deletions(-) diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 03c6510..889c439 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -67,14 +67,13 @@ static TCGv_i64 cpu_fpr[TARGET_DPREGS]; #include "exec/gen-icount.h" typedef struct DisasContext { +DisasContextBase base; target_ulong pc;/* current Program Counter: integer or DYNAMIC_PC */ target_ulong npc; /* next PC: integer or DYNAMIC_PC or JUMP_PC */ target_ulong jump_pc[2]; /* used when JUMP_PC pc value is used */ -DisasJumpType is_jmp; int mem_idx; bool fpu_enabled; bool address_mask_32bit; -bool singlestep; #ifndef CONFIG_USER_ONLY bool supervisor; #ifdef TARGET_SPARC64 @@ -83,7 +82,6 @@ typedef struct DisasContext { #endif uint32_t cc_op; /* current CC operation */ -struct TranslationBlock *tb; sparc_def_t *def; TCGv_i32 t32[3]; TCGv ttl[5]; @@ -342,13 +340,13 @@ static inline TCGv gen_dest_gpr(DisasContext *dc, int reg) static inline bool use_goto_tb(DisasContext *s, target_ulong pc, target_ulong npc) { -if (unlikely(s->singlestep)) { +if (unlikely(s->base.singlestep_enabled || singlestep)) { return false; } #ifndef CONFIG_USER_ONLY -return (pc & TARGET_PAGE_MASK) == (s->tb->pc & TARGET_PAGE_MASK) && - (npc & TARGET_PAGE_MASK) == (s->tb->pc & TARGET_PAGE_MASK); +return (pc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) && + (npc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK); #else return true; #endif @@ -362,7 +360,7 @@ static inline void gen_goto_tb(DisasContext *s, int tb_num, tcg_gen_goto_tb(tb_num); tcg_gen_movi_tl(cpu_pc, pc); tcg_gen_movi_tl(cpu_npc, npc); -tcg_gen_exit_tb((uintptr_t)s->tb + tb_num); +tcg_gen_exit_tb((uintptr_t)s->base.tb + tb_num); } else { /* jump to another page: currently not optimized */ tcg_gen_movi_tl(cpu_pc, pc); @@ -996,7 +994,7 @@ static void gen_branch_a(DisasContext *dc, target_ulong pc1) gen_set_label(l1); gen_goto_tb(dc, 1, npc + 4, npc + 8); -dc->is_jmp = DISAS_NORETURN; +dc->base.is_jmp = DISAS_NORETURN; } static void gen_branch_n(DisasContext *dc, target_ulong pc1) @@ -1079,7 +1077,7 @@ static void gen_exception(DisasContext *dc, int which) t = tcg_const_i32(which); gen_helper_raise_exception(cpu_env, t); tcg_temp_free_i32(t); -dc->is_jmp = DISAS_NORETURN; +dc->base.is_jmp = DISAS_NORETURN; } static void gen_check_align(TCGv addr, int mask) @@ -2442,7 +2440,7 @@ static void gen_ldstub_asi(DisasContext *dc, TCGv dst, TCGv addr, int insn) default: /* ??? In theory, this should be raise DAE_invalid_asi. But the SS-20 roms do ldstuba [%l0] #ASI_M_CTL, %o1. */ -if (tb_cflags(dc->tb) & CF_PARALLEL) { +if (tb_cflags(dc->base.tb) & CF_PARALLEL) { gen_helper_exit_atomic(cpu_env); } else { TCGv_i32 r_asi = tcg_const_i32(da.asi); @@ -3352,7 +3350,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) if (cond == 8) { /* An unconditional trap ends the TB. */ -dc->is_jmp = DISAS_NORETURN; +dc->base.is_jmp = DISAS_NORETURN; goto jmp_insn; } else { /* A conditional trap falls through to the next insn. */ @@ -4332,7 +4330,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn(); tcg_gen_exit_tb(0); -dc->is_jmp = DISAS_NORETURN; +dc->base.is_jmp = DISAS_NORETURN; break; case 0x6: /* V9 wrfprs */ tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2); @@ -4341,7 +4339,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn();
[Qemu-devel] [PATCH v2 02/17] target/sh4: convert to TranslatorOps
This was fairly straightforward since it had already been converted to DisasContextBase; just had to add TARGET_TOO_MANY to the switch in tb_stop. Reviewed-by: Richard HendersonCc: Aurelien Jarno Signed-off-by: Emilio G. Cota --- target/sh4/translate.c | 171 + 1 file changed, 86 insertions(+), 85 deletions(-) diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 012156b..58bdfeb 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -2258,126 +2258,127 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State *env, int *pmax_insns) } #endif -void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb) +static void sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) { +DisasContext *ctx = container_of(dcbase, DisasContext, base); CPUSH4State *env = cs->env_ptr; -DisasContext ctx; -target_ulong pc_start; -int num_insns; -int max_insns; - -pc_start = tb->pc; -ctx.base.pc_next = pc_start; -ctx.tbflags = (uint32_t)tb->flags; -ctx.envflags = tb->flags & TB_FLAG_ENVFLAGS_MASK; -ctx.base.is_jmp = DISAS_NEXT; -ctx.memidx = (ctx.tbflags & (1u << SR_MD)) == 0 ? 1 : 0; +int bound; + +ctx->tbflags = (uint32_t)ctx->base.tb->flags; +ctx->envflags = ctx->base.tb->flags & TB_FLAG_ENVFLAGS_MASK; +ctx->memidx = (ctx->tbflags & (1u << SR_MD)) == 0 ? 1 : 0; /* We don't know if the delayed pc came from a dynamic or static branch, so assume it is a dynamic branch. */ -ctx.delayed_pc = -1; /* use delayed pc from env pointer */ -ctx.base.tb = tb; -ctx.base.singlestep_enabled = cs->singlestep_enabled; -ctx.features = env->features; -ctx.has_movcal = (ctx.tbflags & TB_FLAG_PENDING_MOVCA); -ctx.gbank = ((ctx.tbflags & (1 << SR_MD)) && - (ctx.tbflags & (1 << SR_RB))) * 0x10; -ctx.fbank = ctx.tbflags & FPSCR_FR ? 0x10 : 0; - -max_insns = tb_cflags(tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -max_insns = MIN(max_insns, TCG_MAX_INSNS); +ctx->delayed_pc = -1; /* use delayed pc from env pointer */ +ctx->features = env->features; +ctx->has_movcal = (ctx->tbflags & TB_FLAG_PENDING_MOVCA); +ctx->gbank = ((ctx->tbflags & (1 << SR_MD)) && + (ctx->tbflags & (1 << SR_RB))) * 0x10; +ctx->fbank = ctx->tbflags & FPSCR_FR ? 0x10 : 0; /* Since the ISA is fixed-width, we can bound by the number of instructions remaining on the page. */ -num_insns = -(ctx.base.pc_next | TARGET_PAGE_MASK) / 2; -max_insns = MIN(max_insns, num_insns); - -/* Single stepping means just that. */ -if (ctx.base.singlestep_enabled || singlestep) { -max_insns = 1; -} - -gen_tb_start(tb); -num_insns = 0; +bound = -(ctx->base.pc_next | TARGET_PAGE_MASK) / 2; +ctx->base.max_insns = MIN(ctx->base.max_insns, bound); +} +static void sh4_tr_tb_start(DisasContextBase *dcbase, CPUState *cs) +{ #ifdef CONFIG_USER_ONLY -if (ctx.tbflags & GUSA_MASK) { -num_insns = decode_gusa(, env, _insns); +DisasContext *ctx = container_of(dcbase, DisasContext, base); +CPUSH4State *env = cs->env_ptr; + +if (ctx->tbflags & GUSA_MASK) { +ctx->base.num_insns = decode_gusa(ctx, env, >base.max_insns); } #endif +} -while (ctx.base.is_jmp == DISAS_NEXT - && num_insns < max_insns - && !tcg_op_buf_full()) { -tcg_gen_insn_start(ctx.base.pc_next, ctx.envflags); -num_insns++; +static void sh4_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); -if (unlikely(cpu_breakpoint_test(cs, ctx.base.pc_next, BP_ANY))) { -/* We have hit a breakpoint - make sure PC is up-to-date */ -gen_save_cpu_state(, true); -gen_helper_debug(cpu_env); -ctx.base.is_jmp = DISAS_NORETURN; -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -ctx.base.pc_next += 2; -break; -} +tcg_gen_insn_start(ctx->base.pc_next, ctx->envflags); +} -if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} +static bool sh4_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, +const CPUBreakpoint *bp) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); -ctx.opcode = cpu_lduw_code(env, ctx.base.pc_next); - decode_opc(); -ctx.base.pc_next += 2; -} -if (tb_cflags(tb) & CF_LAST_IO) { -
[Qemu-devel] [PATCH v2 05/17] target/sparc: convert to TranslatorOps
Notes: - Moved the cross-page check from the end of translate_insn to init_disas_context. Tested-by: Mark Cave-AylandCc: Mark Cave-Ayland Cc: Artyom Tarasenko Signed-off-by: Emilio G. Cota --- target/sparc/translate.c | 174 +++ 1 file changed, 86 insertions(+), 88 deletions(-) diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 889c439..40b2eaa 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -5737,99 +5737,91 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) } } -void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) +static void sparc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) { +DisasContext *dc = container_of(dcbase, DisasContext, base); CPUSPARCState *env = cs->env_ptr; -DisasContext dc1, *dc = -int max_insns; -unsigned int insn; - -memset(dc, 0, sizeof(DisasContext)); -dc->base.tb = tb; -dc->base.pc_first = tb->pc; -dc->base.pc_next = tb->pc; -dc->base.is_jmp = DISAS_NEXT; -dc->base.num_insns = 0; -dc->base.singlestep_enabled = cs->singlestep_enabled; +int bound; dc->pc = dc->base.pc_first; -dc->npc = (target_ulong) tb->cs_base; +dc->npc = (target_ulong)dc->base.tb->cs_base; dc->cc_op = CC_OP_DYNAMIC; -dc->mem_idx = tb->flags & TB_FLAG_MMU_MASK; +dc->mem_idx = dc->base.tb->flags & TB_FLAG_MMU_MASK; dc->def = >def; -dc->fpu_enabled = tb_fpu_enabled(tb->flags); -dc->address_mask_32bit = tb_am_enabled(tb->flags); +dc->fpu_enabled = tb_fpu_enabled(dc->base.tb->flags); +dc->address_mask_32bit = tb_am_enabled(dc->base.tb->flags); #ifndef CONFIG_USER_ONLY -dc->supervisor = (tb->flags & TB_FLAG_SUPER) != 0; +dc->supervisor = (dc->base.tb->flags & TB_FLAG_SUPER) != 0; #endif #ifdef TARGET_SPARC64 dc->fprs_dirty = 0; -dc->asi = (tb->flags >> TB_FLAG_ASI_SHIFT) & 0xff; +dc->asi = (dc->base.tb->flags >> TB_FLAG_ASI_SHIFT) & 0xff; #ifndef CONFIG_USER_ONLY -dc->hypervisor = (tb->flags & TB_FLAG_HYPER) != 0; +dc->hypervisor = (dc->base.tb->flags & TB_FLAG_HYPER) != 0; #endif #endif +/* + * if we reach a page boundary, we stop generation so that the + * PC of a TT_TFAULT exception is always in the right page + */ +bound = -(dc->base.pc_first | TARGET_PAGE_MASK) / 4; +dc->base.max_insns = MIN(dc->base.max_insns, bound); +} -max_insns = tb_cflags(tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} -if (dc->base.singlestep_enabled || singlestep) { -max_insns = 1; -} +static void sparc_tr_tb_start(DisasContextBase *db, CPUState *cs) +{ +} -gen_tb_start(tb); -do { -if (dc->npc & JUMP_PC) { -assert(dc->jump_pc[1] == dc->pc + 4); -tcg_gen_insn_start(dc->pc, dc->jump_pc[0] | JUMP_PC); -} else { -tcg_gen_insn_start(dc->pc, dc->npc); -} -dc->base.num_insns++; +static void sparc_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); -if (unlikely(cpu_breakpoint_test(cs, dc->base.pc_next, BP_ANY))) { -if (dc->pc != dc->base.pc_first) { -save_state(dc); -} -gen_helper_debug(cpu_env); -tcg_gen_exit_tb(0); -dc->base.is_jmp = DISAS_NORETURN; -dc->base.pc_next += 4; -goto exit_gen_loop; -} +if (dc->npc & JUMP_PC) { +assert(dc->jump_pc[1] == dc->pc + 4); +tcg_gen_insn_start(dc->pc, dc->jump_pc[0] | JUMP_PC); +} else { +tcg_gen_insn_start(dc->pc, dc->npc); +} +} -if (dc->base.num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} +static bool sparc_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs, + const CPUBreakpoint *bp) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); -insn = cpu_ldl_code(env, dc->pc); -dc->base.pc_next += 4; +if (dc->pc != dc->base.pc_first) { +save_state(dc); +} +gen_helper_debug(cpu_env); +tcg_gen_exit_tb(0); +dc->base.is_jmp = DISAS_NORETURN; +/* update pc_next so that the current instruction is included in tb->size */ +dc->base.pc_next += 4; +return true; +} -disas_sparc_insn(dc, insn); +static void sparc_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *dc = container_of(dcbase, DisasContext, base); +CPUSPARCState *env = cs->env_ptr; +unsigned int insn; -if (dc->base.is_jmp == DISAS_NORETURN) { -break; -} -
[Qemu-devel] [PATCH v2 03/17] target/sparc: convert to DisasJumpType
Reviewed-by: Richard HendersonCc: Mark Cave-Ayland Cc: Artyom Tarasenko Signed-off-by: Emilio G. Cota --- target/sparc/translate.c | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/target/sparc/translate.c b/target/sparc/translate.c index 5aa367a..03c6510 100644 --- a/target/sparc/translate.c +++ b/target/sparc/translate.c @@ -30,6 +30,7 @@ #include "exec/helper-gen.h" #include "trace-tcg.h" +#include "exec/translator.h" #include "exec/log.h" #include "asi.h" @@ -69,7 +70,7 @@ typedef struct DisasContext { target_ulong pc;/* current Program Counter: integer or DYNAMIC_PC */ target_ulong npc; /* next PC: integer or DYNAMIC_PC or JUMP_PC */ target_ulong jump_pc[2]; /* used when JUMP_PC pc value is used */ -int is_br; +DisasJumpType is_jmp; int mem_idx; bool fpu_enabled; bool address_mask_32bit; @@ -995,7 +996,7 @@ static void gen_branch_a(DisasContext *dc, target_ulong pc1) gen_set_label(l1); gen_goto_tb(dc, 1, npc + 4, npc + 8); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; } static void gen_branch_n(DisasContext *dc, target_ulong pc1) @@ -1078,7 +1079,7 @@ static void gen_exception(DisasContext *dc, int which) t = tcg_const_i32(which); gen_helper_raise_exception(cpu_env, t); tcg_temp_free_i32(t); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; } static void gen_check_align(TCGv addr, int mask) @@ -3351,7 +3352,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) if (cond == 8) { /* An unconditional trap ends the TB. */ -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; goto jmp_insn; } else { /* A conditional trap falls through to the next insn. */ @@ -4331,7 +4332,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn(); tcg_gen_exit_tb(0); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; break; case 0x6: /* V9 wrfprs */ tcg_gen_xor_tl(cpu_tmp0, cpu_src1, cpu_src2); @@ -4340,7 +4341,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn(); tcg_gen_exit_tb(0); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; break; case 0xf: /* V9 sir, nop if user */ #if !defined(CONFIG_USER_ONLY) @@ -4468,7 +4469,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn(); tcg_gen_exit_tb(0); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; #endif } break; @@ -4624,7 +4625,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) save_state(dc); gen_op_next_insn(); tcg_gen_exit_tb(0); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; break; case 1: // htstate // XXX gen_op_wrhtstate(); @@ -5690,7 +5691,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned int insn) } else if (dc->npc == JUMP_PC) { /* we can do a static jump */ gen_branch2(dc, dc->jump_pc[0], dc->jump_pc[1], cpu_cond); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; } else { dc->pc = dc->npc; dc->npc = dc->npc + 4; @@ -5752,6 +5753,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) pc_start = tb->pc; dc->pc = pc_start; last_pc = dc->pc; +dc->is_jmp = DISAS_NEXT; dc->npc = (target_ulong) tb->cs_base; dc->cc_op = CC_OP_DYNAMIC; dc->mem_idx = tb->flags & TB_FLAG_MMU_MASK; @@ -5796,7 +5798,7 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb) } gen_helper_debug(cpu_env); tcg_gen_exit_tb(0); -dc->is_br = 1; +dc->is_jmp = DISAS_NORETURN; goto exit_gen_loop; } @@ -5808,8 +5810,9 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock * tb)
[Qemu-devel] [PATCH v2 09/17] target/mips: convert to TranslatorOps
Notes: - DISAS_TOO_MANY replaces the former "break" in the translation loop. However, care must be taken not to overwrite a previous condition in is_jmp; that's why in translate_insn we first check is_jmp and return if it's != DISAS_NEXT. - Added an assert in translate_insn, before exiting due to an exception, to make sure that is_jmp is set to DISAS_EXCP (the exception generation function always sets it.) - Added an assert for the default case in is_jmp's switch. Cc: Aurelien JarnoCc: Yongbok Kim Signed-off-by: Emilio G. Cota --- target/mips/translate.c | 227 1 file changed, 113 insertions(+), 114 deletions(-) diff --git a/target/mips/translate.c b/target/mips/translate.c index 08bd140..f01139c 100644 --- a/target/mips/translate.c +++ b/target/mips/translate.c @@ -1432,6 +1432,7 @@ static TCGv_i64 msa_wr_d[64]; typedef struct DisasContext { DisasContextBase base; target_ulong saved_pc; +target_ulong next_page_start; uint32_t opcode; int insn_flags; int32_t CP0_Config1; @@ -20194,24 +20195,12 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) } } -void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) +static void mips_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) { +DisasContext *ctx = container_of(dcbase, DisasContext, base); CPUMIPSState *env = cs->env_ptr; -DisasContext ctx1; -DisasContext *ctx = -target_ulong next_page_start; -int max_insns; -int insn_bytes; -int is_slot; - -ctx->base.tb = tb; -ctx->base.pc_first = tb->pc; -ctx->base.pc_next = tb->pc; -ctx->base.is_jmp = DISAS_NEXT; -ctx->base.singlestep_enabled = cs->singlestep_enabled; -ctx->base.num_insns = 0; -next_page_start = (ctx->base.pc_first & TARGET_PAGE_MASK) + +ctx->next_page_start = (ctx->base.pc_first & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; ctx->saved_pc = -1; ctx->insn_flags = env->insn_flags; @@ -20245,99 +20234,102 @@ void gen_intermediate_code(CPUState *cs, TranslationBlock *tb) #endif ctx->default_tcg_memop_mask = (ctx->insn_flags & ISA_MIPS32R6) ? MO_UNALN : MO_ALIGN; -max_insns = tb_cflags(tb) & CF_COUNT_MASK; -if (max_insns == 0) { -max_insns = CF_COUNT_MASK; -} -if (max_insns > TCG_MAX_INSNS) { -max_insns = TCG_MAX_INSNS; -} -LOG_DISAS("\ntb %p idx %d hflags %04x\n", tb, ctx->mem_idx, ctx->hflags); -gen_tb_start(tb); -while (ctx->base.is_jmp == DISAS_NEXT) { -tcg_gen_insn_start(ctx->base.pc_next, ctx->hflags & MIPS_HFLAG_BMASK, - ctx->btarget); -ctx->base.num_insns++; +LOG_DISAS("\ntb %p idx %d hflags %04x\n", ctx->base.tb, ctx->mem_idx, + ctx->hflags); +} -if (unlikely(cpu_breakpoint_test(cs, ctx->base.pc_next, BP_ANY))) { -save_cpu_state(ctx, 1); -ctx->base.is_jmp = DISAS_NORETURN; -gen_helper_raise_exception_debug(cpu_env); -/* The address covered by the breakpoint must be included in - [tb->pc, tb->pc + tb->size) in order to for it to be - properly cleared -- thus we increment the PC here so that - the logic setting tb->size below does the right thing. */ -ctx->base.pc_next += 4; -goto done_generating; -} +static void mips_tr_tb_start(DisasContextBase *dcbase, CPUState *cs) +{ +} -if (ctx->base.num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) { -gen_io_start(); -} +static void mips_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) +{ +DisasContext *ctx = container_of(dcbase, DisasContext, base); -is_slot = ctx->hflags & MIPS_HFLAG_BMASK; -if (!(ctx->hflags & MIPS_HFLAG_M16)) { -ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next); -insn_bytes = 4; -decode_opc(env, ctx); -} else if (ctx->insn_flags & ASE_MICROMIPS) { -ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next); -insn_bytes = decode_micromips_opc(env, ctx); -} else if (ctx->insn_flags & ASE_MIPS16) { -ctx->opcode = cpu_lduw_code(env, ctx->base.pc_next); -insn_bytes = decode_mips16_opc(env, ctx); -} else { -generate_exception_end(ctx, EXCP_RI); -break; -} +tcg_gen_insn_start(ctx->base.pc_next, ctx->hflags & MIPS_HFLAG_BMASK, + ctx->btarget); +} -if (ctx->hflags & MIPS_HFLAG_BMASK) { -if (!(ctx->hflags & (MIPS_HFLAG_BDS16 | MIPS_HFLAG_BDS32 | -MIPS_HFLAG_FBNSLOT))) { -/* force to generate branch as there is neither delay nor - forbidden slot */ -is_slot = 1; -} -
[Qemu-devel] [PATCH v2 00/17] Translation loop conversion for sh4/sparc/mips/s390x/openrisc/riscv targets
v1: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg00445.html Changes since v1: - Rebase onto master - Add R-b's - Add riscv conversion - s390x: + fix comment in s390x_tr_breakpoint_check You can fetch this series from: https://github.com/cota/qemu/tree/trloop-conv-v2 Thanks, Emilio --- accel/tcg/translator.c | 21 +- include/exec/translator.h |8 +- target/alpha/translate.c|6 +- target/arm/translate-a64.c |8 +- target/arm/translate.c |9 +- target/hppa/translate.c |7 +- target/i386/translate.c |5 +- target/mips/translate.c | 623 +- target/openrisc/translate.c | 226 ++-- target/ppc/translate.c |5 +- target/riscv/translate.c| 255 ++--- target/s390x/translate.c| 1529 - target/sh4/translate.c | 171 +-- target/sparc/translate.c| 207 ++-- 14 files changed, 1522 insertions(+), 1558 deletions(-)
Re: [Qemu-devel] [PULL 10/25] aarch64-linux-user: Add support for SVE signal frame records
On 6 April 2018 at 19:12, Peter Maydellwrote: > On 9 March 2018 at 17:26, Peter Maydell wrote: >> From: Richard Henderson >> >> Depending on the currently selected size of the SVE vector registers, >> we can either store the data within the "standard" allocation, or we >> may beedn to allocate additional space with an EXTRA record. >> >> Signed-off-by: Richard Henderson >> Message-id: 20180303143823.27055-6-richard.hender...@linaro.org >> Reviewed-by: Peter Maydell >> Signed-off-by: Peter Maydell >> --- >> linux-user/signal.c | 210 >> +++- >> 1 file changed, 192 insertions(+), 18 deletions(-) > > I did a 'git bisect' looking for when we introduced the segv > bug described in https://bugs.launchpad.net/qemu/+bug/1761535, > and git bisect thinks this commit is it. > > At least for me, with a xenial aarch64 chroot, with this > patch if I chroot in and run 'ls' then we get a spurious > segfault (I think in the guest bash): > > e104462:xenial:chroot$ sudo chroot xenial-aarch64 > root@e104462:/# ls > Deadlock.classbin home hotspot_pid17113.log > mnt root srv var > Deadlock.java boot hotspot_pid14759.log hotspot_pid19407.log > opt runsys > HelloWorld.class dev hotspot_pid16542.log lib > proc sbin tmp > HelloWorld.java etc hotspot_pid16895.log media > risu sdbg9 usr > qemu: uncaught target signal 11 (Segmentation fault) - core dumped > Segmentation fault (core dumped) > > Reverting f914baef8e39f7 on master fixes the segfault. Er, I mean "reverting 8c5931de0ac77388096d79c". Accidentally quoted the git hash of the local revert-commit I was testing rather than of the commit it's reverting... thanks -- PMM
Re: [Qemu-devel] [PULL for-2.12 0/3] Qcrypto next patches
On 6 April 2018 at 14:30, Daniel P. Berrangéwrote: > The following changes since commit 0e87fdc966d05f4e5ad868034fcd8ee2a08ca62d: > > Update version for v2.12.0-rc2 release (2018-04-04 20:37:20 +0100) > > are available in the Git repository at: > > https://github.com/berrange/qemu tags/qcrypto-next-pull-request > > for you to fetch changes up to 057ad0b46992e3ec4ce29b9103162aa3c683f347: > > crypto: ensure we use a predictable TLS priority setting (2018-04-06 > 11:28:31 +0100) > > > > Two improvements to TLS docs, and a fix for the test suite to > avoid breakage on Fedora >= 28 > Applied, thanks. -- PMM
Re: [Qemu-devel] [PULL 10/25] aarch64-linux-user: Add support for SVE signal frame records
On 9 March 2018 at 17:26, Peter Maydellwrote: > From: Richard Henderson > > Depending on the currently selected size of the SVE vector registers, > we can either store the data within the "standard" allocation, or we > may beedn to allocate additional space with an EXTRA record. > > Signed-off-by: Richard Henderson > Message-id: 20180303143823.27055-6-richard.hender...@linaro.org > Reviewed-by: Peter Maydell > Signed-off-by: Peter Maydell > --- > linux-user/signal.c | 210 > +++- > 1 file changed, 192 insertions(+), 18 deletions(-) I did a 'git bisect' looking for when we introduced the segv bug described in https://bugs.launchpad.net/qemu/+bug/1761535, and git bisect thinks this commit is it. At least for me, with a xenial aarch64 chroot, with this patch if I chroot in and run 'ls' then we get a spurious segfault (I think in the guest bash): e104462:xenial:chroot$ sudo chroot xenial-aarch64 root@e104462:/# ls Deadlock.classbin home hotspot_pid17113.log mnt root srv var Deadlock.java boot hotspot_pid14759.log hotspot_pid19407.log opt runsys HelloWorld.class dev hotspot_pid16542.log lib proc sbin tmp HelloWorld.java etc hotspot_pid16895.log media risu sdbg9 usr qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (core dumped) Reverting f914baef8e39f7 on master fixes the segfault. Richard (Henderson): I'll have a closer look at this on Monday if you don't get to it first... thanks -- PMM
Re: [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file
On 04/06/2018 12:28 PM, Laszlo Ersek wrote: > I've created an RFC-level "qapi/firmware.json" schema file, based on > this discussion. It "builds", and the generated documentation looks > acceptable, superficially speaking. > > Before I post "qapi/firmware.json" for getting comments, I'd like to > write JSON text that (a) describes firmware that I use, and (b) conforms > to the schema. IOW, I'd like to validate whether the schema is good > enough for describing at least such firmware that I know. > > Is there a tool that generates example JSON objects from a given schema? I know the QMP shell (scripts/qmp/qmp-shell) lets you enter commands with a lot less typing than full JSON, and has a mode where it will then echo the full JSON command it constructed from what you typed. To be able to quickly validate examples, it may be sufficient to temporarily add a new QMP command 'check-firmware': { 'command': 'check-firmware', 'boxed': true, 'data': 'Firmware' } assuming 'Firmware' is your top-level 'struct' in the QAPI file, then implement a trivial: qmp_check_firmware(Firmware *obj, Error **errp) { return 0; } so that you can then run QMP shell, and type: check-firmware arg1=foo arg2=bar ... which will then generate the corresponding JSON, then either successfully do nothing (what you typed validated, AND you have the JSON output printed), or print an error (what you typed failed QAPI validation, perhaps because it had an unrecognized key, was missing a required key, used a wrong type, etc). > I vaguely recall there used to be one. Otherwise, writing the examples > manually looks arduous (and I wouldn't know how to verify them against > the schema). Similarly, if you generate a command the produces a 'Firmware' as the return value, then you can populate the generated C struct (since you did manage to run the QAPI generator over your new file, you should be able to look at the C struct it generated), then output that over QMP to show the counterpart JSON that matches the struct as populated. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 1761535] Re: qemu-aarch64-static docker arm64v8/openjdk coredump
I realized I had a javac lying around from last time somebody wanted me to debug a java problem, and I'm also seeing SEGVs with simpler programs like ls (!), so I'll have a look at those and hopefully that will be the same cause as what you're seeing. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1761535 Title: qemu-aarch64-static docker arm64v8/openjdk coredump Status in QEMU: New Bug description: I am using qemu-aarch64-static to run the arm64v8/openjdk official image on my x86 machine. Using QEMU master, I immediately hit a bug which hangs the container. With Ubuntu default version qemu-aarch64 version 2.5.0 (Debian 1:2.5+dfsg-5ubuntu10.24) and qemu-aarch64 version 2.11.1 (v2.11.1-dirty) the hang does not take place. To reproduce (and get to the core dump): $ /tmp/tmptgyg3nvh/qemu-aarch64-static/qemu-aarch64-static -version qemu-aarch64 version 2.11.91 (v2.12.0-rc1-5-g47d3b60-dirty) Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers $ docker run -it -v /tmp/tmptgyg3nvh/qemu-aarch64-static:/usr/bin/qemu-aarch64-static arm64v8/openjdk /bin/bash root@bf75cf45d311:/# javac Usage: javac where possible options include: -g Generate all debugging info <...snip...> @Read options and filenames from file qemu: uncaught target signal 11 (Segmentation fault) - core dumped ...TERMINAL HANGS... To get the core dump, In a separate terminal: # snapshot the file system of the hung image $ docker commit $(docker ps -aqf "name=latest_qemu") qemu_coredump # connect with known working qemu $ docker run -t -v /usr/bin/qemu-aarch64-static:/usr/bin/qemu-aarch64-static -i qemu_coredump /bin/bash $$ ls -lat total 10608 -rw-r--r-- 1 root root 10792960 Mar 29 18:02 qemu_bash_20180329-180251_1.core drwxrwxrwt 5 root root 4096 Mar 29 18:02 tmp To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1761535/+subscriptions
Re: [Qemu-devel] [PULL 01/20] sys_membarrier: fix up include directives
On 04/06/2018 12:11 PM, Paolo Bonzini wrote: > From: Bruce Rogers> > Our rule right now is to use <> for external headers only. > util/sys_membarrier.c violates that. Fix it up. > > Signed-off-by: Bruce Rogers > Message-Id: <20180329151018.15319-1-brog...@suse.com> > Signed-off-by: Paolo Bonzini > --- > util/sys_membarrier.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) Keeping this one as-is is fine, since it is already part of a pull request, but... > > diff --git a/util/sys_membarrier.c b/util/sys_membarrier.c > index 8dcb53e..1362c0c 100644 > --- a/util/sys_membarrier.c > +++ b/util/sys_membarrier.c > @@ -6,9 +6,9 @@ > * Author: Paolo Bonzini > */ > > -#include > -#include > -#include > +#include "qemu/osdep.h" > +#include "qemu/sys_membarrier.h" > +#include "qemu/error-report.h" > > #ifdef CONFIG_LINUX > #include Our style also recommends ordering things as: "qemu/osdep.h" all "any other qemu.h" to minimize chances of collisions from something in a qemu header causing a system header to go wrong (that is, osdep.h has to go first, because it might influence a system header, but nothing else should risk influencing a system header). So if you wanted, you could also sink the "qemu/sys_membarrier.h" and "qemu/error-report.h" includes after the and any other system headers not shown in the context of the patch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [Qemu-arm] [PATCH v5 3/3] target/arm: Add the XML dynamic generation
Hi Alex, First of all, thanks for the review! >> +static int arm_gdb_get_sysreg(CPUARMState *env, uint8_t *buf, int reg) >> +{ >> +ARMCPU *cpu = arm_env_get_cpu(env); >> +const ARMCPRegInfo *ri; >> +uint32_t key; >> + >> +key = cpu->dyn_xml.cpregs_keys[reg]; >> +ri = get_arm_cp_reginfo(cpu->cp_regs, key); >> +if (ri) { >> +if (cpreg_field_is_64bit(ri)) { >> +return gdb_get_reg64(buf, (uint64_t)read_raw_cp_reg(env, ri)); >> +} else { >> +return gdb_get_reg32(buf, (uint32_t)read_raw_cp_reg(env, ri)); >> +} >> +} >> +return 0; > There is something odd going on here because if I run a simple little > features binary > (https://github.com/stsquad/testcases/blob/master/aarch64/features.c) I > get: > > ID_AA64ISAR0_EL1: 0x00011120 > ID_AA64ISAR1_EL1: 0x > ID_AA64MMFR0_EL1: 0xff00 > ID_AA64MMFR1_EL1: 0x > ID_AA64PFR0_EL1 : 0x0011 > ID_AA64PFR1_EL1 : 0x > ID_AA64DFR0_EL1 : 0x0006 > ID_AA64DFR1_EL1 : 0x > MIDR_EL1: 0x411fd070 > MPIDR_EL1 : 0x8000 > REVIDR_EL1 : 0x > > However in the gdb output we see: > > ID_AA64ISAR0_EL1 0x11120 69920 > ID_AA64ISAR1_EL1 0x0 0 > ID_AA64MMFR0_EL1 0x1124 4388 <-? > ID_AA64MMFR1_EL1 0x0 0 > ID_PFR00x131305 <-name and value? > ID_AA64PFR1_EL10x0 0 > ID_AA64DFR0_EL10x10305106 271601926 <-? > ID_AA64DFR1_EL10x0 0 > REVIDR_EL1 0x0 0 > > So why the discrepancies? > ID_AA64MMFR0_EL1 0x11244388 <-? > ID_AA64DFR0_EL10x10305106271601926 <-? I get the same value in x1 (= 0x1124) and x2 (= 0x10305106) when I execute the following instructions on the guest: MRS x1, ID_AA64MMFR0_EL1 MRS x2, ID_AA64DFR0_EL1 So, I think that there is no problem on how GDB is reading these registers! > ID_PFR00x131 305 <-name and value? This is not ID_AA64PFR0_EL1 - the ID_AA64PFR0_EL1 is not registered in our dynamic XML as it has "ARM_CP_NO_RAW" type. So should we add some modifications to handle this special case? Best regards, Abdallah
Re: [Qemu-devel] [RFC] Defining firmware (OVMF, et al) metadata format & file
On 03/08/18 11:17, Daniel P. Berrangé wrote: > On Thu, Mar 08, 2018 at 08:52:45AM +0100, Gerd Hoffmann wrote: >> Hi, >> [*] Open question: Who, between QEMU and libvirt, should define the said firmware metadata format and file? >>> >>> IMHO QEMU should be defining the format, because the file will contain >>> info about certain QEMU features associated with the firmware (eg smm). >>> Also there are potentially other non-libvirt mgmt apps that spawn QEMU >>> which would like this info (eg libguestfs), so having libvirt define the >>> format is inappropriate. >>> >>> I'd suggest we just need something in docs/specs/firmware-metadata.rst >>> for QEMU source tree. >>> >>> Potentially QEMU could even use the metadata files itself for finding >>> the default firmeware images, instead of compiling this info into its >>> binaries. I wouldn't suggest we need todo that right away, but bear it >>> in mind as a potential use case. >> >> With qemu using this itself in mind it probably makes sense to specify >> this as qapi schema. That'll simplify parsing and using these files in >> qemu, and possibly simplifies things on the libvirt side too. > > I was thinking of an 'ini' style format, similar to that used by systemd > unit files, but a JSON format file is a nicer fit with QEMU & Libvirt if > we describe it with qapi. I've created an RFC-level "qapi/firmware.json" schema file, based on this discussion. It "builds", and the generated documentation looks acceptable, superficially speaking. Before I post "qapi/firmware.json" for getting comments, I'd like to write JSON text that (a) describes firmware that I use, and (b) conforms to the schema. IOW, I'd like to validate whether the schema is good enough for describing at least such firmware that I know. Is there a tool that generates example JSON objects from a given schema? I vaguely recall there used to be one. Otherwise, writing the examples manually looks arduous (and I wouldn't know how to verify them against the schema). Thanks! Laszlo
[Qemu-devel] [PULL 20/20] Add missing bit for SSE instr in VEX decoding
From: Eugene MinibaevThe 2-byte VEX prefix imples a leading 0Fh opcode byte. Signed-off-by: Eugene Minibaev Signed-off-by: Paolo Bonzini --- target/i386/translate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 3b7ce92..c9ed8dc 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -4563,9 +4563,11 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) #endif rex_r = (~vex2 >> 4) & 8; if (b == 0xc5) { +/* 2-byte VEX prefix: Rlpp, implied 0f leading opcode byte */ vex3 = vex2; -b = x86_ldub_code(env, s); +b = x86_ldub_code(env, s) | 0x100; } else { +/* 3-byte VEX prefix: RXBm wlpp */ #ifdef TARGET_X86_64 s->rex_x = (~vex2 >> 3) & 8; s->rex_b = (~vex2 >> 2) & 8; -- 1.8.3.1
[Qemu-devel] [PULL 18/20] dump: Fix build with newer gcc
From: Eric Blakegcc 8 on rawhide is picky enough to complain: /home/dummy/qemu/dump.c: In function 'create_header32': /home/dummy/qemu/dump.c:817:5: error: 'strncpy' output truncated before terminating nul copying 8 bytes from a string of the same length [-Werror=stringop-truncation] strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); ^~~~ But we already have SIG_LEN defined as the right length without needing to do a strlen(), and memcpy() is better than strncpy() when we know we do not want a trailing NUL byte. Signed-off-by: Eric Blake Signed-off-by: Paolo Bonzini --- dump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dump.c b/dump.c index 669f715..b54cd42 100644 --- a/dump.c +++ b/dump.c @@ -814,7 +814,7 @@ static void create_header32(DumpState *s, Error **errp) size = sizeof(DiskDumpHeader32); dh = g_malloc0(size); -strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); +memcpy(dh->signature, KDUMP_SIGNATURE, SIG_LEN); dh->header_version = cpu_to_dump32(s, 6); block_size = s->dump_info.page_size; dh->block_size = cpu_to_dump32(s, block_size); @@ -926,7 +926,7 @@ static void create_header64(DumpState *s, Error **errp) size = sizeof(DiskDumpHeader64); dh = g_malloc0(size); -strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); +memcpy(dh->signature, KDUMP_SIGNATURE, SIG_LEN); dh->header_version = cpu_to_dump32(s, 6); block_size = s->dump_info.page_size; dh->block_size = cpu_to_dump32(s, block_size); -- 1.8.3.1
[Qemu-devel] [PULL 15/20] qemu-pr-helper: Daemonize before dropping privileges
From: Michal PrivoznikAfter we've dropped privileges it might be not possible to write pidfile. For instance, if this binary is run as root (because user wants it to write pidfile to some privileged location) writing pidfile fails because privileges are dropped before we even get to that. Signed-off-by: Michal Privoznik Signed-off-by: Paolo Bonzini --- scsi/qemu-pr-helper.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index 21e1b8e..eeff80a 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -1081,13 +1081,6 @@ int main(int argc, char **argv) accept_client, NULL, NULL); -#ifdef CONFIG_LIBCAP -if (drop_privileges() < 0) { -error_report("Failed to drop privileges: %s", strerror(errno)); -exit(EXIT_FAILURE); -} -#endif - if (daemonize) { if (daemon(0, 0) < 0) { error_report("Failed to daemonize: %s", strerror(errno)); @@ -1096,6 +1089,13 @@ int main(int argc, char **argv) write_pidfile(); } +#ifdef CONFIG_LIBCAP +if (drop_privileges() < 0) { +error_report("Failed to drop privileges: %s", strerror(errno)); +exit(EXIT_FAILURE); +} +#endif + state = RUNNING; do { main_loop_wait(false); -- 1.8.3.1
[Qemu-devel] [PULL 17/20] device-crash-test: Remove fixed isa-fdc entry
From: Thomas HuthFixed by commit b3da551 ("fdc: Exit if ISA controller does not support DMA", 2018-03-16). Signed-off-by: Thomas Huth Signed-off-by: Paolo Bonzini --- scripts/device-crash-test | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/device-crash-test b/scripts/device-crash-test index 24c7bf5..5d17dc6 100755 --- a/scripts/device-crash-test +++ b/scripts/device-crash-test @@ -217,7 +217,6 @@ ERROR_WHITELIST = [ {'exitcode':-6, 'log':r"Object .* is not an instance of type generic-pc-machine", 'loglevel':logging.ERROR}, {'exitcode':-6, 'log':r"Object .* is not an instance of type e500-ccsr", 'loglevel':logging.ERROR}, {'exitcode':-6, 'log':r"vmstate_register_with_alias_id: Assertion `!se->compat \|\| se->instance_id == 0' failed", 'loglevel':logging.ERROR}, -{'exitcode':-6, 'device':'isa-fdc', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'isa-serial', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'mioe3680_pci', 'loglevel':logging.ERROR, 'expected':True}, {'exitcode':-11, 'device':'pcm3680_pci', 'loglevel':logging.ERROR, 'expected':True}, -- 1.8.3.1
[Qemu-devel] [PULL 14/20] virtio-serial: fix heapover-flow
From: linzhechengCheck device having the feature of VIRTIO_CONSOLE_F_EMERG_WRITE before get config->emerg_wr. It is neccessary because sizeof(virtio_console_config) is 8 byte if VirtIOSerial doesn't have the feature of VIRTIO_CONSOLE_F_EMERG_WRITE(see virtio_serial_device_realize), read/write emerg_wr will lead to heap-over-flow. Signed-off-by: linzhecheng Message-Id: <20180328133435.20112-1-linzhech...@huawei.com> Signed-off-by: Paolo Bonzini --- hw/char/virtio-serial-bus.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 9470bd7..d2dd8ab 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -580,13 +580,16 @@ static void set_config(VirtIODevice *vdev, const uint8_t *config_data) VirtIOSerial *vser = VIRTIO_SERIAL(vdev); struct virtio_console_config *config = (struct virtio_console_config *)config_data; -uint8_t emerg_wr_lo = le32_to_cpu(config->emerg_wr); VirtIOSerialPort *port = find_first_connected_console(vser); VirtIOSerialPortClass *vsc; +uint8_t emerg_wr_lo; -if (!config->emerg_wr) { +if (!virtio_has_feature(vser->host_features, +VIRTIO_CONSOLE_F_EMERG_WRITE) || !config->emerg_wr) { return; } + +emerg_wr_lo = le32_to_cpu(config->emerg_wr); /* Make sure we don't misdetect an emergency write when the guest * does a short config write after an emergency write. */ config->emerg_wr = 0; -- 1.8.3.1
[Qemu-devel] [PULL 19/20] maint: Add .mailmap entries for patches claiming list authorship
From: Eric BlakeThe list did not author any patches, but it does rewrite the 'From:' header of messages sent from any domain with restrictive SPF policies that would otherwise prevent the message from reaching all list recipients. If a maintainer is not careful to undo the list header rewrite, and the author did not include a manual 'From:' line in the body to fix the munged header, then 'git am' happily attributes the patch to the list. Add some mailmap entries to correct the few that have escaped our attention; while we also work on improving the tooling to catch the problem in the future before a merge is even made. Also improve the comments occurring in the file, including line length improvements. Signed-off-by: Eric Blake Signed-off-by: Paolo Bonzini --- .mailmap | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/.mailmap b/.mailmap index cf689b9..778a4d4 100644 --- a/.mailmap +++ b/.mailmap @@ -1,6 +1,7 @@ -# This mailmap just translates the weird addresses from the original import into git -# into proper addresses so that they are counted properly in git shortlog output. -# +# This mailmap fixes up author names/addresses. + +# The first section translates weird addresses from the original git import +# into proper addresses so that they are counted properly by git shortlog. Andrzej Zaborowski balrog Anthony Liguori aliguori Anthony Liguori Anthony Liguori @@ -15,10 +16,19 @@ Paul Burton Paul Burton Thiemo Seufer ths malc malc + # There is also a: #(no author) <(no author)@c046a42c-6fe2-441c-8c8c-71466251a162> # for the cvs2svn initialization commit e63c3dc74bf. -# + +# Next, translate a few commits where mailman rewrote the From: line due +# to strict SPF, although we prefer to avoid adding more entries like that. +Ed Swierk Ed Swierk via Qemu-devel +Ian McKellar Ian McKellar via Qemu-devel +Julia Suvorova Julia Suvorova via Qemu-devel +Justin Terry (VM) Justin Terry (VM) via Qemu-devel + + # Also list preferred name forms where people have changed their # git author config Daniel P. Berrangé -- 1.8.3.1
[Qemu-devel] [PULL 16/20] qemu-pr-helper: Write pidfile more often
From: Michal PrivoznikLet's write pidfile even if user did not request --daemon but they requested just --pidfile. Libvirt will use exactly this. Signed-off-by: Michal Privoznik Signed-off-by: Paolo Bonzini --- scsi/qemu-pr-helper.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c index eeff80a..d0f8317 100644 --- a/scsi/qemu-pr-helper.c +++ b/scsi/qemu-pr-helper.c @@ -924,6 +924,7 @@ int main(int argc, char **argv) Error *local_err = NULL; char *trace_file = NULL; bool daemonize = false; +bool pidfile_specified = false; unsigned socket_activation; struct sigaction sa_sigterm; @@ -954,6 +955,7 @@ int main(int argc, char **argv) case 'f': g_free(pidfile); pidfile = g_strdup(optarg); +pidfile_specified = true; break; #ifdef CONFIG_LIBCAP case 'u': { @@ -1086,9 +1088,11 @@ int main(int argc, char **argv) error_report("Failed to daemonize: %s", strerror(errno)); exit(EXIT_FAILURE); } -write_pidfile(); } +if (daemonize || pidfile_specified) +write_pidfile(); + #ifdef CONFIG_LIBCAP if (drop_privileges() < 0) { error_report("Failed to drop privileges: %s", strerror(errno)); -- 1.8.3.1
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
On 6 April 2018 at 18:06,wrote: > "Peter Maydell" wrote on 04/06/2018 09:51:55 > AM: > >> I've now done this, and can reproduce the problem. So the >> issue is generic to 32-bit hosts. > > Supporting evidence: I compiled Cygwin/MINGW with x86_64 and the > resulting binaries work properly > > FWIW, the compile had some anomalous behavior: > > .../qemu-2.12.0-rc2/scripts/feature_to_c.sh: line 71: /usr/bin/sed: > Invalid argument > > line 71 is: > arrayname=xml_feature_$(echo $input | sed 's,.*/,,; s/[-.]/_/g') > > Not sure what the problem is, but I always use "-e" before scripts. That's a bit odd. (POSIX allows both "-e script" and just "script".) What version of sed is this, and what version of /bin/sh ? thanks -- PMM
[Qemu-devel] [PULL 07/20] i386/hyperv: error out if features requested but unsupported
From: Roman KaganIn order to guarantee compatibility on migration, QEMU should have complete control over the features it announces to the guest via CPUID. However, for a number of Hyper-V-related cpu properties, if the corresponding feature is not supported by the underlying KVM, the propery is silently ignored and the feature is not announced to the guest. Refuse to start with an error instead. Signed-off-by: Roman Kagan Message-Id: <20180330170209.20627-3-rka...@virtuozzo.com> Reviewed-by: Eduardo Habkost Signed-off-by: Paolo Bonzini --- target/i386/kvm.c | 43 ++- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index b35623a..6c49954 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -632,11 +632,6 @@ static int hyperv_handle_properties(CPUState *cs) X86CPU *cpu = X86_CPU(cs); CPUX86State *env = >env; -if (cpu->hyperv_time && -kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) { -cpu->hyperv_time = false; -} - if (cpu->hyperv_relaxed_timing) { env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE; } @@ -645,6 +640,12 @@ static int hyperv_handle_properties(CPUState *cs) env->features[FEAT_HYPERV_EAX] |= HV_APIC_ACCESS_AVAILABLE; } if (cpu->hyperv_time) { +if (kvm_check_extension(cs->kvm_state, KVM_CAP_HYPERV_TIME) <= 0) { +fprintf(stderr, "Hyper-V clocksources " +"(requested by 'hv-time' cpu flag) " +"are not supported by kernel\n"); +return -ENOSYS; +} env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE; env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE; env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE; @@ -659,17 +660,41 @@ static int hyperv_handle_properties(CPUState *cs) env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; } -if (cpu->hyperv_crash && has_msr_hv_crash) { +if (cpu->hyperv_crash) { +if (!has_msr_hv_crash) { +fprintf(stderr, "Hyper-V crash MSRs " +"(requested by 'hv-crash' cpu flag) " +"are not supported by kernel\n"); +return -ENOSYS; +} env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE; } env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE; -if (cpu->hyperv_reset && has_msr_hv_reset) { +if (cpu->hyperv_reset) { +if (!has_msr_hv_reset) { +fprintf(stderr, "Hyper-V reset MSR " +"(requested by 'hv-reset' cpu flag) " +"is not supported by kernel\n"); +return -ENOSYS; +} env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE; } -if (cpu->hyperv_vpindex && has_msr_hv_vpindex) { +if (cpu->hyperv_vpindex) { +if (!has_msr_hv_vpindex) { +fprintf(stderr, "Hyper-V VP_INDEX MSR " +"(requested by 'hv-vpindex' cpu flag) " +"is not supported by kernel\n"); +return -ENOSYS; +} env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE; } -if (cpu->hyperv_runtime && has_msr_hv_runtime) { +if (cpu->hyperv_runtime) { +if (!has_msr_hv_runtime) { +fprintf(stderr, "Hyper-V VP_RUNTIME MSR " +"(requested by 'hv-runtime' cpu flag) " +"is not supported by kernel\n"); +return -ENOSYS; +} env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE; } if (cpu->hyperv_synic) { -- 1.8.3.1
[Qemu-devel] [PULL 12/20] hw/dma/i82374: Avoid double creation of the 82374 controller
From: Philippe Mathieu-DaudéQEMU fails when used with the following command line: ./ppc64-softmmu/qemu-system-ppc64 -S -machine 40p -device i82374 qemu-system-ppc64: hw/isa/isa-bus.c:110: isa_bus_dma: Assertion `!bus->dma[0] && !bus->dma[1]' failed. The 40p machine type already creates the device i82374. If specified in the command line, it will try to create it again, hence generating the error. The function isa_bus_dma() isn't supposed to be called twice for the same bus. Check the bus doesn't already have a DMA controller registered before creating the device. Fixes: https://bugs.launchpad.net/qemu/+bug/1721224 Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20180326153441.32641-2-f4...@amsat.org> Signed-off-by: Paolo Bonzini --- hw/dma/i82374.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index 83c87d9..892f655 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -23,6 +23,7 @@ */ #include "qemu/osdep.h" +#include "qapi/error.h" #include "hw/isa/isa.h" #include "hw/dma/i8257.h" @@ -118,13 +119,19 @@ static const MemoryRegionPortio i82374_portio_list[] = { static void i82374_realize(DeviceState *dev, Error **errp) { I82374State *s = I82374(dev); +ISABus *isa_bus = isa_bus_from_device(ISA_DEVICE(dev)); + +if (isa_get_dma(isa_bus, 0)) { +error_setg(errp, "DMA already initialized on ISA bus"); +return; +} +i8257_dma_init(isa_bus, true); portio_list_init(>port_list, OBJECT(s), i82374_portio_list, s, "i82374"); portio_list_add(>port_list, isa_address_space_io(>parent_obj), s->iobase); -i8257_dma_init(isa_bus_from_device(ISA_DEVICE(dev)), true); memset(s->commands, 0, sizeof(s->commands)); } -- 1.8.3.1
[Qemu-devel] [PULL 11/20] hw/scsi: support SCSI-2 passthrough without PI
From: Daniel Henrique BarbozaQEMU SCSI code makes assumptions about how the PROTECT and BYTCHK works in the protocol, denying support for PI (Protection Information) in case the guest OS requests it. However, in SCSI versions 2 and older, there is no PI concept in the protocol. This means that when dealing with such devices: - there is no PROTECT bit in byte 5 of the standard INQUIRY response. The whole byte is marked as "Reserved"; - there is no RDPROTECT in byte 2 of READ. We have 'Logical Unit Number' in this field instead; - there is no VRPROTECT in byte 2 of VERIFY. We have 'Logical Unit Number' in this field instead. This also means that the BYTCHK bit in this case is not related to PI. Since QEMU does not consider these changes, a SCSI passthrough using a SCSI-2 device will not work. It will mistake these fields with PI information and return Illegal Request SCSI SENSE thinking that the driver is asking for PI support. This patch fixes it by adding a new attribute called 'scsi_version' that is read from the standard INQUIRY response of passthrough devices. This allows for a version verification before applying conditions related to PI that doesn't apply for older versions. Reported-by: Dac Nguyen Signed-off-by: Daniel Henrique Barboza Message-Id: <20180327211451.14647-1-danie...@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c| 2 +- hw/scsi/scsi-generic.c | 47 --- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 9400b97..ded23d3 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -3041,7 +3041,7 @@ static Property scsi_block_properties[] = { DEFINE_PROP_BOOL("share-rw", SCSIDiskState, qdev.conf.share_rw, false), DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0), DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version, - 5), + -1), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c index 1870085..381f04e 100644 --- a/hw/scsi/scsi-generic.c +++ b/hw/scsi/scsi-generic.c @@ -194,17 +194,40 @@ static void scsi_read_complete(void * opaque, int ret) r->buf[3] |= 0x80; } } -if (s->type == TYPE_DISK && -r->req.cmd.buf[0] == INQUIRY && -r->req.cmd.buf[2] == 0xb0) { -uint32_t max_transfer = -blk_get_max_transfer(s->conf.blk) / s->blocksize; - -assert(max_transfer); -stl_be_p(>buf[8], max_transfer); -/* Also take care of the opt xfer len. */ -stl_be_p(>buf[12], - MIN_NON_ZERO(max_transfer, ldl_be_p(>buf[12]))); +if (r->req.cmd.buf[0] == INQUIRY) { +/* + * EVPD set to zero returns the standard INQUIRY data. + * + * Check if scsi_version is unset (-1) to avoid re-defining it + * each time an INQUIRY with standard data is received. + * scsi_version is initialized with -1 in scsi_generic_reset + * and scsi_disk_reset, making sure that we'll set the + * scsi_version after a reset. If the version field of the + * INQUIRY response somehow changes after a guest reboot, + * we'll be able to keep track of it. + * + * On SCSI-2 and older, first 3 bits of byte 2 is the + * ANSI-approved version, while on later versions the + * whole byte 2 contains the version. Check if we're dealing + * with a newer version and, in that case, assign the + * whole byte. + */ +if (s->scsi_version == -1 && !(r->req.cmd.buf[1] & 0x01)) { +s->scsi_version = r->buf[2] & 0x07; +if (s->scsi_version > 2) { +s->scsi_version = r->buf[2]; +} +} +if (s->type == TYPE_DISK && r->req.cmd.buf[2] == 0xb0) { +uint32_t max_transfer = +blk_get_max_transfer(s->conf.blk) / s->blocksize; + +assert(max_transfer); +stl_be_p(>buf[8], max_transfer); +/* Also take care of the opt xfer len. */ +stl_be_p(>buf[12], + MIN_NON_ZERO(max_transfer, ldl_be_p(>buf[12]))); +} } scsi_req_data(>req, len); scsi_req_unref(>req); @@ -550,6 +573,8 @@ static void scsi_generic_realize(SCSIDevice *s, Error **errp) DPRINTF("block size %d\n", s->blocksize); +/* Only used by scsi-block, but initialize it nevertheless to be clean. */ +s->default_scsi_version = -1; scsi_generic_read_device_identification(s); } -- 1.8.3.1
[Qemu-devel] [PULL 10/20] scsi-disk: allow customizing the SCSI version
We would like to have different behavior for passthrough devices depending on the SCSI version they expose. To prepare for that, allow the user of emulated devices to specify the desired SCSI level, and adjust the emulation according to the property value. The next patch will set the level for scsi-block and scsi-generic devices. Based on a patch by Daniel Henrique Barboza. Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c| 29 - hw/scsi/scsi-generic.c | 1 + include/hw/scsi/scsi.h | 2 ++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index f8ed8cf..9400b97 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -825,7 +825,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) * block characteristics VPD page by default. Not all of SPC-3 * is actually implemented, but we're good enough. */ -outbuf[2] = 5; +outbuf[2] = s->qdev.default_scsi_version; outbuf[3] = 2 | 0x10; /* Format 2, HiSup */ if (buflen > 36) { @@ -2193,7 +2193,11 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) case READ_12: case READ_16: DPRINTF("Read (sector %" PRId64 ", count %u)\n", r->req.cmd.lba, len); -if (r->req.cmd.buf[1] & 0xe0) { +/* Protection information is not supported. For SCSI versions 2 and + * older (as determined by snooping the guest's INQUIRY commands), + * there is no RD/WR/VRPROTECT, so skip this check in these versions. + */ +if (s->qdev.scsi_version > 2 && (r->req.cmd.buf[1] & 0xe0)) { goto illegal_request; } if (!check_lba_range(s, r->req.cmd.lba, len)) { @@ -2224,7 +2228,7 @@ static int32_t scsi_disk_dma_command(SCSIRequest *req, uint8_t *buf) * As far as DMA is concerned, we can treat it the same as a write; * scsi_block_do_sgio will send VERIFY commands. */ -if (r->req.cmd.buf[1] & 0xe0) { +if (s->qdev.scsi_version > 2 && (r->req.cmd.buf[1] & 0xe0)) { goto illegal_request; } if (!check_lba_range(s, r->req.cmd.lba, len)) { @@ -2270,6 +2274,8 @@ static void scsi_disk_reset(DeviceState *dev) /* reset tray statuses */ s->tray_locked = 0; s->tray_open = 0; + +s->qdev.scsi_version = s->qdev.default_scsi_version; } static void scsi_disk_resize_cb(void *opaque) @@ -2814,6 +2820,8 @@ static bool scsi_block_is_passthrough(SCSIDiskState *s, uint8_t *buf) static int32_t scsi_block_dma_command(SCSIRequest *req, uint8_t *buf) { SCSIBlockReq *r = (SCSIBlockReq *)req; +SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev); + r->cmd = req->cmd.buf[0]; switch (r->cmd >> 5) { case 0: @@ -2839,8 +2847,11 @@ static int32_t scsi_block_dma_command(SCSIRequest *req, uint8_t *buf) abort(); } -if (r->cdb1 & 0xe0) { -/* Protection information is not supported. */ +/* Protection information is not supported. For SCSI versions 2 and + * older (as determined by snooping the guest's INQUIRY commands), + * there is no RD/WR/VRPROTECT, so skip this check in these versions. + */ +if (s->qdev.scsi_version > 2 && (req->cmd.buf[1] & 0xe0)) { scsi_check_condition(>req, SENSE_CODE(INVALID_FIELD)); return 0; } @@ -2952,6 +2963,8 @@ static Property scsi_hd_properties[] = { DEFINE_PROP_UINT64("max_io_size", SCSIDiskState, max_io_size, DEFAULT_MAX_IO_SIZE), DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0), +DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version, + 5), DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf), DEFINE_PROP_END_OF_LIST(), }; @@ -2997,6 +3010,8 @@ static Property scsi_cd_properties[] = { DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0), DEFINE_PROP_UINT64("max_io_size", SCSIDiskState, max_io_size, DEFAULT_MAX_IO_SIZE), +DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version, + 5), DEFINE_PROP_END_OF_LIST(), }; @@ -3025,6 +3040,8 @@ static Property scsi_block_properties[] = { DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk), DEFINE_PROP_BOOL("share-rw", SCSIDiskState, qdev.conf.share_rw, false), DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0), +DEFINE_PROP_INT32("scsi_version", SCSIDiskState, qdev.default_scsi_version, + 5), DEFINE_PROP_END_OF_LIST(), }; @@ -3065,6 +3082,8 @@ static Property scsi_disk_properties[] = { DEFAULT_MAX_UNMAP_SIZE), DEFINE_PROP_UINT64("max_io_size", SCSIDiskState, max_io_size, DEFAULT_MAX_IO_SIZE), +
[Qemu-devel] [PULL 09/20] scsi-disk: Don't enlarge min_io_size to max_io_size
From: Fam ZhengSome backends report big max_io_sectors. Making min_io_size the same value in this case will make it impossible for guest to align memory, therefore the disk may not be usable at all. Do not enlarge them when they are zero. Reported-by: David Gibson Signed-off-by: Fam Zheng Message-Id: <20180327164141.19075-1-f...@redhat.com> Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-disk.c | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index f5ab767..f8ed8cf 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -714,10 +714,12 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) /* min_io_size and opt_io_size can't be greater than * max_io_sectors */ -min_io_size = -MIN_NON_ZERO(min_io_size, max_io_sectors); -opt_io_size = -MIN_NON_ZERO(opt_io_size, max_io_sectors); +if (min_io_size) { +min_io_size = MIN(min_io_size, max_io_sectors); +} +if (opt_io_size) { +opt_io_size = MIN(opt_io_size, max_io_sectors); +} } /* required VPD size with unmap support */ buflen = 0x40; -- 1.8.3.1
[Qemu-devel] [PULL 04/20] memfd: fix vhost-user-test on non-memfd capable host
From: Marc-André LureauOn RHEL7, memfd is not supported, and vhost-user-test fails: TEST: tests/vhost-user-test... (pid=10248) /x86_64/vhost-user/migrate: qemu-system-x86_64: -object memory-backend-memfd,id=mem,size=2M,: failed to create memfd FAIL There is a qemu_memfd_check() to prevent running memfd path, but it also checks for fallback implementation. Let's specialize qemu_memfd_check() to check memfd only, while qemu_memfd_alloc_check() checks for the qemu_memfd_alloc() API. Reported-by: Miroslav Rezanina Tested-by: Miroslav Rezanina Signed-off-by: Marc-André Lureau Message-Id: <20180328121804.16203-1-marcandre.lur...@redhat.com> Signed-off-by: Paolo Bonzini --- hw/virtio/vhost.c| 2 +- include/qemu/memfd.h | 1 + util/memfd.c | 30 +- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 250f886..27c1ec5 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -1223,7 +1223,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) { error_setg(>migration_blocker, "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature."); -} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_check()) { +} else if (vhost_dev_log_is_shared(hdev) && !qemu_memfd_alloc_check()) { error_setg(>migration_blocker, "Migration disabled: failed to allocate shared memory"); } diff --git a/include/qemu/memfd.h b/include/qemu/memfd.h index de10198..49e7963 100644 --- a/include/qemu/memfd.h +++ b/include/qemu/memfd.h @@ -18,6 +18,7 @@ int qemu_memfd_create(const char *name, size_t size, bool hugetlb, uint64_t hugetlbsize, unsigned int seals, Error **errp); +bool qemu_memfd_alloc_check(void); void *qemu_memfd_alloc(const char *name, size_t size, unsigned int seals, int *fd, Error **errp); void qemu_memfd_free(void *ptr, size_t size, int fd); diff --git a/util/memfd.c b/util/memfd.c index 07d579e..277f721 100644 --- a/util/memfd.c +++ b/util/memfd.c @@ -173,7 +173,13 @@ enum { MEMFD_TODO }; -bool qemu_memfd_check(void) +/** + * qemu_memfd_alloc_check(): + * + * Check if qemu_memfd_alloc() can allocate, including using a + * fallback implementation when host doesn't support memfd. + */ +bool qemu_memfd_alloc_check(void) { static int memfd_check = MEMFD_TODO; @@ -188,3 +194,25 @@ bool qemu_memfd_check(void) return memfd_check == MEMFD_OK; } + +/** + * qemu_memfd_check(): + * + * Check if host supports memfd. + */ +bool qemu_memfd_check(void) +{ +static int memfd_check = MEMFD_TODO; + +if (memfd_check == MEMFD_TODO) { +int mfd = memfd_create("test", 0); +if (mfd >= 0) { +memfd_check = MEMFD_OK; +close(mfd); +} else { +memfd_check = MEMFD_KO; +} +} + +return memfd_check == MEMFD_OK; +} -- 1.8.3.1
[Qemu-devel] [PULL 06/20] i386/hyperv: add hv-frequencies cpu property
From: Roman KaganIn order to guarantee compatibility on migration, QEMU should have complete control over the features it announces to the guest via CPUID. However, the availability of Hyper-V frequency MSRs (HV_X64_MSR_TSC_FREQUENCY and HV_X64_MSR_APIC_FREQUENCY) depends solely on the support for them in the underlying KVM. Introduce "hv-frequencies" cpu property (off by default) which gives QEMU full control over whether these MSRs are announced. While at this, drop the redundant check of the cpu tsc frequency, and decouple this feature from hv-time. Signed-off-by: Roman Kagan Reviewed-by: Eduardo Habkost Message-Id: <20180330170209.20627-2-rka...@virtuozzo.com> Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 1 + target/i386/cpu.h | 1 + target/i386/kvm.c | 13 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 555ae79..1a6b082 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4761,6 +4761,7 @@ static Property x86_cpu_properties[] = { DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false), DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false), DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false), +DEFINE_PROP_BOOL("hv-frequencies", X86CPU, hyperv_frequencies, false), DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true), DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false), DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true), diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 78db1b8..1b219fa 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1296,6 +1296,7 @@ struct X86CPU { bool hyperv_runtime; bool hyperv_synic; bool hyperv_stimer; +bool hyperv_frequencies; bool check_cpuid; bool enforce_cpuid; bool expose_kvm; diff --git a/target/i386/kvm.c b/target/i386/kvm.c index d23fff1..b35623a 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -648,11 +648,16 @@ static int hyperv_handle_properties(CPUState *cs) env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE; env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE; env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE; - -if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) { -env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; -env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; +} +if (cpu->hyperv_frequencies) { +if (!has_msr_hv_frequencies) { +fprintf(stderr, "Hyper-V frequency MSRs " +"(requested by 'hv-frequencies' cpu flag) " +"are not supported by kernel\n"); +return -ENOSYS; } +env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS; +env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE; } if (cpu->hyperv_crash && has_msr_hv_crash) { env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE; -- 1.8.3.1
[Qemu-devel] [PULL 13/20] kvmclock: fix clock_is_reliable on migration from QEMU < 2.9
From: Michael ChapmanWhen migrating from a pre-2.9 QEMU, no clock_is_reliable flag is transferred. We should assume that the source host has an unreliable KVM_GET_CLOCK, rather than using whatever was determined locally, to ensure that any drift from the TSC-based value calculated by the guest is corrected. Signed-off-by: Michael Chapman Message-Id: <20180406053406.774-1-m...@very.puzzling.org> Signed-off-by: Paolo Bonzini --- hw/i386/kvm/clock.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/hw/i386/kvm/clock.c b/hw/i386/kvm/clock.c index 1707434..7dac319 100644 --- a/hw/i386/kvm/clock.c +++ b/hw/i386/kvm/clock.c @@ -242,6 +242,19 @@ static const VMStateDescription kvmclock_reliable_get_clock = { }; /* + * When migrating, assume the source has an unreliable + * KVM_GET_CLOCK unless told otherwise. + */ +static int kvmclock_pre_load(void *opaque) +{ +KVMClockState *s = opaque; + +s->clock_is_reliable = false; + +return 0; +} + +/* * When migrating, read the clock just before migration, * so that the guest clock counts during the events * between: @@ -268,6 +281,7 @@ static const VMStateDescription kvmclock_vmsd = { .name = "kvmclock", .version_id = 1, .minimum_version_id = 1, +.pre_load = kvmclock_pre_load, .pre_save = kvmclock_pre_save, .fields = (VMStateField[]) { VMSTATE_UINT64(clock, KVMClockState), -- 1.8.3.1
[Qemu-devel] [PULL 02/20] target/i386: Fix andn instruction
From: Alexandro Sanchez BachIn commit 7073fbada733c8d10992f00772c9b9299d740e9b, the `andn` instruction was implemented via `tcg_gen_andc` but passes the operands in the wrong order: - X86 defines `andn dest,src1,src2` as: dest = ~src1 & src2 - TCG defines `andc dest,src1,src2` as: dest = src1 & ~src2 The following simple test shows the issue: #include #include int main(void) { uint32_t ret = 0; __asm ( "mov $0xFF00, %%ecx\n" "mov $0x0F0F, %%eax\n" "andn %%ecx, %%eax, %%ecx\n" "mov %%ecx, %0\n" : "=r" (ret)); printf("%08X\n", ret); return 0; } This patch fixes the problem by simply swapping the order of the two last arguments in `tcg_gen_andc_tl`. Reported-by: Alexandro Sanchez Bach Signed-off-by: Alexandro Sanchez Bach Cc: qemu-sta...@nongnu.org Signed-off-by: Paolo Bonzini --- target/i386/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 0135415..3b7ce92 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -3802,7 +3802,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b, } ot = mo_64_32(s->dflag); gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0); -tcg_gen_andc_tl(cpu_T0, cpu_regs[s->vex_v], cpu_T0); +tcg_gen_andc_tl(cpu_T0, cpu_T0, cpu_regs[s->vex_v]); gen_op_mov_reg_v(ot, reg, cpu_T0); gen_op_update1_cc(); set_cc_op(s, CC_OP_LOGICB + ot); -- 1.8.3.1
[Qemu-devel] [PULL 08/20] configure: Add missing configure options to help text
From: Thomas HuthWe forgot to mention --with-git, --libexecdir and --with-pkgversion so far. Signed-off-by: Thomas Huth Message-Id: <1522163370-18544-1-git-send-email-th...@redhat.com> Reviewed-by: Eric Blake Signed-off-by: Paolo Bonzini --- configure | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure b/configure index a2301dd..752dd9e 100755 --- a/configure +++ b/configure @@ -1497,16 +1497,19 @@ Advanced options (experts only): --install=INSTALLuse specified install [$install] --python=PYTHON use specified python [$python] --smbd=SMBD use specified smbd [$smbd] + --with-git=GIT use specified git [$git] --static enable static build [$static] --mandir=PATHinstall man pages in PATH --datadir=PATH install firmware in PATH$confsuffix --docdir=PATHinstall documentation in PATH$confsuffix --bindir=PATHinstall binaries in PATH --libdir=PATHinstall libraries in PATH + --libexecdir=PATHinstall helper binaries in PATH --sysconfdir=PATHinstall config in PATH$confsuffix --localstatedir=PATH install local state in PATH (set at runtime on win32) --firmwarepath=PATH search PATH for firmware files --with-confsuffix=SUFFIX suffix for QEMU data inside datadir/libdir/sysconfdir [$confsuffix] + --with-pkgversion=VERS use specified string as sub-version of the package --enable-debug enable common debug build options --enable-sanitizers enable default sanitizers --disable-strip disable stripping binaries -- 1.8.3.1
[Qemu-devel] [PULL 03/20] scripts/checkpatch.pl: Bug fix
From: Su HangCommit 2b9aef6fcd96ba7ed8c1ee723e391901852d344c introduced a regression: checkpatch.pl started complaining about the following valid pattern: do { /* something */ } while (condition); Fix the script to once again permit this pattern. Signed-off-by: Su Hang Message-Id: <1522029982-4650-1-git-send-email-suhan...@mails.ucas.ac.cn> Signed-off-by: Paolo Bonzini --- scripts/checkpatch.pl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 57daae0..d52207a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2356,6 +2356,18 @@ sub process { # check for missing bracing around if etc if ($line =~ /(^.*)\b(?:if|while|for)\b/ && $line !~ /\#\s*if/) { + my $allowed = 0; + + # Check the pre-context. + if ($line =~ /(\}.*?)$/) { + my $pre = $1; + + if ($line !~ /else/) { + print "APW: ALLOWED: pre<$pre> line<$line>\n" + if $dbg_adv_apw; + $allowed = 1; + } + } my ($level, $endln, @chunks) = ctx_statement_full($linenr, $realcnt, 1); if ($dbg_adv_apw) { @@ -2364,7 +2376,6 @@ sub process { if $#chunks >= 1; } if ($#chunks >= 0 && $level == 0) { - my $allowed = 0; my $seen = 0; my $herectx = $here . "\n"; my $ln = $linenr - 1; @@ -2408,7 +2419,7 @@ sub process { $allowed = 1; } } - if ($seen != ($#chunks + 1)) { + if ($seen != ($#chunks + 1) && !$allowed) { ERROR("braces {} are necessary for all arms of this statement\n" . $herectx); } } -- 1.8.3.1
[Qemu-devel] [PULL 00/20] Miscellaneous patches for QEMU 2.12-rc
The following changes since commit 0e87fdc966d05f4e5ad868034fcd8ee2a08ca62d: Update version for v2.12.0-rc2 release (2018-04-04 20:37:20 +0100) are available in the git repository at: git://github.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 0af74e1178c1f4ba495b499b2ba2a8c7c453b72a: Add missing bit for SSE instr in VEX decoding (2018-04-06 18:42:49 +0200) Miscellaneous bugfixes, including crash fixes from Alexey, Peter M. and Thomas. Alexandro Sanchez Bach (1): target/i386: Fix andn instruction Bruce Rogers (1): sys_membarrier: fix up include directives Daniel Henrique Barboza (1): hw/scsi: support SCSI-2 passthrough without PI Eric Blake (2): dump: Fix build with newer gcc maint: Add .mailmap entries for patches claiming list authorship Eugene Minibaev (1): Add missing bit for SSE instr in VEX decoding Fam Zheng (1): scsi-disk: Don't enlarge min_io_size to max_io_size Justin Terry (VM) (1): target/i386: WHPX: set CPUID_EXT_HYPERVISOR bit Marc-André Lureau (1): memfd: fix vhost-user-test on non-memfd capable host Michael Chapman (1): kvmclock: fix clock_is_reliable on migration from QEMU < 2.9 Michal Privoznik (2): qemu-pr-helper: Daemonize before dropping privileges qemu-pr-helper: Write pidfile more often Paolo Bonzini (1): scsi-disk: allow customizing the SCSI version Philippe Mathieu-Daudé (1): hw/dma/i82374: Avoid double creation of the 82374 controller Roman Kagan (2): i386/hyperv: add hv-frequencies cpu property i386/hyperv: error out if features requested but unsupported Su Hang (1): scripts/checkpatch.pl: Bug fix Thomas Huth (2): configure: Add missing configure options to help text device-crash-test: Remove fixed isa-fdc entry linzhecheng (1): virtio-serial: fix heapover-flow .mailmap| 18 --- configure | 3 ++ dump.c | 4 +-- hw/char/virtio-serial-bus.c | 7 ++-- hw/dma/i82374.c | 9 +- hw/i386/kvm/clock.c | 14 hw/scsi/scsi-disk.c | 39 -- hw/scsi/scsi-generic.c | 48 --- hw/virtio/vhost.c | 2 +- include/hw/scsi/scsi.h | 2 ++ include/qemu/memfd.h| 1 + scripts/checkpatch.pl | 15 +++-- scripts/device-crash-test | 1 - scsi/qemu-pr-helper.c | 18 +++ target/i386/cpu.c | 1 + target/i386/cpu.h | 1 + target/i386/kvm.c | 56 target/i386/translate.c | 6 ++-- target/i386/whpx-all.c | 79 - util/memfd.c| 30 - util/sys_membarrier.c | 6 ++-- 21 files changed, 300 insertions(+), 60 deletions(-) -- 1.8.3.1
[Qemu-devel] [PULL 05/20] target/i386: WHPX: set CPUID_EXT_HYPERVISOR bit
From: "Justin Terry (VM)"Implements the CPUID trap for CPUID 1 to include the CPUID_EXT_HYPERVISOR flag in the ECX results. This was preventing some older linux kernels from booting when trying to access MSR's that dont make sense when virtualized. Signed-off-by: Justin Terry (VM) Message-Id: <20180326170658.606-1-jute...@microsoft.com> Signed-off-by: Paolo Bonzini --- target/i386/whpx-all.c | 79 +- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index bf33d32..5843517 100644 --- a/target/i386/whpx-all.c +++ b/target/i386/whpx-all.c @@ -911,12 +911,62 @@ static int whpx_vcpu_run(CPUState *cpu) ret = 1; break; +case WHvRunVpExitReasonX64Cpuid: { +WHV_REGISTER_VALUE reg_values[5] = {0}; +WHV_REGISTER_NAME reg_names[5]; +UINT32 reg_count = 5; +UINT64 rip, rax, rcx, rdx, rbx; + +rip = vcpu->exit_ctx.VpContext.Rip + + vcpu->exit_ctx.VpContext.InstructionLength; +switch (vcpu->exit_ctx.CpuidAccess.Rax) { +case 1: +rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax; +/* Advertise that we are running on a hypervisor */ +rcx = +vcpu->exit_ctx.CpuidAccess.DefaultResultRcx | +CPUID_EXT_HYPERVISOR; + +rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx; +rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx; +break; +default: +rax = vcpu->exit_ctx.CpuidAccess.DefaultResultRax; +rcx = vcpu->exit_ctx.CpuidAccess.DefaultResultRcx; +rdx = vcpu->exit_ctx.CpuidAccess.DefaultResultRdx; +rbx = vcpu->exit_ctx.CpuidAccess.DefaultResultRbx; +} + +reg_names[0] = WHvX64RegisterRip; +reg_names[1] = WHvX64RegisterRax; +reg_names[2] = WHvX64RegisterRcx; +reg_names[3] = WHvX64RegisterRdx; +reg_names[4] = WHvX64RegisterRbx; + +reg_values[0].Reg64 = rip; +reg_values[1].Reg64 = rax; +reg_values[2].Reg64 = rcx; +reg_values[3].Reg64 = rdx; +reg_values[4].Reg64 = rbx; + +hr = WHvSetVirtualProcessorRegisters(whpx->partition, + cpu->cpu_index, + reg_names, + reg_count, + reg_values); + +if (FAILED(hr)) { +error_report("WHPX: Failed to set CpuidAccess state registers," + " hr=%08lx", hr); +} +ret = 0; +break; +} case WHvRunVpExitReasonNone: case WHvRunVpExitReasonUnrecoverableException: case WHvRunVpExitReasonInvalidVpRegisterValue: case WHvRunVpExitReasonUnsupportedFeature: case WHvRunVpExitReasonX64MsrAccess: -case WHvRunVpExitReasonX64Cpuid: case WHvRunVpExitReasonException: default: error_report("WHPX: Unexpected VP exit code %d", @@ -1272,6 +1322,33 @@ static int whpx_accel_init(MachineState *ms) goto error; } +memset(, 0, sizeof(WHV_PARTITION_PROPERTY)); +prop.ExtendedVmExits.X64CpuidExit = 1; +hr = WHvSetPartitionProperty(whpx->partition, + WHvPartitionPropertyCodeExtendedVmExits, + , + sizeof(WHV_PARTITION_PROPERTY)); + +if (FAILED(hr)) { +error_report("WHPX: Failed to enable partition extended X64CpuidExit" + " hr=%08lx", hr); +ret = -EINVAL; +goto error; +} + +UINT32 cpuidExitList[] = {1}; +hr = WHvSetPartitionProperty(whpx->partition, + WHvPartitionPropertyCodeCpuidExitList, + cpuidExitList, + RTL_NUMBER_OF(cpuidExitList) * sizeof(UINT32)); + +if (FAILED(hr)) { +error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx", + hr); +ret = -EINVAL; +goto error; +} + hr = WHvSetupPartition(whpx->partition); if (FAILED(hr)) { error_report("WHPX: Failed to setup partition, hr=%08lx", hr); -- 1.8.3.1
[Qemu-devel] [PULL 01/20] sys_membarrier: fix up include directives
From: Bruce RogersOur rule right now is to use <> for external headers only. util/sys_membarrier.c violates that. Fix it up. Signed-off-by: Bruce Rogers Message-Id: <20180329151018.15319-1-brog...@suse.com> Signed-off-by: Paolo Bonzini --- util/sys_membarrier.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/util/sys_membarrier.c b/util/sys_membarrier.c index 8dcb53e..1362c0c 100644 --- a/util/sys_membarrier.c +++ b/util/sys_membarrier.c @@ -6,9 +6,9 @@ * Author: Paolo Bonzini */ -#include -#include -#include +#include "qemu/osdep.h" +#include "qemu/sys_membarrier.h" +#include "qemu/error-report.h" #ifdef CONFIG_LINUX #include -- 1.8.3.1
Re: [Qemu-devel] [PATCH v11 01/15] migration: Set error state in case of error
Daniel P. Berrangewrote: > On Fri, Mar 16, 2018 at 05:49:07PM +, Daniel P. Berrangé wrote: >> On Fri, Mar 16, 2018 at 12:53:49PM +0100, Juan Quintela wrote: >> > Signed-off-by: Juan Quintela >> > --- >> > migration/ram.c | 20 >> > 1 file changed, 20 insertions(+) >> > >> > diff --git a/migration/ram.c b/migration/ram.c >> > index 7266351fd0..1b8095a358 100644 >> > --- a/migration/ram.c >> > +++ b/migration/ram.c >> > @@ -414,6 +414,16 @@ static void terminate_multifd_send_threads(Error >> > *errp) >> > { >> > int i; >> > >> > +if (errp) { >> > +MigrationState *s = migrate_get_current(); >> > +migrate_set_error(s, errp); >> >> This doesn't look quiet right. You're checking if 'errp' is a non-NULL, >> which just tells you if the caller wants to collect the error, not >> whether an error has happened. For the latter you need >> >> if (errp && *errp) >> >> seems a little strange though for the caller to pass an error into this >> method for reporting. > > Oh wait, I'm being mislead by the unusual parameter name. > > An "errp" name should only ever be used for a "Error **", but we > only have an "Error *" here. Copy & Paste O:-) > So just fix the parameter name to be "err" instead of "errp". Done. Thanks,
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
"Peter Maydell"wrote on 04/06/2018 09:51:55 AM: > I've now done this, and can reproduce the problem. So the > issue is generic to 32-bit hosts. Supporting evidence: I compiled Cygwin/MINGW with x86_64 and the resulting binaries work properly FWIW, the compile had some anomalous behavior: .../qemu-2.12.0-rc2/scripts/feature_to_c.sh: line 71: /usr/bin/sed: Invalid argument line 71 is: arrayname=xml_feature_$(echo $input | sed 's,.*/,,; s/[-.]/_/g') Not sure what the problem is, but I always use "-e" before scripts. Later the make failed with the following error: > gdbstub-xml.c:695:28: error: ‘xml_feature_’ undeclared here (not in a function) > { "i386-64bit-core.xml", xml_feature_ }, ^~~~ > gdbstub-xml.c:61:19: warning: ‘xml_feature_i386_64bit_core_xml’ defined but not used [-Wunused-const-variable=] > static const char xml_feature_i386_64bit_core_xml[] = { >^~~ > make[1]: *** [/cygdrive/c/scm/Deos/products/qemu/branches/2.12/output/qemu-2.12.0-rc2/rules.mak:66: gdbstub-xml.o] Error 1 > make: *** [Makefile:478: subdir-x86_64-softmmu] Error 2
Re: [Qemu-devel] [PATCH] iotests: Split 214 off of 122
On 04/06/2018 11:41 AM, Max Reitz wrote: > Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122 > regarding how the qcow2 driver handles an incorrect compressed data > length value. This does not really fit into 122, as that file is > supposed to contain qemu-img convert test cases, which this case is not. > So this patch splits it off into its own file; maybe we will even get > more qcow2-only compression tests in the future. > > Also, that test case does not work with refcount_bits=1, so mark that > option as unsupported. > > Signed-off-by: Max Reitz> --- > Kind of a v2 for "iotests: 122 needs at least two refcount bits now" > (fulfills the same purpose, but also splits the case into its own file > so you can still run 122 with refcount_bits=1 [Eric]). > > I was a bit lost what to do about the copyright text, since this test > case was written by Berto. I figured I'd drop the "owner" variable (it > isn't used anyway), but I put "Red Hat" into the copyright line -- > currently every test has copyright information, so I decided it'd be > difficult to leave that out, and I figured I simply cannot claim > copyright for Igalia. So, here we go. Yeah, you'll want Berto's Signed-off-by if you tweak that line, or at least his Reviewed-by otherwise :) But you can have mine, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.12] vfio-ccw: fix memory leak in vfio_ccw_realize()
On Fri, 06 Apr 2018 17:14:19 +0200 Greg Kurzwrote: > The vfio_ccw_realize() function currently leaks vcdev->vdev.name if > the subchannel is already attached or if vfio_get_device() fails. > > This happens because vcdev->vdev.name is expected to be freed in > vfio_put_device() which isn't called in this case. > > Adding g_free(vcdev->vdev.name) on these two error paths would be > enough to fix the leak, but this is unfortunate since we would then > have three locations doing this. Also, this would be inconsistent > with the label based rollback scheme of this function. This is all a bit awkward :( But I'd actually prefer adding the two g_free calls for now, as that would be a nice, small patch that my brain can still process at this point in the evening :) Your patch would be a nice cleanup for 2.13, though. Alternatively, I could be convinced to queue this if I get a R-b from someone :) > > The root issue is that vcdev->vdev.name is set before vfio_get_device() > is called, which theoretically prevents to call vfio_put_device() to > do the freeing. Well actually, we could call it anyway because > vfio_put_base_device() is a nop if the device isn't attached, but this > would be confusing. > > This patch hence moves all the logic of attaching the device to a > separate vfio_ccw_get_device() function, which is the counterpart > of vfio_put_device(). While here, vfio_put_device() is renamed to > vfio_ccw_put_device() for consistency. > > Signed-off-by: Greg Kurz > --- > hw/vfio/ccw.c | 54 -- > 1 file changed, 36 insertions(+), 18 deletions(-) > > diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c > index 4e5855741a64..49ae986d288d 100644 > --- a/hw/vfio/ccw.c > +++ b/hw/vfio/ccw.c > @@ -292,12 +292,44 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev) > g_free(vcdev->io_region); > } > > -static void vfio_put_device(VFIOCCWDevice *vcdev) > +static void vfio_ccw_put_device(VFIOCCWDevice *vcdev) > { > g_free(vcdev->vdev.name); > vfio_put_base_device(>vdev); > } > > +static int vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev, > + Error **errp) > +{ > +char *name = g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid, > + vcdev->cdev.hostid.ssid, > + vcdev->cdev.hostid.devid); > +VFIODevice *vbasedev; > + > +QLIST_FOREACH(vbasedev, >device_list, next) { > +if (strcmp(vbasedev->name, name) == 0) { > +error_setg(errp, "vfio: subchannel %s has already been attached", > + name); > +goto out_err; > +} > +} > + > +if (vfio_get_device(group, vcdev->cdev.mdevid, >vdev, errp)) { > +goto out_err; > +} > + > +vcdev->vdev.ops = _ccw_ops; > +vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; > +vcdev->vdev.name = name; > +vcdev->vdev.dev = >cdev.parent_obj.parent_obj; > + > +return 0; > + > +out_err: > +g_free(name); > +return -1; > +} > + > static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) > { > char *tmp, group_path[PATH_MAX]; > @@ -327,7 +359,6 @@ static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, > Error **errp) > > static void vfio_ccw_realize(DeviceState *dev, Error **errp) > { > -VFIODevice *vbasedev; > VFIOGroup *group; > CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev); > S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev); > @@ -348,20 +379,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error > **errp) > goto out_group_err; > } > > -vcdev->vdev.ops = _ccw_ops; > -vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; > -vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid, > - cdev->hostid.ssid, > cdev->hostid.devid); > -vcdev->vdev.dev = dev; > -QLIST_FOREACH(vbasedev, >device_list, next) { > -if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) { > -error_setg(, "vfio: subchannel %s has already been attached", > - vcdev->vdev.name); > -goto out_device_err; > -} > -} > - > -if (vfio_get_device(group, cdev->mdevid, >vdev, )) { > +if (vfio_ccw_get_device(group, vcdev, )) { > goto out_device_err; > } > > @@ -380,7 +398,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error > **errp) > out_notifier_err: > vfio_ccw_put_region(vcdev); > out_region_err: > -vfio_put_device(vcdev); > +vfio_ccw_put_device(vcdev); > out_device_err: > vfio_put_group(group); > out_group_err: > @@ -401,7 +419,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error > **errp) > > vfio_ccw_unregister_io_notifier(vcdev); > vfio_ccw_put_region(vcdev); > -vfio_put_device(vcdev); > +vfio_ccw_put_device(vcdev); >
Re: [Qemu-devel] [PULL for-2.12 0/1] target/hppa: seabios-hppa update
On 6 April 2018 at 14:18, Richard Hendersonwrote: > Helge Deller has fixed some bios bugs and has asked > that I update qemu thus. > > > r~ > > > The following changes since commit 0e87fdc966d05f4e5ad868034fcd8ee2a08ca62d: > > Update version for v2.12.0-rc2 release (2018-04-04 20:37:20 +0100) > > are available in the Git repository at: > > git://github.com/rth7680/qemu.git tags/pull-hppa-20180407 > > for you to fetch changes up to 5aa48600823a1fc2f5c8c3ec3e93227d4638e3a4: > > Update seabios-hppa (2018-04-06 23:14:51 +1000) > > > Update hppa-softmmu bios > > > Richard Henderson (1): > Update seabios-hppa > > pc-bios/hppa-firmware.img | Bin 215696 -> 215936 bytes > roms/seabios-hppa | 2 +- > 2 files changed, 1 insertion(+), 1 deletion(-) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 0/3] Add a CentOS test image to run docker tests
On 04/04/2018 04:51, Fam Zheng wrote: > Docker testing on patchew has long suffered from 'make check' hangings. The > cleanness of VM testing is the cure. Now let's add a CentOS 7 image to run the > tests. It's purely ad-hoc, but hopefully still easy to understand and use for > everyone. > > The first patch makes passing source code from host to the container in VM > working, and is a nice clean up. > > The second patch makes caches work, to speed up repetitive runs like on > patchew. > > The last patch adds the new image that does the job. Two out of three docker > tests running on patchew.org are added to the image. I'll wait for Peter to > fix > the 'docker-test-quick@centos6' hanging (oob test) before adding it too. I think it's a good idea. The tests are the same as before, only the environment is more self-contained. Paolo > Fam > > Fam Zheng (3): > archive-source.sh: Drop submodule code > tests: Add an option for snapshot (default: off) > tests: Add centos VM testing > > scripts/archive-source.sh | 47 +++ > tests/vm/basevm.py| 7 +++- > tests/vm/centos | 82 > +++ > 3 files changed, 92 insertions(+), 44 deletions(-) > create mode 100755 tests/vm/centos >
Re: [Qemu-devel] [PATCH] Add missing bit for SSE instr in VEX decoding
On 06/04/2018 15:41, Eugene Minibaev wrote: > Signed-off-by: Eugene Minibaev> --- > It seems that x86 vector instructions encoded in VEX are not properly > decoded because of missing bit, here is the example: Applied, thanks! Paolo > IN: > 0x08048060: c5 f9 6f c1 vmovdqa %xmm1, %xmm0 > 0x08048064: b8 01 00 00 00 movl $1, %eax > 0x08048069: bb 00 00 00 00 movl $0, %ebx > 0x0804806e: cd 80int $0x80 > > OUT: [size=191] > 0x604370c0: 41 8b 6e ec movl -0x14(%r14), %ebp > 0x604370c4: 85 edtestl%ebp, %ebp > 0x604370c6: 0f 8c a9 00 00 00jl 0x60437175 > 0x604370cc: 41 8b 6e 08 movl 8(%r14), %ebp > 0x604370d0: 0f b7 ed movzwl %bp, %ebp > 0x604370d3: 49 8b fe movq %r14, %rdi > 0x604370d6: 8b f5movl %ebp, %esi > 0x604370d8: e8 24 7f cd ff callq0x6010f001 > 0x604370dd: 41 8b 6e 18 movl 0x18(%r14), %ebp > 0x604370e1: 65 67 0f b7 6d 00movzwl %gs:(%ebp), %ebp > 0x604370e7: 41 8b 5e 08 movl 8(%r14), %ebx > 0x604370eb: 0f b7 db movzwl %bx, %ebx > 0x604370ee: 49 8b fe movq %r14, %rdi > 0x604370f1: 8b f3movl %ebx, %esi > 0x604370f3: 8b d5movl %ebp, %edx > 0x604370f5: e8 b1 06 cd ff callq0x601077ab > 0x604370fa: 41 8b 6e 38 movl 0x38(%r14), %ebp > 0x604370fe: d1 e5shll $1, %ebp > 0x60437100: 41 8b 5e 18 movl 0x18(%r14), %ebx > ... > 0x6043716b: ba 02 00 00 00 movl $2, %edx > 0x60437170: e8 20 8d cb ff callq0x600efe95 > 0x60437175: b8 43 70 43 60 movl $0x60437043, %eax > 0x6043717a: e9 99 fe ff ff jmp 0x60437018 > > qemu: uncaught target signal 11 (Segmentation fault) - core dumped > make: *** [Makefile:6: run] Segmentation fault (core dumped) > --- > target/i386/translate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target/i386/translate.c b/target/i386/translate.c > index 0135415d92..e2ce7e4061 100644 > --- a/target/i386/translate.c > +++ b/target/i386/translate.c > @@ -4564,7 +4564,7 @@ static target_ulong disas_insn(DisasContext *s, > CPUState *cpu) > rex_r = (~vex2 >> 4) & 8; > if (b == 0xc5) { > vex3 = vex2; > -b = x86_ldub_code(env, s); > +b = x86_ldub_code(env, s) | 0x100; > } else { > #ifdef TARGET_X86_64 > s->rex_x = (~vex2 >> 3) & 8; >
Re: [Qemu-devel] [PATCH] iotests: Split 214 off of 122
Sorry, Berto, forgot to CC you again... On 2018-04-06 18:41, Max Reitz wrote: > Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122 > regarding how the qcow2 driver handles an incorrect compressed data > length value. This does not really fit into 122, as that file is > supposed to contain qemu-img convert test cases, which this case is not. > So this patch splits it off into its own file; maybe we will even get > more qcow2-only compression tests in the future. > > Also, that test case does not work with refcount_bits=1, so mark that > option as unsupported. > > Signed-off-by: Max Reitz> --- > Kind of a v2 for "iotests: 122 needs at least two refcount bits now" > (fulfills the same purpose, but also splits the case into its own file > so you can still run 122 with refcount_bits=1 [Eric]). > > I was a bit lost what to do about the copyright text, since this test > case was written by Berto. I figured I'd drop the "owner" variable (it > isn't used anyway), but I put "Red Hat" into the copyright line -- > currently every test has copyright information, so I decided it'd be > difficult to leave that out, and I figured I simply cannot claim > copyright for Igalia. So, here we go. > --- > tests/qemu-iotests/122 | 47 --- > tests/qemu-iotests/122.out | 33 > tests/qemu-iotests/214 | 96 > ++ > tests/qemu-iotests/214.out | 35 + > tests/qemu-iotests/group | 1 + > 5 files changed, 132 insertions(+), 80 deletions(-) > create mode 100755 tests/qemu-iotests/214 > create mode 100644 tests/qemu-iotests/214.out > > diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 > index 6cf4fcb866..45b359c2ba 100755 > --- a/tests/qemu-iotests/122 > +++ b/tests/qemu-iotests/122 > @@ -129,53 +129,6 @@ $QEMU_IO -c "read -P 0x44 1023k1k" "$TEST_IMG" 2>&1 > | _filter_qemu_io | _fil > $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > > > -echo > -echo "=== Corrupted size field in compressed cluster descriptor ===" > -echo > -# Create an empty image and fill half of it with compressed data. > -# The L2 entries of the two compressed clusters are located at > -# 0x80 and 0x88, their original values are 0x400800a0 > -# and 0x400800a00802 (5 sectors for compressed data each). > -_make_test_img 8M -o cluster_size=2M > -$QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \ > - 2>&1 | _filter_qemu_io | _filter_testdir > - > -# Reduce size of compressed data to 4 sectors: this corrupts the image. > -poke_file "$TEST_IMG" $((0x80)) "\x40\x06" > -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > - > -# 'qemu-img check' however doesn't see anything wrong because it > -# doesn't try to decompress the data and the refcounts are consistent. > -# TODO: update qemu-img so this can be detected. > -_check_test_img > - > -# Increase size of compressed data to the maximum (8192 sectors). > -# This makes QEMU read more data (8192 sectors instead of 5, host > -# addresses [0xa0, 0xdf]), but the decompression algorithm > -# stops once we have enough to restore the uncompressed cluster, so > -# the rest of the data is ignored. > -poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe" > -# Do it also for the second compressed cluster (L2 entry at 0x88). > -# In this case the compressed data would span 3 host clusters > -# (host addresses: [0xa00802, 0xe00801]) > -poke_file "$TEST_IMG" $((0x88)) "\x7f\xfe" > - > -# Here the image is too small so we're asking QEMU to read beyond the > -# end of the image. > -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > -# But if we grow the image we won't be reading beyond its end anymore. > -$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > - > -# The refcount data is however wrong because due to the increased size > -# of the compressed data it now reaches the following host clusters. > -# This can be repaired by qemu-img check by increasing the refcount of > -# those clusters. > -# TODO: update qemu-img to correct the compressed cluster size instead. > -_check_test_img -r all > -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > -$QEMU_IO -c "read -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | > _filter_testdir > - > echo > echo "=== Full allocation with -S 0 ===" > echo > diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out > index a6b7fe007e..47d8656db8 100644 > --- a/tests/qemu-iotests/122.out > +++ b/tests/qemu-iotests/122.out > @@ -99,39 +99,6 @@ read 1024/1024 bytes at offset 1047552 > read 1046528/1046528 bytes at offset 1048576 > 1022 KiB, X ops;
[Qemu-devel] [PATCH] iotests: Split 214 off of 122
Commit abd3622cc03cf41ed542126a540385f30a4c0175 added a case to 122 regarding how the qcow2 driver handles an incorrect compressed data length value. This does not really fit into 122, as that file is supposed to contain qemu-img convert test cases, which this case is not. So this patch splits it off into its own file; maybe we will even get more qcow2-only compression tests in the future. Also, that test case does not work with refcount_bits=1, so mark that option as unsupported. Signed-off-by: Max Reitz--- Kind of a v2 for "iotests: 122 needs at least two refcount bits now" (fulfills the same purpose, but also splits the case into its own file so you can still run 122 with refcount_bits=1 [Eric]). I was a bit lost what to do about the copyright text, since this test case was written by Berto. I figured I'd drop the "owner" variable (it isn't used anyway), but I put "Red Hat" into the copyright line -- currently every test has copyright information, so I decided it'd be difficult to leave that out, and I figured I simply cannot claim copyright for Igalia. So, here we go. --- tests/qemu-iotests/122 | 47 --- tests/qemu-iotests/122.out | 33 tests/qemu-iotests/214 | 96 ++ tests/qemu-iotests/214.out | 35 + tests/qemu-iotests/group | 1 + 5 files changed, 132 insertions(+), 80 deletions(-) create mode 100755 tests/qemu-iotests/214 create mode 100644 tests/qemu-iotests/214.out diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 index 6cf4fcb866..45b359c2ba 100755 --- a/tests/qemu-iotests/122 +++ b/tests/qemu-iotests/122 @@ -129,53 +129,6 @@ $QEMU_IO -c "read -P 0x44 1023k1k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _fil $QEMU_IO -c "read -P 01024k 1022k" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir -echo -echo "=== Corrupted size field in compressed cluster descriptor ===" -echo -# Create an empty image and fill half of it with compressed data. -# The L2 entries of the two compressed clusters are located at -# 0x80 and 0x88, their original values are 0x400800a0 -# and 0x400800a00802 (5 sectors for compressed data each). -_make_test_img 8M -o cluster_size=2M -$QEMU_IO -c "write -c -P 0x11 0 2M" -c "write -c -P 0x11 2M 2M" "$TEST_IMG" \ - 2>&1 | _filter_qemu_io | _filter_testdir - -# Reduce size of compressed data to 4 sectors: this corrupts the image. -poke_file "$TEST_IMG" $((0x80)) "\x40\x06" -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir - -# 'qemu-img check' however doesn't see anything wrong because it -# doesn't try to decompress the data and the refcounts are consistent. -# TODO: update qemu-img so this can be detected. -_check_test_img - -# Increase size of compressed data to the maximum (8192 sectors). -# This makes QEMU read more data (8192 sectors instead of 5, host -# addresses [0xa0, 0xdf]), but the decompression algorithm -# stops once we have enough to restore the uncompressed cluster, so -# the rest of the data is ignored. -poke_file "$TEST_IMG" $((0x80)) "\x7f\xfe" -# Do it also for the second compressed cluster (L2 entry at 0x88). -# In this case the compressed data would span 3 host clusters -# (host addresses: [0xa00802, 0xe00801]) -poke_file "$TEST_IMG" $((0x88)) "\x7f\xfe" - -# Here the image is too small so we're asking QEMU to read beyond the -# end of the image. -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir -# But if we grow the image we won't be reading beyond its end anymore. -$QEMU_IO -c "write -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir - -# The refcount data is however wrong because due to the increased size -# of the compressed data it now reaches the following host clusters. -# This can be repaired by qemu-img check by increasing the refcount of -# those clusters. -# TODO: update qemu-img to correct the compressed cluster size instead. -_check_test_img -r all -$QEMU_IO -c "read -P 0x11 0 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir -$QEMU_IO -c "read -P 0x22 4M 4M" "$TEST_IMG" 2>&1 | _filter_qemu_io | _filter_testdir - echo echo "=== Full allocation with -S 0 ===" echo diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out index a6b7fe007e..47d8656db8 100644 --- a/tests/qemu-iotests/122.out +++ b/tests/qemu-iotests/122.out @@ -99,39 +99,6 @@ read 1024/1024 bytes at offset 1047552 read 1046528/1046528 bytes at offset 1048576 1022 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -=== Corrupted size field in compressed cluster descriptor === - -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8388608 -wrote 2097152/2097152 bytes at offset 0 -2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -wrote 2097152/2097152 bytes at
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
On 6 April 2018 at 15:51, Peter Maydellwrote: > On 6 April 2018 at 14:33, Peter Maydell wrote: >> On this end I should try this with a 32-bit Linux host. > > I've now done this, and can reproduce the problem. So the > issue is generic to 32-bit hosts. > > I'll see if I can figure out what's going wrong. I've tracked down the issue -- it is with the arm frontend's handling of the tcg_insn_start parameters. Specifically, we have a 3-operand tcg_insn_start: tcg_gen_insn_start(dc->pc, (dc->condexec_cond << 4) | (dc->condexec_mask >> 1), 0); dc->insn_start = tcg_last_op(); where we patch in the 3rd operand later sometimes in disas_set_insn_syndrome(): tcg_set_insn_param(s->insn_start, 2, syn); Unfortunately, if we're running on a setup where TARGET_LONG_BITS > TCG_TARGET_REG_BITS (ie 32 bit guest on 64 bit host), tcg_gen_insn_start() has under the hood split the 3 operands we gave it into 6, and so we end up patching the wrong one. The effect is that the first time the icount code needs to call io_recompile, we set condexec_bits to a bogus value which is also too big for its space in tb_flags and the CPSR, and execution starts to diverge from there onward. The following change fixes this: diff --git a/target/arm/translate.h b/target/arm/translate.h index c47febf99d..f04ece9cfd 100644 --- a/target/arm/translate.h +++ b/target/arm/translate.h @@ -120,7 +120,15 @@ static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn) /* We check and clear insn_start_idx to catch multiple updates. */ assert(s->insn_start != NULL); +#if TARGET_LONG_BITS <= TCG_TARGET_REG_BITS tcg_set_insn_param(s->insn_start, 2, syn); +#else +/* tcg_gen_insn_start has split every target_ulong argument to + * op_insn_start into two 32-bit arguments, so we want the low + * half of the 3rd input argument, which is at index 4. + */ +tcg_set_insn_param(s->insn_start, 4, syn); +#endif s->insn_start = NULL; } but I'm not convinced it's the neatest way to do it. Nobody else plays this game with tcg_set_insn_param() (except icount, which doesn't do it with target_ulong sized values), so this bug is specific to the arm target. Richard: do you have a cleaner suggestion than throwing this ifdef into the arm code? thanks -- PMM
[Qemu-devel] [Bug 855800] Re: KVM crashes when attempting to restart migration
** Changed in: qemu-kvm (Ubuntu) Status: Confirmed => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/855800 Title: KVM crashes when attempting to restart migration Status in QEMU: Incomplete Status in qemu-kvm package in Ubuntu: Incomplete Bug description: Operations performed: Sequence to trigger crash: * Start two kvm systems, one on gerph (primary), one on nbuild2 (listening for incoming migration) - do not use -daemonize * On gerph, connect to monitor. * "migrate -d -b tcp:nbuild2:" * "info migrate" * "migrate_cancel" * "info migrate" * "migrate -d -b tcp:nbuild2:" * crashed with assertion: kvm: block-migration.c:355: flush_blks: Assertion `block_mig_state.read_done >= 0' failed. Connection closed by foreign host. [1]+ Aborted (core dumped) kvm -drive file=./copy-disk2.img,boot=on -m 4096 -serial mon:telnet::23023,server,nowait -balloon virtio -vnc :99 -usbdevice tablet -net nic,macaddr=f6:a6:31:53:89:9a,model=rtl8139,vlan=0 -net tap,vlan=0 Repeating the operations above often dies in different places; just repeat the cancel and restart the operation. Because the KVM system dies, the underlying VM is obviously terminated. Distribution: jfletcher@gerph:~$ lsb_release -rd Description: Ubuntu 10.04.3 LTS Release: 10.04 Package: jfletcher@gerph:~$ apt-cache policy kvm kvm: Installed: 1:84+dfsg-0ubuntu16+0.12.3+noroms+0ubuntu9.15 Candidate: 1:84+dfsg-0ubuntu16+0.12.3+noroms+0ubuntu9.15 Version table: *** 1:84+dfsg-0ubuntu16+0.12.3+noroms+0ubuntu9.15 0 500 http://gb.archive.ubuntu.com/ubuntu/ lucid-updates/main Packages 500 http://security.ubuntu.com/ubuntu/ lucid-security/main Packages 100 /var/lib/dpkg/status 1:84+dfsg-0ubuntu16+0.12.3+noroms+0ubuntu9 0 500 http://gb.archive.ubuntu.com/ubuntu/ lucid/main Packages To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/855800/+subscriptions
Re: [Qemu-devel] [PATCH for-2.12] tap: set vhostfd passed from qemu cli to non-blocking
On 04/06/2018 10:44 AM, Eric Blake wrote: On 04/06/2018 07:03 AM, Brijesh Singh wrote: A guest boot hangs while probing the network interface when iommu_platform=on is used. The following qemu cli hangs without this patch: # $QEMU \ -netdev tap,fd=3,id=hostnet0,vhost=on,vhostfd=4 3<>/dev/tap67 4<>/dev/host-net \ -device virtio-net-pci,netdev=hostnet0,id=net0,iommu_platform=on,disable-legacy=on \ ... Commit: c471ad0e9bd46 (vhost_net: device IOTLB support) took care of setting vhostfd to non-blocking when QEMU opens /dev/host-net but if the fd is passed from qemu cli then we need to ensure that fd is set to non-blocking. Fixes: c471ad0e9bd46 "vhost_net: device IOTLB support" Cc: Michael S. TsirkinCc: Jason Wang Signed-off-by: Brijesh Singh --- net/tap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/tap.c b/net/tap.c index 2b3a36f9b50d..8c026fbf95cd 100644 --- a/net/tap.c +++ b/net/tap.c @@ -693,6 +693,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, NetClientState *peer, } return; } +fcntl(vhostfd, F_SETFL, O_NONBLOCK); Please use qemu_set_nonblock() instead. Sure will do.
Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception
On 04/05/2018 06:38 PM, Tony Krowiak wrote: > On 04/03/2018 05:36 AM, Cornelia Huck wrote: >> On Mon, 2 Apr 2018 12:36:27 -0400 >> Tony Krowiakwrote: >> >>> On 03/26/2018 05:03 AM, Pierre Morel wrote: On 26/03/2018 10:32, David Hildenbrand wrote: > On 16.03.2018 00:24, Tony Krowiak wrote: >> +/* >> + * The Query Configuration Information (QCI) function (fc == 4) >> does not >> + * set a response code in reg 1, so check for that along with the >> + * AP feature. >> + */ >> +if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { >> +env->regs[1] = 0x1; >> + >> +return 0; >> +} > This would imply an operation exception in case fc==4, which sounds very > wrong. It depends but I think that the S390_FEAT_AP_QUERY_CONFIG_INFO must be tested to know what to answer. If the feature is there, QCI must be answered correctly. >>> This is an interesting proposition which raises several issues that will >>> need to >>> be addressed. The only time the PQAP(QCI) instruction is intercepted is >>> when: >>> * A vfio-ap device is NOT defined for the guest because the vfio_ap >>> device driver >>> will set ECA.28 and the PQAP(QCI) instruction will be interpreted >>> * STFLE.12 is set for the guest >>> >>> You say that the QCI must be answered correctly, but what is the correct >>> response? >>> If we return the matrix - i.e., APM, ADM and AQM - configured via the >>> mediated >>> matrix device's sysfs attributes files, then if any APQNs are defined in >>> the matrix, >>> we will have to handle *ALL* AP instructions by executing them on behalf >>> of the >>> guest. I suppose we could return an empty matrix in which case the AP >>> bus will come >>> up without any devices on the guest, but what is the expectation of an >>> admin who >>> deliberately configures the mediated matrix device? Should we forego >>> handling interception >>> of AP instructions and consider a guest configuration that turns on >>> S390_FEAT_AP but >>> does not define a vfio-ap device to be erroneous and terminate starting >>> of the guest? >>> Anybody have any thoughts? >> Hard to really give good advice without access to the documentation, but: >> - If we tell the guest that the feature is available, but it does not >> get any cards to use, returning an empty matrix makes the most sense >> to me. >> - I would not tie starting the guest to the presence of a vfio-ap >> device. Having the feature available in theory but without any >> devices actually being usable by the guest does not really sound >> wrong (can we hotplug this later?) > For this phase of development, it is my opinion that introducing AP > instruction > interception handlers is superfluous for the following reasons: > > 1. Interception handling was introduced solely to ensure an operation > exception would > not be injected into the guest when CPU model feature for AP (i.e., ap=on) > is specified but a VFIO AP device (i.e., -device vfio-ap,sysfsdev=$path) > is not. We can kind of (i.e. modulo EECA.28) ensure this in a different fashion I think. How about proclaiming a 'has ap instructions, but nothing to see here' in the SIE interpreted flavor (ECA.28 set) the default way of having ap instructions under KVM. This should be easily accomplished with an all zero CRYCB and eca.28 set. The for the guest to actually get real work done with AP we would still require some sort of driver to either provide a non-zero matrix by altering the CRYCB or unsettling ECA.28 and doing the intercepted flavor. Please notice, the cpu facility ap would still keep it's semantic 'has ap instructions' (opposed to 'has ap instructions implemented in SIE interpreted flavor). And give us all the flexibility. Yet implementing what we want to have in absence of a driver would become much easier (under the assumption that ECA.28 equals EECA.28). How about this?
Re: [Qemu-devel] [PATCH for-2.12] tap: set vhostfd passed from qemu cli to non-blocking
On 04/06/2018 07:03 AM, Brijesh Singh wrote: > A guest boot hangs while probing the network interface when > iommu_platform=on is used. > > The following qemu cli hangs without this patch: > > # $QEMU \ > -netdev tap,fd=3,id=hostnet0,vhost=on,vhostfd=4 3<>/dev/tap67 > 4<>/dev/host-net \ > -device > virtio-net-pci,netdev=hostnet0,id=net0,iommu_platform=on,disable-legacy=on \ > ... > > Commit: c471ad0e9bd46 (vhost_net: device IOTLB support) took care of > setting vhostfd to non-blocking when QEMU opens /dev/host-net but if > the fd is passed from qemu cli then we need to ensure that fd is set > to non-blocking. > > Fixes: c471ad0e9bd46 "vhost_net: device IOTLB support" > Cc: Michael S. Tsirkin> Cc: Jason Wang > Signed-off-by: Brijesh Singh > --- > net/tap.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/tap.c b/net/tap.c > index 2b3a36f9b50d..8c026fbf95cd 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -693,6 +693,7 @@ static void net_init_tap_one(const NetdevTapOptions *tap, > NetClientState *peer, > } > return; > } > +fcntl(vhostfd, F_SETFL, O_NONBLOCK); Please use qemu_set_nonblock() instead. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] iotests: 122 needs at least two refcount bits now
On 2018-04-06 17:32, Eric Blake wrote: > On 04/06/2018 10:17 AM, Max Reitz wrote: >> The new test case for compressed clusters (added in commit >> abd3622cc03cf41ed542126a540385f30a4c0175) requires two refcount bits to >> succeed, so we need to skip the test when refcount_bits=1 was requested. > > We already questioned whether the changes to 122 were appropriate, given > that it didn't even use qemu-img convert. Should we instead split this > into two tests (keeping 122 as it was before, and making the new > compressed refcount munging tests be separate), where only the new test > needs this filter? Can do, why not. Max >> Signed-off-by: Max Reitz>> --- >> tests/qemu-iotests/122 | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 >> index 6cf4fcb866..95e7c51e72 100755 >> --- a/tests/qemu-iotests/122 >> +++ b/tests/qemu-iotests/122 >> @@ -42,6 +42,10 @@ _supported_fmt qcow2 >> _supported_proto file >> _supported_os Linux >> >> +# Repairing the compressed image requires qemu-img check to store a >> +# refcount up to 3, which requires at least two refcount bits. >> +_unsupported_imgopts 'refcount_bits=1[^0-9]' >> + > > This makes sense, but I'm withholding R-b until we decide if splitting > the tests is wiser. > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/2 for-2.12?] iotests: Skip 181 and 201 without userfaultfd
On 2018-04-06 17:30, Eric Blake wrote: > On 04/06/2018 10:17 AM, Max Reitz wrote: >> My non-Fedora testing system does not have a kernel with userfaultfd >> support which causes 181 and 201 to fail. That is annoying. This >> series makes those tests recognize the issue and convert it into a >> _notrun. > > As this is just testsuite fixes, is this worth including in 2.12? Then > again, as we are now building for -rc3, missing the release merely means > tests fail, and not a broken binary. I usually merge test patches independently of freeze, but on the other hand I prefer to only take the really necessary things for the later RCs. Sooo... I haven't decided yet. :-) Max >> Max Reitz (2): >> iotests: Add failure matching to common.qemu >> iotests: Skip 181 and 201 without userfaultfd >> >> tests/qemu-iotests/181 | 13 ++ >> tests/qemu-iotests/201 | 13 ++ >> tests/qemu-iotests/common.qemu | 58 >> +- >> 3 files changed, 77 insertions(+), 7 deletions(-) >> > signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] iotests: 122 needs at least two refcount bits now
On 04/06/2018 10:17 AM, Max Reitz wrote: > The new test case for compressed clusters (added in commit > abd3622cc03cf41ed542126a540385f30a4c0175) requires two refcount bits to > succeed, so we need to skip the test when refcount_bits=1 was requested. We already questioned whether the changes to 122 were appropriate, given that it didn't even use qemu-img convert. Should we instead split this into two tests (keeping 122 as it was before, and making the new compressed refcount munging tests be separate), where only the new test needs this filter? > > Signed-off-by: Max Reitz> --- > tests/qemu-iotests/122 | 4 > 1 file changed, 4 insertions(+) > > diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 > index 6cf4fcb866..95e7c51e72 100755 > --- a/tests/qemu-iotests/122 > +++ b/tests/qemu-iotests/122 > @@ -42,6 +42,10 @@ _supported_fmt qcow2 > _supported_proto file > _supported_os Linux > > +# Repairing the compressed image requires qemu-img check to store a > +# refcount up to 3, which requires at least two refcount bits. > +_unsupported_imgopts 'refcount_bits=1[^0-9]' > + This makes sense, but I'm withholding R-b until we decide if splitting the tests is wiser. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 0/2 for-2.12?] iotests: Skip 181 and 201 without userfaultfd
On 04/06/2018 10:17 AM, Max Reitz wrote: > My non-Fedora testing system does not have a kernel with userfaultfd > support which causes 181 and 201 to fail. That is annoying. This > series makes those tests recognize the issue and convert it into a > _notrun. As this is just testsuite fixes, is this worth including in 2.12? Then again, as we are now building for -rc3, missing the release merely means tests fail, and not a broken binary. > > > Max Reitz (2): > iotests: Add failure matching to common.qemu > iotests: Skip 181 and 201 without userfaultfd > > tests/qemu-iotests/181 | 13 ++ > tests/qemu-iotests/201 | 13 ++ > tests/qemu-iotests/common.qemu | 58 > +- > 3 files changed, 77 insertions(+), 7 deletions(-) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [ARM/FDPIC 4/4] linux-user: ARM-FDPIC: Add arm get tls syscall support
Co-Authored-By: Mickaël GuênéSigned-off-by: Christophe Lyon diff --git a/linux-user/arm/target_syscall.h b/linux-user/arm/target_syscall.h index 94e2a42..afc0772 100644 --- a/linux-user/arm/target_syscall.h +++ b/linux-user/arm/target_syscall.h @@ -16,6 +16,7 @@ struct target_pt_regs { #define ARM_NR_breakpoint (ARM_NR_BASE + 1) #define ARM_NR_cacheflush (ARM_NR_BASE + 2) #define ARM_NR_set_tls (ARM_NR_BASE + 5) +#define ARM_NR_get_tls(ARM_NR_BASE + 6) #define ARM_NR_semihosting 0x123456 #define ARM_NR_thumb_semihosting 0xAB diff --git a/linux-user/main.c b/linux-user/main.c index 00810d6..1814578 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -720,6 +720,9 @@ void cpu_loop(CPUARMState *env) case ARM_NR_breakpoint: env->regs[15] -= env->thumb ? 2 : 4; goto excp_debug; +case ARM_NR_get_tls: +env->regs[0] = cpu_get_tls(env); +break; default: gemu_log("qemu: Unsupported ARM syscall: 0x%x\n", n); -- 2.6.3
[Qemu-devel] [ARM/FDPIC 0/4] FDPIC ABI for ARM
Hello, This patch series implements the QEMU contribution of the FDPIC ABI for ARM targets. This ABI enables to run Linux on ARM MMU-less cores and supports shared libraries to reduce the memory footprint. Without MMU, text and data segment relative distances are different from one process to another, hence the need for a dedicated FDPIC register holding the start address of the data segment. One of the side effects is that function pointers require two words to be represented: the address of the code, and the data segment start address. These two words are designated as "Function Descriptor", hence the "FD PIC" name. On ARM, the FDPIC register is r9 [3]. This work was developed some time ago by STMicroelectronics, and was presented during Linaro Connect SFO15 (September 2015). You can watch the discussion and read the slides [1]. This presentation was related to the toolchain published on github [2], which is based on binutils-2.22, gcc-4.7, uclibc-0.9.33.2, gdb-7.5.1 and qemu-2.3.0, and for which pre-built binaries are available [2]. The ABI itself is described in details in [3]. Our Linux kernel patches have been updated and committed by Nicolas Pitre (Linaro) in July 2017. They are required so that the loader is able to handle this new file type. Indeed, the ELF files are tagged with ELFOSABI_ARM_FDPIC. This new tag has been allocated by ARM, as well as the new relocations involved. This patch series has been rebased on top of QEMU from 2018-03-28. I have also rebased the GCC patch series, but it is still WIP as cleanup is still needed before I can request a review. It can be useful to build a preview toolchain though, so my WIP branch is available at [4]. To build such a toolchain, you'd also need to use my uClibc branch [5]. I am currently working on updating the patches for the other toolchain components, and will upstream them soon. This includes gcc, uclibc, and gdb. This series provides support for ARM v7 and later architectures and has been used to run the GCC tests on arm-linux-gnueabi without regression, as well as arm-linux-uclibceabi. Are the QEMU patches OK for inclusion in master? Thanks, Christophe. [1] http://connect.linaro.org/resource/sfo15/sfo15-406-arm-fdpic-toolset-kernel-libraries-for-cortex-m-cortex-r-mmuless-cores/ [2] https://github.com/mickael-guene/fdpic_manifest [3] https://github.com/mickael-guene/fdpic_doc/blob/master/abi.txt [4] https://git.linaro.org/people/christophe.lyon/gcc.git/log/?h=fdpic-upstream [5] https://git.linaro.org/people/christophe.lyon/uclibc.git/log/?h=uClibc-0.9.33.2-fdpic-upstream Christophe Lyon (4): linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries linux-user: ARM-FDPIC: Add support of FDPIC for ARM. linux-user: ARM-FDPIC: Add support for signals for FDPIC targets linux-user: ARM-FDPIC: Add arm get tls syscall support configure | 10 + include/elf.h | 1 + linux-user/arm/target_syscall.h | 1 + linux-user/elfload.c| 35 linux-user/main.c | 8 linux-user/qemu.h | 3 ++ linux-user/signal.c | 91 + target/arm/cpu.h| 6 +++ 8 files changed, 147 insertions(+), 8 deletions(-) -- 2.6.3
Re: [Qemu-devel] [PATCH for-2.12] dump: Fix build with newer gcc
On 06/04/2018 17:06, Eric Blake wrote: > On 04/03/2018 01:46 PM, Eric Blake wrote: >> On 03/27/2018 03:21 PM, Eric Blake wrote: >>> gcc 8 on rawhide is picky enough to complain: >>> >>> /home/dummy/qemu/dump.c: In function 'create_header32': >>> /home/dummy/qemu/dump.c:817:5: error: 'strncpy' output truncated before >>> terminating nul copying 8 bytes from a string of the same length >>> [-Werror=stringop-truncation] >>> strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); >>> ^~~~ >>> >>> But we already have SIG_LEN defined as the right length without needing >>> to do a strlen(), and memcpy() is better than strncpy() when we know >>> we do not want a trailing NUL byte. >>> >>> Signed-off-by: Eric Blake>>> --- >>> dump.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> I'll include this in my qapi+misc pull request for 2.12-rc2 > > That pull request was abandoned; Paolo, do you want to pick this up in > your misc tree? > Ok. Paolo
[Qemu-devel] [ARM/FDPIC 2/4] linux-user: ARM-FDPIC: Add support of FDPIC for ARM.
Add FDPIC info into image_info structure since interpreter info is on stack and needs to be saved to be accessed later on. Co-Authored-By: Mickaël GuênéSigned-off-by: Christophe Lyon diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 7ba3795..363da67 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -287,6 +287,23 @@ static inline void init_thread(struct target_pt_regs *regs, /* For uClinux PIC binaries. */ /* XXX: Linux does this only on ARM with no MMU (do we care ?) */ regs->uregs[10] = infop->start_data; +#ifdef CONFIG_USE_FDPIC +/* Support ARM FDPIC. */ +/* As described in the ABI document, r7 points to the loadmap info + * prepared by the kernel. If an interpreter is needed, r8 points + * to the interpreter loadmap and r9 points to the interpreter + * PT_DYNAMIC info. If no interpreter is needed, r8 is zer0, and + * r9 points to the main program PT_DYNAMIC info. */ +regs->uregs[7] = infop->loadmap_addr; +if (infop->interpreter_loadmap_addr) { +/* Executable is dynamically loaded. */ +regs->uregs[8] = infop->interpreter_loadmap_addr; +regs->uregs[9] = infop->interpreter_pt_dynamic_addr; +} else { +regs->uregs[8] = 0; +regs->uregs[9] = infop->pt_dynamic_addr; +} +#endif } #define ELF_NREG18 @@ -1692,6 +1709,11 @@ static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong s } #endif +int info_is_fdpic(struct image_info *info) +{ +return (info->personality == PER_LINUX_FDPIC); +} + static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, struct elfhdr *exec, struct image_info *info, @@ -1719,6 +1741,11 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, if (interp_info) { interp_info->other_info = info; sp = loader_build_fdpic_loadmap(interp_info, sp); +info->interpreter_loadmap_addr = interp_info->loadmap_addr; +info->interpreter_pt_dynamic_addr = interp_info->pt_dynamic_addr; +} else { +info->interpreter_loadmap_addr = 0; +info->interpreter_pt_dynamic_addr = 0; } } #endif diff --git a/linux-user/main.c b/linux-user/main.c index ba09b7d..00810d6 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -4868,6 +4868,11 @@ int main(int argc, char **argv, char **envp) env->cp15.sctlr_el[i] |= SCTLR_EE; } #endif + +#if defined(CONFIG_USE_FDPIC) +/* Are we running an FDPIC binary? */ +env->is_fdpic = info_is_fdpic(info); +#endif } #elif defined(TARGET_ARM) { diff --git a/linux-user/qemu.h b/linux-user/qemu.h index 192a0d2..7eaf9e9 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -56,6 +56,8 @@ struct image_info { uint16_tnsegs; void *loadsegs; abi_ulong pt_dynamic_addr; +abi_ulong interpreter_loadmap_addr; +abi_ulong interpreter_pt_dynamic_addr; struct image_info *other_info; #endif }; @@ -182,6 +184,7 @@ abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp, int loader_exec(int fdexec, const char *filename, char **argv, char **envp, struct target_pt_regs * regs, struct image_info *infop, struct linux_binprm *); +int info_is_fdpic(struct image_info *info); uint32_t get_elf_eflags(int fd); int load_elf_binary(struct linux_binprm *bprm, struct image_info *info); diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 19a0c03..90c8ee1 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -629,6 +629,12 @@ typedef struct CPUARMState { const struct arm_boot_info *boot_info; /* Store GICv3CPUState to access from this struct */ void *gicv3state; + +#if defined(CONFIG_USER_ONLY) && defined(CONFIG_USE_FDPIC) +/* We need to know if we have an FDPIC binary to adapt signal + * syscalls. */ +int is_fdpic; +#endif } CPUARMState; /** -- 2.6.3
[Qemu-devel] [ARM/FDPIC 3/4] linux-user: ARM-FDPIC: Add support for signals for FDPIC targets
The FDPIC restorer needs to deal with a function descriptor, hence we have to extend 'retcode' such that it can hold the instructions needed to perform this. The restorer sequence uses the same thumbness as the exception handler (mainly to support Thumb-only architectures). Co-Authored-By: Mickaël GuênéSigned-off-by: Christophe Lyon diff --git a/linux-user/signal.c b/linux-user/signal.c index 2ea3e03..75643d7 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -2039,13 +2039,13 @@ struct sigframe_v1 { struct target_sigcontext sc; abi_ulong extramask[TARGET_NSIG_WORDS-1]; -abi_ulong retcode; +abi_ulong retcode[4]; }; struct sigframe_v2 { struct target_ucontext_v2 uc; -abi_ulong retcode; +abi_ulong retcode[4]; }; struct rt_sigframe_v1 @@ -2054,14 +2054,14 @@ struct rt_sigframe_v1 abi_ulong puc; struct target_siginfo info; struct target_ucontext_v1 uc; -abi_ulong retcode; +abi_ulong retcode[4]; }; struct rt_sigframe_v2 { struct target_siginfo info; struct target_ucontext_v2 uc; -abi_ulong retcode; +abi_ulong retcode[4]; }; #define TARGET_CONFIG_CPU_32 1 @@ -2084,6 +2084,23 @@ static const abi_ulong retcodes[4] = { SWI_SYS_RT_SIGRETURN, SWI_THUMB_RT_SIGRETURN }; +#if defined(CONFIG_USE_FDPIC) +/* + * Stub needed to make sure the FD register (r9) contains the right + * value. + */ +static const unsigned long sigreturn_fdpic_codes[3] = { +0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */ +0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */ +0xe59cf000 /* ldr pc, [r12] to jump into restorer */ +}; + +static const unsigned long sigreturn_fdpic_thumb_codes[3] = { +0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */ +0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */ +0xf000f8dc /* ldr pc, [r12] to jump into restorer */ +}; +#endif static inline int valid_user_regs(CPUARMState *regs) { @@ -2143,7 +2160,19 @@ setup_return(CPUARMState *env, struct target_sigaction *ka, { abi_ulong handler = ka->_sa_handler; abi_ulong retcode; + +#ifdef CONFIG_USE_FDPIC +int thumb; + +if (env->is_fdpic) { +thumb = (((abi_ulong *)g2h(ka->_sa_handler))[0]) & 1; +} else { +thumb = handler & 1; +} +#else int thumb = handler & 1; +#endif + uint32_t cpsr = cpsr_read(env); cpsr &= ~CPSR_IT; @@ -2154,8 +2183,37 @@ setup_return(CPUARMState *env, struct target_sigaction *ka, } if (ka->sa_flags & TARGET_SA_RESTORER) { +#ifdef CONFIG_USE_FDPIC +if (env->is_fdpic) { +/* For FDPIC we ensure that the restorer is called with a + * correct r9 value. For that we need to write code on + * the stack that sets r9 and jumps back to restorer + * value. + */ +if (thumb) { +__put_user(sigreturn_fdpic_thumb_codes[0], rc); +__put_user(sigreturn_fdpic_thumb_codes[1], rc + 1); +__put_user(sigreturn_fdpic_thumb_codes[2], rc + 2); +__put_user((abi_ulong)ka->sa_restorer, rc + 3); +} else { +__put_user(sigreturn_fdpic_codes[0], rc); +__put_user(sigreturn_fdpic_codes[1], rc + 1); +__put_user(sigreturn_fdpic_codes[2], rc + 2); +__put_user((abi_ulong)ka->sa_restorer, rc + 3); +} + +retcode = rc_addr + thumb; +} else +#endif retcode = ka->sa_restorer; } else { +#ifdef CONFIG_USE_FDPIC +if (env->is_fdpic) { +qemu_log_mask(LOG_UNIMP, + "arm: FDPIC signal return not implemented"); +abort(); +} else { +#endif unsigned int idx = thumb; if (ka->sa_flags & TARGET_SA_SIGINFO) { @@ -2165,12 +2223,29 @@ setup_return(CPUARMState *env, struct target_sigaction *ka, __put_user(retcodes[idx], rc); retcode = rc_addr + thumb; +#ifdef CONFIG_USE_FDPIC +} +#endif } env->regs[0] = usig; +#ifdef CONFIG_USE_FDPIC +if (env->is_fdpic) { +env->regs[9] = ((abi_ulong *)g2h(ka->_sa_handler))[1]; +} +#endif env->regs[13] = frame_addr; env->regs[14] = retcode; +#ifdef CONFIG_USE_FDPIC +if (env->is_fdpic) { +env->regs[15] = ((abi_ulong *)g2h(ka->_sa_handler))[0] +& (thumb ? ~1 : ~3); +} else { +env->regs[15] = handler & (thumb ? ~1 : ~3); +} +#else env->regs[15] = handler & (thumb ? ~1 : ~3); +#endif cpsr_write(env, cpsr, CPSR_IT | CPSR_T, CPSRWriteByInstr); } @@ -2264,7 +2339,7 @@ static void setup_frame_v1(int usig, struct target_sigaction *ka, __put_user(set->sig[i], >extramask[i - 1]); } -setup_return(regs, ka, >retcode, frame_addr, usig, +setup_return(regs, ka, frame->retcode, frame_addr, usig, frame_addr
[Qemu-devel] [PATCH 2/2] iotests: Skip 181 and 201 without userfaultfd
userfaultfd support depends on the host kernel, so it may not be available. If so, 181 and 201 should be skipped. Signed-off-by: Max Reitz--- tests/qemu-iotests/181 | 13 + tests/qemu-iotests/201 | 13 + 2 files changed, 26 insertions(+) diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181 index 5e767c6195..e02979378d 100755 --- a/tests/qemu-iotests/181 +++ b/tests/qemu-iotests/181 @@ -96,6 +96,19 @@ echo # Enable postcopy-ram capability both on source and destination silent=yes _send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)" + +qemu_error_no_exit=yes success_or_failure=yes \ +_send_qemu_cmd $dest '' "(qemu)" "Postcopy is not supported" +if [ ${QEMU_STATUS[$dest]} -lt 0 ]; then +_send_qemu_cmd $dest '' "(qemu)" + +_send_qemu_cmd $src 'quit' "" +_send_qemu_cmd $dest 'quit' "" +wait=1 _cleanup_qemu + +_notrun 'Postcopy is not supported' +fi + _send_qemu_cmd $src 'migrate_set_speed 4k' "(qemu)" _send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)" _send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)" diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201 index 11f640f5df..c1a1e00077 100755 --- a/tests/qemu-iotests/201 +++ b/tests/qemu-iotests/201 @@ -82,6 +82,19 @@ echo silent=yes _send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)" + +qemu_error_no_exit=yes success_or_failure=yes \ +_send_qemu_cmd $dest '' "(qemu)" "Postcopy is not supported" +if [ ${QEMU_STATUS[$dest]} -lt 0 ]; then +_send_qemu_cmd $dest '' "(qemu)" + +_send_qemu_cmd $src 'quit' "" +_send_qemu_cmd $dest 'quit' "" +wait=1 _cleanup_qemu + +_notrun 'Postcopy is not supported' +fi + _send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)" _send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)" -- 2.14.3
[Qemu-devel] [ARM/FDPIC 1/4] linux-user: ARM-FDPIC: Add configure option to support loading of FDPIC binaries
Adds --enable-fdpic and --disable-fdpic configure options. This feature is disabled by default, that's why it is not described in the "Optional features" help section (which are enabled by default if possible). FDPIC ELF objects are identified with e_ident[EI_OSABI] == ELFOSABI_ARM_FDPIC. Co-Authored-By: Mickaël GuênéSigned-off-by: Christophe Lyon diff --git a/configure b/configure index 4d0e92c..af4c14b 100755 --- a/configure +++ b/configure @@ -451,6 +451,7 @@ jemalloc="no" replication="yes" vxhs="" libxml2="" +fdpic="no" supported_cpu="no" supported_os="no" @@ -1374,6 +1375,10 @@ for opt do ;; --disable-git-update) git_update=no ;; + --disable-fdpic) fdpic="no" + ;; + --enable-fdpic) fdpic="yes" + ;; *) echo "ERROR: unknown option $opt" echo "Try '$0 --help' for more information" @@ -1544,6 +1549,8 @@ Advanced options (experts only): xen pv domain builder --enable-debug-stack-usage track the maximum stack usage of stacks created by qemu_alloc_stack + --disable-fdpic disable loading of FDPIC binary (default) + --enable-fdpic enable loading of FDPIC binary Optional features, enabled with --enable-FEATURE and disabled with --disable-FEATURE, default is enabled if available: @@ -7085,6 +7092,9 @@ fi echo "LDFLAGS+=$ldflags" >> $config_target_mak echo "QEMU_CFLAGS+=$cflags" >> $config_target_mak +if [ "$fdpic" = "yes" ]; then +echo "CONFIG_USE_FDPIC=y" >> $config_target_mak +fi done # for target in $targets diff --git a/include/elf.h b/include/elf.h index c0dc9bb..934dbbd 100644 --- a/include/elf.h +++ b/include/elf.h @@ -1483,6 +1483,7 @@ typedef struct elf64_shdr { #define ELFOSABI_TRU64 10 /* Compaq TRU64 UNIX. */ #define ELFOSABI_MODESTO11 /* Novell Modesto. */ #define ELFOSABI_OPENBSD12 /* OpenBSD. */ +#define ELFOSABI_ARM_FDPIC 65 /* ARM FDPIC */ #define ELFOSABI_ARM97 /* ARM */ #define ELFOSABI_STANDALONE 255 /* Standalone (embedded) application */ diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 23e3495..7ba3795 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1658,6 +1658,14 @@ static void zero_bss(abi_ulong elf_bss, abi_ulong last_bss, int prot) } #ifdef CONFIG_USE_FDPIC + +#ifdef TARGET_ARM +static int elf_is_fdpic(struct elfhdr *exec) +{ +return exec->e_ident[EI_OSABI] == ELFOSABI_ARM_FDPIC; +} +#endif + static abi_ulong loader_build_fdpic_loadmap(struct image_info *info, abi_ulong sp) { uint16_t n; -- 2.6.3
[Qemu-devel] [PATCH 0/2] iotests: Skip 181 and 201 without userfaultfd
My non-Fedora testing system does not have a kernel with userfaultfd support which causes 181 and 201 to fail. That is annoying. This series makes those tests recognize the issue and convert it into a _notrun. Max Reitz (2): iotests: Add failure matching to common.qemu iotests: Skip 181 and 201 without userfaultfd tests/qemu-iotests/181 | 13 ++ tests/qemu-iotests/201 | 13 ++ tests/qemu-iotests/common.qemu | 58 +- 3 files changed, 77 insertions(+), 7 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH] iotests: 122 needs at least two refcount bits now
The new test case for compressed clusters (added in commit abd3622cc03cf41ed542126a540385f30a4c0175) requires two refcount bits to succeed, so we need to skip the test when refcount_bits=1 was requested. Signed-off-by: Max Reitz--- tests/qemu-iotests/122 | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122 index 6cf4fcb866..95e7c51e72 100755 --- a/tests/qemu-iotests/122 +++ b/tests/qemu-iotests/122 @@ -42,6 +42,10 @@ _supported_fmt qcow2 _supported_proto file _supported_os Linux +# Repairing the compressed image requires qemu-img check to store a +# refcount up to 3, which requires at least two refcount bits. +_unsupported_imgopts 'refcount_bits=1[^0-9]' + TEST_IMG="$TEST_IMG".base _make_test_img 64M $QEMU_IO -c "write -P 0x11 0 64M" "$TEST_IMG".base 2>&1 | _filter_qemu_io | _filter_testdir -- 2.14.3
[Qemu-devel] [PATCH 1/2] iotests: Add failure matching to common.qemu
Currently, common.qemu only allows to match for results indicating success. The only way to fail is by provoking a timeout. However, sometimes we do have a defined failure output and can match for that, which saves us from having to wait for the timeout in case of failure. Because failure can sometimes just result in a _notrun in the test, it is actually important to care about being able to fail quickly. Also, sometimes we simply do not get any specific output in case of success. The only way to handle this currently would be to define an error message as the string to look for, which means that actual success results in a timeout. This is really bad because it unnecessarily slows down a succeeding test. Therefore, this patch adds a new parameter $success_or_failure to _timed_wait_for and _send_qemu_cmd. Setting this to a non-empty string makes both commands expect two match parameters: If the first matches, the function succeeds. If the second matches, the function fails. Signed-off-by: Max Reitz--- tests/qemu-iotests/common.qemu | 58 +- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index 85f66b852c..f285484951 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -52,11 +52,29 @@ _in_fd=4 # response is not echoed out. # If $mismatch_only is set, only non-matching responses will # be echoed. +# +# If $success_or_failure is set, the meaning of the arguments is +# changed as follows: +# $2: A string to search for in the response; if found, this indicates +# success and ${QEMU_STATUS[$1]} is set to 0. +# $3: A string to search for in the response; if found, this indicates +# failure and the test is either aborted (if $qemu_error_no_exit +# is not set) or ${QEMU_STATUS[$1]} is set to -1 (otherwise). function _timed_wait_for() { local h=${1} shift +if [ -z "${success_or_failure}" ]; then +success_match=${*} +failure_match= +else +success_match=${1} +failure_match=${2} +fi + +timeout=yes + QEMU_STATUS[$h]=0 while IFS= read -t ${QEMU_COMM_TIMEOUT} resp <&${QEMU_OUT[$h]} do @@ -64,10 +82,18 @@ function _timed_wait_for() echo "${resp}" | _filter_testdir | _filter_qemu \ | _filter_qemu_io | _filter_qmp | _filter_hmp fi -grep -q "${*}" < <(echo "${resp}") +if [ -n "${failure_match}" ]; then +grep -q "${failure_match}" < <(echo "${resp}") +if [ $? -eq 0 ]; then +timeout= +break +fi +fi +grep -q "${success_match}" < <(echo "${resp}") if [ $? -eq 0 ]; then return -elif [ -z "${silent}" ] && [ -n "${mismatch_only}" ]; then +fi +if [ -z "${silent}" ] && [ -n "${mismatch_only}" ]; then echo "${resp}" | _filter_testdir | _filter_qemu \ | _filter_qemu_io | _filter_qmp | _filter_hmp fi @@ -75,8 +101,12 @@ function _timed_wait_for() done QEMU_STATUS[$h]=-1 if [ -z "${qemu_error_no_exit}" ]; then -echo "Timeout waiting for ${*} on handle ${h}" -exit 1 # Timeout means the test failed +if [ -n "${timeout}" ]; then +echo "Timeout waiting for ${success_match} on handle ${h}" +else +echo "Wrong response matching ${failure_match} on handle ${h}" +fi +exit 1 # Timeout or wrong match mean the test failed fi } @@ -96,6 +126,11 @@ function _timed_wait_for() # If $qemu_error_no_exit is set, then even if the expected response # is not seen, we will not exit. $QEMU_STATUS[$1] will be set it -1 in # that case. +# +# If $success_or_failure is set, then the last two strings are the +# strings the response will be scanned for. The first of the two +# indicates success, the latter indicates failure. Failure is handled +# like a timeout. function _send_qemu_cmd() { local h=${1} @@ -109,14 +144,23 @@ function _send_qemu_cmd() use_error="no" fi # This array element extraction is done to accommodate pathnames with spaces -cmd=${@: 1:${#@}-1} -shift $(($# - 1)) +if [ -z "${success_or_failure}" ]; then +cmd=${@: 1:${#@}-1} +shift $(($# - 1)) +else +cmd=${@: 1:${#@}-2} +shift $(($# - 2)) +fi while [ ${count} -gt 0 ] do echo "${cmd}" >&${QEMU_IN[${h}]} if [ -n "${1}" ]; then -qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}" +if [ -z "${success_or_failure}" ]; then +qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}" +else +qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}" "${2}" +fi if [ ${QEMU_STATUS[$h]} -eq
[Qemu-devel] [PATCH for-2.12] vfio-ccw: fix memory leak in vfio_ccw_realize()
The vfio_ccw_realize() function currently leaks vcdev->vdev.name if the subchannel is already attached or if vfio_get_device() fails. This happens because vcdev->vdev.name is expected to be freed in vfio_put_device() which isn't called in this case. Adding g_free(vcdev->vdev.name) on these two error paths would be enough to fix the leak, but this is unfortunate since we would then have three locations doing this. Also, this would be inconsistent with the label based rollback scheme of this function. The root issue is that vcdev->vdev.name is set before vfio_get_device() is called, which theoretically prevents to call vfio_put_device() to do the freeing. Well actually, we could call it anyway because vfio_put_base_device() is a nop if the device isn't attached, but this would be confusing. This patch hence moves all the logic of attaching the device to a separate vfio_ccw_get_device() function, which is the counterpart of vfio_put_device(). While here, vfio_put_device() is renamed to vfio_ccw_put_device() for consistency. Signed-off-by: Greg Kurz--- hw/vfio/ccw.c | 54 -- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 4e5855741a64..49ae986d288d 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -292,12 +292,44 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev) g_free(vcdev->io_region); } -static void vfio_put_device(VFIOCCWDevice *vcdev) +static void vfio_ccw_put_device(VFIOCCWDevice *vcdev) { g_free(vcdev->vdev.name); vfio_put_base_device(>vdev); } +static int vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev, + Error **errp) +{ +char *name = g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid, + vcdev->cdev.hostid.ssid, + vcdev->cdev.hostid.devid); +VFIODevice *vbasedev; + +QLIST_FOREACH(vbasedev, >device_list, next) { +if (strcmp(vbasedev->name, name) == 0) { +error_setg(errp, "vfio: subchannel %s has already been attached", + name); +goto out_err; +} +} + +if (vfio_get_device(group, vcdev->cdev.mdevid, >vdev, errp)) { +goto out_err; +} + +vcdev->vdev.ops = _ccw_ops; +vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; +vcdev->vdev.name = name; +vcdev->vdev.dev = >cdev.parent_obj.parent_obj; + +return 0; + +out_err: +g_free(name); +return -1; +} + static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) { char *tmp, group_path[PATH_MAX]; @@ -327,7 +359,6 @@ static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) static void vfio_ccw_realize(DeviceState *dev, Error **errp) { -VFIODevice *vbasedev; VFIOGroup *group; CcwDevice *ccw_dev = DO_UPCAST(CcwDevice, parent_obj, dev); S390CCWDevice *cdev = DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev); @@ -348,20 +379,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) goto out_group_err; } -vcdev->vdev.ops = _ccw_ops; -vcdev->vdev.type = VFIO_DEVICE_TYPE_CCW; -vcdev->vdev.name = g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid, - cdev->hostid.ssid, cdev->hostid.devid); -vcdev->vdev.dev = dev; -QLIST_FOREACH(vbasedev, >device_list, next) { -if (strcmp(vbasedev->name, vcdev->vdev.name) == 0) { -error_setg(, "vfio: subchannel %s has already been attached", - vcdev->vdev.name); -goto out_device_err; -} -} - -if (vfio_get_device(group, cdev->mdevid, >vdev, )) { +if (vfio_ccw_get_device(group, vcdev, )) { goto out_device_err; } @@ -380,7 +398,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp) out_notifier_err: vfio_ccw_put_region(vcdev); out_region_err: -vfio_put_device(vcdev); +vfio_ccw_put_device(vcdev); out_device_err: vfio_put_group(group); out_group_err: @@ -401,7 +419,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error **errp) vfio_ccw_unregister_io_notifier(vcdev); vfio_ccw_put_region(vcdev); -vfio_put_device(vcdev); +vfio_ccw_put_device(vcdev); vfio_put_group(group); if (cdc->unrealize) {
Re: [Qemu-devel] [PATCH for-2.12] maint: Add .mailmap entries for patches claiming list authorship
On 04/03/2018 01:45 PM, Eric Blake wrote: > On 03/26/2018 01:41 PM, Eric Blake wrote: >> The list did not author any patches, but it does rewrite the >> 'From:' header of messages sent from any domain with restrictive >> SPF policies that would otherwise prevent the message from reaching >> all list recipients. If a maintainer is not careful to undo the >> list header rewrite, and the author did not include a manual >> 'From:' line in the body to fix the munged header, then 'git am' >> happily attributes the patch to the list. Add some mailmap >> entries to correct the few that have escaped our attention; while >> we also work on improving the tooling to catch the problem in >> the future before a merge is even made. >> >> Also improve the comments occurring in the file, including line >> length improvements. >> >> Signed-off-by: Eric Blake>> --- >> >> Probably worth inclusion in 2.12-rc1, since it makes statistics >> about the release easier to follow. > > I'll include this in my qapi+misc pull for 2.12-rc2 That pull request was abandoned; Paolo, do you want to pick this up in your misc tree? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH for-2.12] dump: Fix build with newer gcc
On 04/03/2018 01:46 PM, Eric Blake wrote: > On 03/27/2018 03:21 PM, Eric Blake wrote: >> gcc 8 on rawhide is picky enough to complain: >> >> /home/dummy/qemu/dump.c: In function 'create_header32': >> /home/dummy/qemu/dump.c:817:5: error: 'strncpy' output truncated before >> terminating nul copying 8 bytes from a string of the same length >> [-Werror=stringop-truncation] >> strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE)); >> ^~~~ >> >> But we already have SIG_LEN defined as the right length without needing >> to do a strlen(), and memcpy() is better than strncpy() when we know >> we do not want a trailing NUL byte. >> >> Signed-off-by: Eric Blake>> --- >> dump.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) > > I'll include this in my qapi+misc pull request for 2.12-rc2 That pull request was abandoned; Paolo, do you want to pick this up in your misc tree? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] Add missing bit for SSE instr in VEX decoding
Signed-off-by: Eugene Minibaev--- It seems that x86 vector instructions encoded in VEX are not properly decoded because of missing bit, here is the example: IN: 0x08048060: c5 f9 6f c1 vmovdqa %xmm1, %xmm0 0x08048064: b8 01 00 00 00 movl $1, %eax 0x08048069: bb 00 00 00 00 movl $0, %ebx 0x0804806e: cd 80int $0x80 OUT: [size=191] 0x604370c0: 41 8b 6e ec movl -0x14(%r14), %ebp 0x604370c4: 85 edtestl%ebp, %ebp 0x604370c6: 0f 8c a9 00 00 00jl 0x60437175 0x604370cc: 41 8b 6e 08 movl 8(%r14), %ebp 0x604370d0: 0f b7 ed movzwl %bp, %ebp 0x604370d3: 49 8b fe movq %r14, %rdi 0x604370d6: 8b f5movl %ebp, %esi 0x604370d8: e8 24 7f cd ff callq0x6010f001 0x604370dd: 41 8b 6e 18 movl 0x18(%r14), %ebp 0x604370e1: 65 67 0f b7 6d 00movzwl %gs:(%ebp), %ebp 0x604370e7: 41 8b 5e 08 movl 8(%r14), %ebx 0x604370eb: 0f b7 db movzwl %bx, %ebx 0x604370ee: 49 8b fe movq %r14, %rdi 0x604370f1: 8b f3movl %ebx, %esi 0x604370f3: 8b d5movl %ebp, %edx 0x604370f5: e8 b1 06 cd ff callq0x601077ab 0x604370fa: 41 8b 6e 38 movl 0x38(%r14), %ebp 0x604370fe: d1 e5shll $1, %ebp 0x60437100: 41 8b 5e 18 movl 0x18(%r14), %ebx ... 0x6043716b: ba 02 00 00 00 movl $2, %edx 0x60437170: e8 20 8d cb ff callq0x600efe95 0x60437175: b8 43 70 43 60 movl $0x60437043, %eax 0x6043717a: e9 99 fe ff ff jmp 0x60437018 qemu: uncaught target signal 11 (Segmentation fault) - core dumped make: *** [Makefile:6: run] Segmentation fault (core dumped) --- target/i386/translate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/translate.c b/target/i386/translate.c index 0135415d92..e2ce7e4061 100644 --- a/target/i386/translate.c +++ b/target/i386/translate.c @@ -4564,7 +4564,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu) rex_r = (~vex2 >> 4) & 8; if (b == 0xc5) { vex3 = vex2; -b = x86_ldub_code(env, s); +b = x86_ldub_code(env, s) | 0x100; } else { #ifdef TARGET_X86_64 s->rex_x = (~vex2 >> 3) & 8; -- 2.16.3
Re: [Qemu-devel] [PATCH v3 3/7] s390x/cpumodel: Set up CPU model for AP device support
On 16/03/2018 00:24, Tony Krowiak wrote: A new CPU model feature and two new CPU model facilities are introduced to support AP devices for a KVM guest. CPU model features: 1. The KVM_S390_VM_CPU_FEAT_AP CPU model feature indicates that AP facilities are installed. This feature will be enabled by the kernel only if the AP facilities are installed on the linux host. This feature must be turned on from userspace to access AP devices from the KVM guest. The QEMU command line to turn this feature looks something like this: qemu-system-s390x ... -cpu xxx,ap=on CPU model facilities: 1. The S390_FEAT_AP_QUERY_CONFIG_INFO feature indicates the AP Query Configuration Information (QCI) facility is installed. This feature will be enabled by the kernel only if the QCI is installed on the host. 2. The S390_FEAT_AP_FACILITY_TEST feature indicates that the AP Facility Test (APFT) facility is installed. This feature will be enabled by the kernel only if the APFT facility is installed on the host. Signed-off-by: Tony Krowiak--- target/s390x/cpu_features.c |3 +++ target/s390x/cpu_features_def.h |3 +++ target/s390x/cpu_models.c |2 ++ target/s390x/gen-features.c |3 +++ target/s390x/kvm.c |1 + 5 files changed, 12 insertions(+), 0 deletions(-) diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c index a5619f2..1abe987 100644 --- a/target/s390x/cpu_features.c +++ b/target/s390x/cpu_features.c @@ -36,8 +36,10 @@ static const S390FeatDef s390_features[] = { FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"), FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"), FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"), +FEAT_INIT("qci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"), FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"), FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"), +FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"), FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"), FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"), FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"), @@ -125,6 +127,7 @@ static const S390FeatDef s390_features[] = { FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 2"), FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, "Collaborative-memory-management facility"), +FEAT_INIT("ap", S390_FEAT_TYPE_MISC, 0, "AP facilities installed"), FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit in general registers)"), FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 bit in parameter list)"), diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h index 7c5915c..8998b65 100644 --- a/target/s390x/cpu_features_def.h +++ b/target/s390x/cpu_features_def.h @@ -27,8 +27,10 @@ typedef enum { S390_FEAT_SENSE_RUNNING_STATUS, S390_FEAT_CONDITIONAL_SSKE, S390_FEAT_CONFIGURATION_TOPOLOGY, +S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_IPTE_RANGE, S390_FEAT_NONQ_KEY_SETTING, +S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_EXTENDED_TRANSLATION_2, S390_FEAT_MSA, S390_FEAT_LONG_DISPLACEMENT, @@ -118,6 +120,7 @@ typedef enum { /* Misc */ S390_FEAT_DAT_ENH_2, S390_FEAT_CMM, +S390_FEAT_AP, /* PLO */ S390_FEAT_PLO_CL, diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index cf82589..7e2af09 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -769,6 +769,8 @@ static void check_consistency(const S390CPUModel *model) { S390_FEAT_PRNO_TRNG_QRTCR, S390_FEAT_MSA_EXT_5 }, { S390_FEAT_PRNO_TRNG, S390_FEAT_MSA_EXT_5 }, { S390_FEAT_SIE_KSS, S390_FEAT_SIE_F2 }, +{ S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_AP }, +{ S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP }, }; int i; diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c index 0cdbc15..0d5b0f7 100644 --- a/target/s390x/gen-features.c +++ b/target/s390x/gen-features.c @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = { S390_FEAT_ADAPTER_INT_SUPPRESSION, S390_FEAT_EDAT_2, S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2, +S390_FEAT_AP_QUERY_CONFIG_INFO, Placing QCI feature in the full feature for z13 has for effect that: 1) it must be on the qemu command line. Do we really want this? QCI is always available when AP is available in the real hardware before z13. 2) AP exist since z10 and QCI since EC12 AFAIK Do we enable both only beginning with z13? +
Re: [Qemu-devel] [PULL for-2.12 0/1] tcg vector fix
On 6 April 2018 at 14:14, Richard Hendersonwrote: > Fixes the reported problem w/ ppc64 host (gcc 4.8.5) + aa64 guest. > > > r~ > > > The following changes since commit 0e87fdc966d05f4e5ad868034fcd8ee2a08ca62d: > > Update version for v2.12.0-rc2 release (2018-04-04 20:37:20 +0100) > > are available in the Git repository at: > > git://github.com/rth7680/qemu.git tags/pull-tcg-20180407 > > for you to fetch changes up to 6cb1d3b8517572031a22675280ec642972cdb395: > > tcg: Fix out-of-line generic vector compares (2018-04-06 23:08:50 +1000) > > > Fix generic host vector compares. > > > Richard Henderson (1): > tcg: Fix out-of-line generic vector compares > > accel/tcg/tcg-runtime-gvec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied, thanks. -- PMM
Re: [Qemu-devel] -icount changes physical address assignments in QEMU 2.10/2.11
On 6 April 2018 at 14:33, Peter Maydellwrote: > On this end I should try this with a 32-bit Linux host. I've now done this, and can reproduce the problem. So the issue is generic to 32-bit hosts. I'll see if I can figure out what's going wrong. In the meantime, you can probably work around it by building 64-bit binaries rather than 32-bit ones (they will perform a bit better I think, so unless you need to retain support for running on 32-bit Windows that would be a good idea anyway). thanks -- PMM
Re: [Qemu-devel] [PATCH 05/19] target/hppa: Unify specializations of OR
On 02/17/2018 09:31 PM, Richard Henderson wrote: > With decodetree.py, the specializations would conflict so we > must have a single entry point for all variants of OR. > > Signed-off-by: Richard Henderson> --- > target/hppa/translate.c | 108 > +++- > 1 file changed, 60 insertions(+), 48 deletions(-) > Reviewed-by: Bastian Koppelmann Cheers, Bastian
Re: [Qemu-devel] [PATCH] hw/sparc64/sun4u: Fix introspection by converting prom instance_init to realize
On 06.04.2018 16:41, Mark Cave-Ayland wrote: > On 05/04/18 10:32, Thomas Huth wrote: > >> The instance_init function of devices should always succeed to be able >> to introspect the device. However, the instance_init function of the >> "openprom" device can currently fail, for example like this: >> >> $ echo "{'execute':'qmp_capabilities'}"\ >> "{'execute':'device-list-properties',"\ >> " 'arguments':{'typename':'openprom'}}" \ >> | sparc64-softmmu/qemu-system-sparc64 -M sun4v,accel=qtest >> -qmp stdio >> {"QMP": {"version": {"qemu": {"micro": 91, "minor": 11, "major": 2}, >> "package": "build-all"}, "capabilities": []}} >> {"return": {}} >> RAMBlock "sun4u.prom" already registered, abort! >> Aborted (core dumped) >> >> This should not happen. Fix this problem by moving the affected code from >> instance_init into a realize function instead. >> >> Signed-off-by: Thomas Huth>> --- >> hw/sparc64/sun4u.c | 18 -- >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c >> index 2044a52..d62f5a2 100644 >> --- a/hw/sparc64/sun4u.c >> +++ b/hw/sparc64/sun4u.c >> @@ -425,13 +425,19 @@ static void prom_init(hwaddr addr, const char >> *bios_name) >> } >> } >> -static void prom_init1(Object *obj) >> +static void prom_realize(DeviceState *ds, Error **errp) >> { >> - PROMState *s = OPENPROM(obj); >> - SysBusDevice *dev = SYS_BUS_DEVICE(obj); >> + PROMState *s = OPENPROM(ds); >> + SysBusDevice *dev = SYS_BUS_DEVICE(ds); >> + Error *local_err = NULL; >> + >> + memory_region_init_ram_nomigrate(>prom, OBJECT(ds), "sun4u.prom", >> + PROM_SIZE_MAX, _err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> - memory_region_init_ram_nomigrate(>prom, obj, "sun4u.prom", >> PROM_SIZE_MAX, >> - _fatal); >> vmstate_register_ram_global(>prom); >> memory_region_set_readonly(>prom, true); >> sysbus_init_mmio(dev, >prom); >> @@ -446,6 +452,7 @@ static void prom_class_init(ObjectClass *klass, >> void *data) >> DeviceClass *dc = DEVICE_CLASS(klass); >> dc->props = prom_properties; >> + dc->realize = prom_realize; >> } >> static const TypeInfo prom_info = { >> @@ -453,7 +460,6 @@ static const TypeInfo prom_info = { >> .parent = TYPE_SYS_BUS_DEVICE, >> .instance_size = sizeof(PROMState), >> .class_init = prom_class_init, >> - .instance_init = prom_init1, >> }; > > Looks good to me: > > Reviewed-by: Mark Cave-Ayland > > I thought this would have been caught by the device introspect test, or > is this something you've found whilst trying to add sun4v to the list of > machines to include in the test? I've found it with this patch here: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg05033.html The current device-introspect-test only checks with the "none" machine, so it did not detect this problem yet since this only happens when you use the sun4v machine. Thomas
Re: [Qemu-devel] [PATCH v3 6/7] s390x/kvm: handle AP instruction interception
On 06/04/2018 16:08, Pierre Morel wrote: On 16/03/2018 00:24, Tony Krowiak wrote: If the CPU model indicates that AP facility is installed on the guest (i.e., -cpu ,ap=on), then the expectation is that the AP bus running in the guest will initialize; however, if the AP instructions are not being interpreted by the firmware, then they will be intercepted and routed back to QEMU for handling. If a handler is not defined to process the intercepted instruciton, then an operation exception will be injected into the guest, in which case the AP bus will not initialize. There are two situations where AP instructions will not be interpreted: 1. The guest is not configured with a vfio-ap device (i.e., -device vfio-ap,sysfsdev=$path-to-mdev). The realize function for the vfio-ap device enables interpretive execution of AP instructions. 2. The guest is a second level guest but the first level guest has not enabled interpretive execution. This patch introduces AP instruction handlers to ensure the AP bus module initializes on the guest when the AP facility is installed on the guest but AP instructions are not being interpreted. The logic incorporated is: * If the CPU model indicates AP instructions are installed * Set the status response code for the instruction to indicate that the APQN contained in the instruction is not valid. This is a valid response because there will be no devices configured for the guest in any of the above scenarios. * Else return an error from the handler. This will result in an operation being injected into the guest and the AP bus will not initialize on the guest. That is commensurate with how things work today. Signed-off-by: Tony Krowiak--- hw/vfio/ap.c | 45 ++ include/hw/s390x/ap-device.h | 6 + target/s390x/kvm.c | 14 + 3 files changed, 65 insertions(+), 0 deletions(-) diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c index b397bb1..88e744d 100644 --- a/hw/vfio/ap.c +++ b/hw/vfio/ap.c @@ -148,6 +148,51 @@ static void vfio_ap_unrealize(DeviceState *dev, Error **errp) kvm_s390_set_interpret_ap(0); } +int ap_device_handle_nqap(S390CPU *cpu) +{ + CPUS390XState *env = >env; + + if (s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x1; + + return 0; + } + + return -EOPNOTSUPP; +} + +int ap_device_handle_dqap(S390CPU *cpu) +{ + CPUS390XState *env = >env; + + if (s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x1; + + return 0; + } + + return -EOPNOTSUPP; +} + +int ap_device_handle_pqap(S390CPU *cpu) +{ + CPUS390XState *env = >env; + int fc = 4 & (env->regs[0] >> 24); + + /* + * The Query Configuration Information (QCI) function (fc == 4) does not + * set a response code in reg 1, so check for that along with the + * AP feature. + */ APFT-t must be taken care of, depending on APQN and APFT feature: If APFT feature is enabled it should report the right information for the existing AP If it is not enabled it should report OPERATION_EXCEPTION hum, sorry, only if interception of TAPQ-t is enabled and it is not. so forget it. Sorry for the noise. + if ((fc != 4) && s390_has_feat(S390_FEAT_AP)) { + env->regs[1] = 0x1; + + return 0; + } + + return -EOPNOTSUPP; +} + static Property vfio_ap_properties[] = { DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev), DEFINE_PROP_END_OF_LIST(), diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h index 693df90..d45ae38 100644 --- a/include/hw/s390x/ap-device.h +++ b/include/hw/s390x/ap-device.h @@ -11,6 +11,8 @@ #ifndef HW_S390X_AP_DEVICE_H #define HW_S390X_AP_DEVICE_H +#include "cpu.h" + #define AP_DEVICE_TYPE "ap-device" typedef struct APDevice { @@ -35,4 +37,8 @@ static inline APDevice *to_ap_dev(DeviceState *dev) #define AP_DEVICE_CLASS(klass) \ OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE) +int ap_device_handle_nqap(S390CPU *cpu); +int ap_device_handle_dqap(S390CPU *cpu); +int ap_device_handle_pqap(S390CPU *cpu); + #endif /* HW_S390X_AP_DEVICE_H */ diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 2812e28..a636394 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -50,6 +50,7 @@ #include "exec/memattrs.h" #include "hw/s390x/s390-virtio-ccw.h" #include "hw/s390x/s390-virtio-hcall.h" +#include "hw/s390x/ap-device.h" #ifndef DEBUG_KVM #define DEBUG_KVM 0 @@ -88,6 +89,9 @@ #define PRIV_B2_CHSC 0x5f #define PRIV_B2_SIGA 0x74 #define PRIV_B2_XSCH 0x76 +#define PRIV_B2_NQAP 0xad +#define PRIV_B2_DQAP 0xae +#define PRIV_B2_PQAP 0xaf #define PRIV_EB_SQBS 0x8a #define PRIV_EB_PCISTB 0xd0