Re: [RFC v2 3/6] cpus: extract out kvm-specific code to accel/kvm
Ciao Paolo,
On 7/7/20 7:01 PM, Paolo Bonzini wrote:
> On 07/07/20 15:58, Claudio Fontana wrote:
>> +static void kvm_kick_vcpu_thread(CPUState *cpu)
>> +{
>> +cpus_kick_thread(cpu);
>> +}
>> +
>
> I would just use cpus_kick_thread instead of wrapping it (and likewise
Here I left the common code in cpus_kick_thread, which is just used "as is" for
kvm,
but for hax f.e. we have:
void hax_kick_vcpu_thread(CPUState *cpu)
{
/*
* FIXME: race condition with the exit_request check in
* hax_vcpu_hax_exec
*/
cpu->exit_request = 1;
cpus_kick_thread(cpu);
}
Maybe we will need additional code that specializes the kick also for HVF?
I cannot run cpus_kick_thread for _all_ accels, because it is not good for
Windows (whpx and hax-windows).
> would provide a global function for noop synchronization).
I will look into providing global noops, thanks!
>
> start_vcpu_thread is also a candidate for abstraction, so that
Thanks, will look into this!
The accel that I expect will resist this will be tcg, but will try to fit it.
> kvm_start_vcpu_thread(CPUState *cpu) would be just
>
> qemu_start_vcpu_thread(cpu, "KVM", kvm_vcpu_thread_fn);
>
> Paolo
>
>
Ciao,
Claudio
Re: Failure prints during format or mounting a usb storage device
On Wed, Jul 8, 2020 at 12:29 AM Gerd Hoffmann wrote: > Hi, > > > > Why does 7ad3d51ebb8a522ffcad391c4bef281245739dde look at short-not-ok? > > > > Because the patch changes dev-storage to terminate the transfer if a > > short packet is received, so I figured 'short-not-ok' should affect > > that behavior. > > I don't think so. dev-storage should not need to look at short-not-ok. > > > I guess instead I could add another flag that only hcd-dwc2 sets. Does > > that sound OK to you? > > Sounds like that'll be another workaround. dev-storage should not need > to know what kind of host adapter is used ... > > A usb-storage transfer looks like this: > > (1) out transfer with the command (USB_MSDM_CBW) > (2) data transfer, might be: > - out (USB_MSDM_DATAOUT) for writes, or > - in (USB_MSDM_DATAIN) for reads, or > - nothing. > depending on the scsi command. > (3) in transfer with the status (USB_MSDM_CSW). > > (1) and (3) usually fit into a single usb packet. > (2) can be multiple usb packets. > > The critical case seem to be reads, i.e. we have in transfers for > both (2) and (3), and the transition from USB_MSDM_DATAIN state to > USB_MSDM_CSW state. > > What is the sequence of usb packets submitted by the guest to hcd-dwc2 > for reads? Where exactly does it expect a short transfer? > DWC2 needs a short transfer to indicate the end of all IN transfers that are not an exact multiple of max packet size. This means that the guest USB driver submits all read requests not with the actual length of the read, but with a length that is rounded up to a multiple of MPS. IIRC, without the dev-storage patch, the very first SCSI command would get stuck waiting for the CSW, because the CSW is not a multiple of MPS. I will have to work on getting a debug trace for you, I'll get back to you with that. Thanks, Paul
[PATCH] tests/docker: update toolchain set in debian-xtensa-cross
Switch to the prebuilt xtensa toolchains release 2020.07. Drop csp toolchain as the csp core is not a part of QEMU. Add de233_fpu and dsp3400 toolchains to enable DFPU and FPU2000 tests. Signed-off-by: Max Filippov --- tests/docker/dockerfiles/debian-xtensa-cross.docker | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/docker/dockerfiles/debian-xtensa-cross.docker b/tests/docker/dockerfiles/debian-xtensa-cross.docker index beb73f46baa6..ba4148299c5a 100644 --- a/tests/docker/dockerfiles/debian-xtensa-cross.docker +++ b/tests/docker/dockerfiles/debian-xtensa-cross.docker @@ -18,12 +18,12 @@ RUN apt-get update && \ git \ python3-minimal -ENV CPU_LIST csp dc232b dc233c -ENV TOOLCHAIN_RELEASE 2018.02 +ENV CPU_LIST dc232b dc233c de233_fpu dsp3400 +ENV TOOLCHAIN_RELEASE 2020.07 RUN for cpu in $CPU_LIST; do \ curl -#SL http://github.com/foss-xtensa/toolchain/releases/download/$TOOLCHAIN_RELEASE/x86_64-$TOOLCHAIN_RELEASE-xtensa-$cpu-elf.tar.gz \ | tar -xzC /opt; \ done -ENV PATH $PATH:/opt/$TOOLCHAIN_RELEASE/xtensa-dc232b-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dc233c-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-csp-elf/bin +ENV PATH $PATH:/opt/$TOOLCHAIN_RELEASE/xtensa-dc232b-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dc233c-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-de233_fpu-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dsp3400-elf/bin -- 2.20.1
Qemu core dump when stop guest with virtio-blk(remote storage) and iothread
Description of problem:
Qemu core dump when stop guest with virtio-blk(remote storage) and
iothread
Version-Release number pf selected component (if applicable):
kernel version:4.19.36.bsk.9-amd64
qemu-kvm version:QEMU emulator version 2.12.1
How reproducible:
100%
Steps to Reproduce:
1. Start guest, one virtio-blk(remote storage) and iothread parameter
/data00/qemu/x86_64-softmmu/qemu-system-x86_64
-name guest=instance-03,debug-threads=on
-kvm /dev/kvm
-S
-object
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-21-instance-03/master-key.aes
-machine pc-i440fx-2.12,accel=kvm,usb=off,dump-guest-core=off
-cpu qemu64,+kvm_pv_eoi
-m 7630
-mem-prealloc -mem-path /dev/hugepages/libvirt/qemu/21-instance-03 -smp
4,sockets=4,cores=1,threads=1
-object iothread,id=iothread1 -object iothread,id=iothread2 -object
iothread,id=iothread3 -object iothread,id=iothread4
-no-user-config
-nodefaults
-chardev socket,id=charmonitor,fd=21,server,nowait
-mon chardev=charmonitor,id=monitor,mode=control
-rtc base=utc,driftfix=slew
-global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown
-boot strict=on -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x7
-drive file=remote drive
path,format=raw,if=none,id=drive-virtio-disk0,cache=none,aio=native,throttling.bps-read=3000,throttling.bps-write=8000,throttling.iops-read=800,throttling.iops-write=400
-device
virtio-blk-pci,iothread=iothread1,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,write-cache=on
-chardev pty,id=charserial0 -device
isa-serial,chardev=charserial0,id=serial0
-device usb-tablet,id=input0,bus=usb.0,port=1 -vnc 0.0.0.0:0 -k en-us
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny
-device pvpanic,ioport=1285 -msg timestamp=on -d int,unimp,guest_errors
2.stop guest and start guest repeatedly
virsh qemu-monitor-command --domain instance-03 '{"execute":"stop",
"arguments":{}}'
virsh qemu-monitor-command --domain instance-03 '{"execute":"cont",
"arguments":{}}'
3.Actual results:
Qemu core dump with error msg:
(qemu) qemu: qemu_mutex_unlock_impl: Operation not permitted
Expected results:
Guest can stop and start successfully
Additional info:
[Current thread is 1 (Thread 0x7ff188abb700 (LWP 2229116))]
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x7ff19d73542a in __GI_abort () at abort.c:89
#2 0x560d55a3ca65 in error_exit (err=err@entry=1,
msg=msg@entry=0x560d566e9650 <__func__.18241> "qemu_mutex_unlock_impl") at
util/qemu-thread-posix.c:37
#3 0x560d55e76dc8 in qemu_mutex_unlock_impl
(mutex=mutex@entry=0x560d58456e30, file=file@entry=0x560d566e87df
"util/async.c", line=line@entry=509) at util/qemu-thread-posix.c:99
#4 0x560d55e71580 in aio_context_release (ctx=0x560d58456dd0) at
util/async.c:509
#5 0x560d55acf344 in virtio_blk_rw_complete (opaque=,
ret=0) at /data00/susieqemu/hw/block/virtio-blk.c:126
#6 0x560d55dcabc9 in blk_aio_complete (acb=0x7ff17c001bf0) at
block/block-backend.c:1345
#7 0x560d55e8a25b in coroutine_trampoline (i0=,
i1=) at util/coroutine-ucontext.c:116
#8 0x7ff19d745000 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#9 0x7ffd3691e190 in ?? ()
#10 0x in ?? ()
Backtrace stopped: Cannot access memory at address 0x7fef29cdd000
Thread 3 (Thread 0x7ff1a38f23c0 (LWP 2229113)):
#0 memory_region_ioeventfd_before (a=..., b=...) at
/data00/susieqemu/memory.c:185
#1 address_space_add_del_ioeventfds (fds_old_nb=65, fds_old=0x560d59733960,
fds_new_nb=64, fds_new=0x560d59ba6d10, as=0x560d598728b0) at
/data00/susieqemu/memory.c:794
#2 address_space_update_ioeventfds (as=as@entry=0x560d598728b0) at
/data00/susieqemu/memory.c:877
#3 0x560d55aabd98 in memory_region_transaction_commit () at
/data00/susieqemu/memory.c:1080
#4 0x560d55aae260 in memory_region_del_eventfd
(mr=mr@entry=0x560d599a2b40, addr=, size=size@entry=0,
match_data=, data=, e=) at
/data00/susieqemu/memory.c:2274
#5 0x560d55d113da in virtio_pci_ioeventfd_assign (d=0x560d599a1e70,
notifier=0x560d599db8d8, n=0, assign=) at
hw/virtio/virtio-pci.c:268
#6 0x560d55d14fef in virtio_bus_set_host_notifier (bus=0x560d599a9f68,
n=n@entry=0, assign=assign@entry=false) at hw/virtio/virtio-bus.c:289
#7 0x560d55ad2886 in virtio_blk_data_plane_stop (vdev=) at
/data00/susieqemu/hw/block/dataplane/virtio-blk.c:295
#8 0x560d55d1459e in virtio_bus_stop_ioeventfd (bus=0x560d599a9f68) at
hw/virtio/virtio-bus.c:246
#9 0x560d55b0
Re: [PATCH] tests/docker: update toolchain set in debian-xtensa-cross
Patchew URL: https://patchew.org/QEMU/20200708082347.27318-1-jcmvb...@gmail.com/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === - +...E +== +ERROR: test_pause (__main__.TestSingleBlockdevUnalignedLength) +-- +Traceback (most recent call last): + File "041", line 108, in test_pause --- Ran 104 tests -OK +FAILED (errors=1) TESTiotest-qcow2: 042 TESTcheck-qtest-x86_64: tests/qtest/numa-test TESTiotest-qcow2: 043 --- Not run: 259 Failures: 041 Failed 1 of 119 iotests make: *** [check-tests/check-block.sh] Error 1 Traceback (most recent call last): File "./tests/docker/docker.py", line 669, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=9ee680f810cb41a3b059b69570beb6de', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-_ehhlpye/src/docker-src.2020-07-08-04.29.35.26714:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=9ee680f810cb41a3b059b69570beb6de make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-_ehhlpye/src' make: *** [docker-run-test-quick@centos7] Error 2 real19m19.531s user0m9.446s The full log is available at http://patchew.org/logs/20200708082347.27318-1-jcmvb...@gmail.com/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH 00/21] target/xtensa: implement double precision FPU
Max Filippov writes: > On Tue, Jul 7, 2020 at 12:21 PM Alex Bennée wrote: >> Well it ran some xtensa tests thanks to the docker cross compiler >> support. Do you know what toolchains we need? >> >> Currently we have the following: >> >> ENV CPU_LIST csp dc232b dc233c >> ENV TOOLCHAIN_RELEASE 2018.02 >> >> RUN for cpu in $CPU_LIST; do \ >> curl -#SL >> http://github.com/foss-xtensa/toolchain/releases/download/$TOOLCHAIN_RELEASE/x86_64-$TOOLCHAIN_RELEASE-xtensa-$cpu-elf.tar.gz >> \ > > Oh, that's a familiar URL. Let me do the new batch of toolchains and > add FPU2000/DFPU configurations there. > And this is tests/docker/dockerfiles/debian-xtensa-cross.docker, > right? Yep - you could also tweak tests/tcg/configure.sh if you want to handle the presence of the toolchain in your local environment. We might want to expand the configurations emitted to tests/tcg/config-xtensa-softmmu.mak to handle the multiple binaries though unless a single binary can call the relevant toolchains with tweaks to CFLAGS? > I can add new configurations there as well and add one more patch > to this series. Cool ;-) -- Alex Bennée
Re: [PATCH] tests/docker: update toolchain set in debian-xtensa-cross
Max Filippov writes: > Switch to the prebuilt xtensa toolchains release 2020.07. > Drop csp toolchain as the csp core is not a part of QEMU. > Add de233_fpu and dsp3400 toolchains to enable DFPU and FPU2000 tests. Queued to misc/for-5.1-rc0, thanks. > > Signed-off-by: Max Filippov > --- > tests/docker/dockerfiles/debian-xtensa-cross.docker | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tests/docker/dockerfiles/debian-xtensa-cross.docker > b/tests/docker/dockerfiles/debian-xtensa-cross.docker > index beb73f46baa6..ba4148299c5a 100644 > --- a/tests/docker/dockerfiles/debian-xtensa-cross.docker > +++ b/tests/docker/dockerfiles/debian-xtensa-cross.docker > @@ -18,12 +18,12 @@ RUN apt-get update && \ > git \ > python3-minimal > > -ENV CPU_LIST csp dc232b dc233c > -ENV TOOLCHAIN_RELEASE 2018.02 > +ENV CPU_LIST dc232b dc233c de233_fpu dsp3400 > +ENV TOOLCHAIN_RELEASE 2020.07 > > RUN for cpu in $CPU_LIST; do \ > curl -#SL > http://github.com/foss-xtensa/toolchain/releases/download/$TOOLCHAIN_RELEASE/x86_64-$TOOLCHAIN_RELEASE-xtensa-$cpu-elf.tar.gz > \ > | tar -xzC /opt; \ > done > > -ENV PATH > $PATH:/opt/$TOOLCHAIN_RELEASE/xtensa-dc232b-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dc233c-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-csp-elf/bin > +ENV PATH > $PATH:/opt/$TOOLCHAIN_RELEASE/xtensa-dc232b-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dc233c-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-de233_fpu-elf/bin:/opt/$TOOLCHAIN_RELEASE/xtensa-dsp3400-elf/bin -- Alex Bennée
Re: [PATCH v4 08/12] hw/nvram: NPCM7xx OTP device model
On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
> This supports reading and writing OTP fuses and keys. Only fuse reading
> has been tested. Protection is not implemented.
>
> Reviewed-by: Avi Fishman
> Signed-off-by: Havard Skinnemoen
> ---
> hw/arm/npcm7xx.c | 32 +++
> hw/nvram/Makefile.objs | 1 +
> hw/nvram/npcm7xx_otp.c | 405 +
> include/hw/arm/npcm7xx.h | 3 +
> include/hw/nvram/npcm7xx_otp.h | 94
> 5 files changed, 535 insertions(+)
> create mode 100644 hw/nvram/npcm7xx_otp.c
> create mode 100644 include/hw/nvram/npcm7xx_otp.h
>
...
> +static void npcm7xx_init_fuses(NPCM7xxState *s)
> +{
> +NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
> +uint32_t value;
> +
> +value = tswap32(nc->disabled_modules);
> +npcm7xx_otp_array_write(&s->fuse_array, &value, 64, sizeof(value));
What is magic offset 64 for?
...
> diff --git a/hw/nvram/npcm7xx_otp.c b/hw/nvram/npcm7xx_otp.c
> new file mode 100644
> index 00..18908bc839
> --- /dev/null
> +++ b/hw/nvram/npcm7xx_otp.c
...
> +/* Common register write handler for both OTP classes. */
> +static void npcm7xx_otp_write(NPCM7xxOTPState *s, NPCM7xxOTPRegister reg,
> + uint32_t value)
> +{
> +switch (reg) {
> +case NPCM7XX_OTP_FST:
> +/* RDST is cleared by writing 1 to it. */
> +if (value & FST_RDST) {
> +s->regs[NPCM7XX_OTP_FST] &= ~FST_RDST;
> +}
> +/* Preserve read-only and write-one-to-clear bits */
> +value =
> +(value & ~FST_RO_MASK) | (s->regs[NPCM7XX_OTP_FST] &
> FST_RO_MASK);
Trivial to review as:
value &= ~FST_RO_MASK;
value |= s->regs[NPCM7XX_OTP_FST] & FST_RO_MASK;
> +break;
> +
> +case NPCM7XX_OTP_FADDR:
> +break;
> +
> +case NPCM7XX_OTP_FDATA:
> +/*
> + * This register is cleared by writing a magic value to it; no other
> + * values can be written.
> + */
> +if (value == FDATA_CLEAR) {
> +value = 0;
> +} else {
> +value = s->regs[NPCM7XX_OTP_FDATA];
> +}
> +break;
> +
> +case NPCM7XX_OTP_FCFG:
> +value = npcm7xx_otp_compute_fcfg(s->regs[NPCM7XX_OTP_FCFG], value);
> +break;
> +
> +case NPCM7XX_OTP_FCTL:
> +switch (value) {
> +case FCTL_READ_CMD:
> +npcm7xx_otp_read_array(s);
> +break;
> +
> +case FCTL_PROG_CMD1:
> +/*
> + * Programming requires writing two separate magic values to this
> + * register; this is the first one. Just store it so it can be
> + * verified later when the second magic value is received.
> + */
> +break;
> +
> +case FCTL_PROG_CMD2:
> +/*
> + * Only initiate programming if we received the first half of the
> + * command immediately before this one.
> + */
> +if (s->regs[NPCM7XX_OTP_FCTL] == FCTL_PROG_CMD1) {
> +npcm7xx_otp_program_array(s);
> +}
> +break;
> +
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: unrecognized FCNTL value 0x%" PRIx32 "\n",
> + DEVICE(s)->canonical_path, value);
> +break;
> +}
> +if (value != FCTL_PROG_CMD1) {
> +value = 0;
> +}
> +break;
> +
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: write to invalid offset 0x%zx\n",
> + DEVICE(s)->canonical_path, reg * sizeof(uint32_t));
> +return;
> +}
> +
> +s->regs[reg] = value;
> +}
...
> diff --git a/include/hw/nvram/npcm7xx_otp.h b/include/hw/nvram/npcm7xx_otp.h
> new file mode 100644
> index 00..b52330dadc
> --- /dev/null
> +++ b/include/hw/nvram/npcm7xx_otp.h
> @@ -0,0 +1,94 @@
> +/*
> + * Nuvoton NPCM7xx OTP (Fuse Array) Interface
> + *
> + * Copyright 2020 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +#ifndef NPCM7XX_OTP_H
> +#define NPCM7XX_OTP_H
> +
> +#include "exec/memory.h"
> +#include "hw/sysbus.h"
> +
> +/* Each OTP module holds 8192 bits of one-time programmable storage */
> +#define NPCM7XX_OTP_ARRAY_BITS (8192)
> +#define NPCM7XX_OTP_ARRAY_BYTES (NPCM7XX_OTP_ARRAY_BITS / 8)
You could replace 8 by BITS_PER_BYTE.
> +
> +/**
> + * enum NPCM7xxOTPRegister - 32-bit register
Migrating custom qemu.org infrastructure to GitLab
Dear QEMU community, QEMU currently has a static website, wiki, git repo hosting, and special-purpose cronjobs/containers running in VMs. There is currently no system administrator looking after our infrastructure so the most urgent tasks fall onto me, the remainder are ignored/postponed. The current situation exposes qemu.org to the risk of downtime and security issues. Another limitation is that each piece of infrastructure is managed separately and one-time contributors cannot easily propose changes because they do not have access. It would be much better to use our existing code review process so that anyone can make changes to infrastructure by sending a patch. GitLab's Continuous Integration (CI) system provides a powerful way to perform actions defined in yaml files in qemu.git. This includes running scripts, builds, publishing build artifacts, etc. We have already begun using it for automated builds and tests: https://gitlab.com/qemu-project/qemu/-/blob/master/.gitlab-ci.yml GitLab also offers git repo hosting, wikis, issue tracking, and other features. It is possible to log in using GitHub, Google, or Twitter single sign-on if you do not want to create another account. As more open source projects use GitLab it becomes easier for one-time contributors who will already be familiar with the tools from other projects. Here is a full list of GitLab's features: https://about.gitlab.com/features/ GitLab offers the gold/ultimate tier for free to open source projects: https://about.gitlab.com/solutions/open-source/ GitLab itself is open source and can be self-hosted if we decide to leave in the future. With this in mind I propose moving qemu.org infrastructure to GitLab incrementally. This needs to be done carefully to avoid disruption and only where GitLab meets the requirements. The QEMU project will continue to have access to cloud hosting for running custom infrastructure or adding runners to GitLab CI to improve CI performance. The following infrastructure components can be considered for GitLab migration: 1. qemu-web.git static site generation. GitLab CI/CD can build the static website on each qemu-web.git commit and publish the HTML artifacts. 2. wiki.qemu.org is a MediaWiki instance. Account creation is a hurdle to one-time or new contributors. It is unclear whether GitLab's wiki is expressive enough for a lossless conversion of the existing QEMU wiki. Any volunteers interested in evaluating the wiki migration would be appreciated. 3. Git repo hosting is a core feature of GitLab and we already have a qemu.git mirror. Hosting the repos on GitLab reduces the need for qemu.org ssh access. 4. The QEMU release process can be moved to CI/CD so that publishing stable releases and release candidates is less dependent on one committer's machine or scripts. 5. Issue tracking. Launchpad more or less works, but the login always bothers me. If we move git repo hosting then it makes sense to do issue tracking on GitLab too. There is a snowball effect where the experience is improved the more GitLab features we use, so I hope that most of these migrations will be possible. Next steps: * If you have an interest in one or more of these infrastructure components, please join the discussion. * If there are no volunteers for an infrastructure component I'll slowly work my way through evaluating GitLab and propose migrations Stefan
Re: qemu-system-ppc64 abort()s with pcie bridges
On Wed, 8 Jul 2020 10:03:47 +0200 Thomas Huth wrote: > > Hi, > > qemu-system-ppc64 currently abort()s when it is started with a pcie > bridge device: > > $ qemu-system-ppc64 -M pseries-5.1 -device pcie-pci-bridge > Unexpected error in object_property_find() at qom/object.c:1240: > qemu-system-ppc64: -device pcie-pci-bridge: Property '.chassis_nr' not found > Aborted (core dumped) > > or: > > $ qemu-system-ppc64 -M pseries -device dec-21154-p2p-bridge > Unexpected error in object_property_find() at qom/object.c:1240: > qemu-system-ppc64: -device dec-21154-p2p-bridge: Property '.chassis_nr' > not found > Aborted (core dumped) > > That's kind of ugly, and it shows up as error when running > scripts/device-crash-test. Is there an easy way to avoid the abort() and > fail more gracefully here? > And even worse, this can tear down a running guest with hotplug :\ (qemu) device_add pcie-pci-bridge Unexpected error in object_property_find() at /home/greg/Work/qemu/qemu-ppc/qom/object.c:1240: Property '.chassis_nr' not found Aborted (core dumped) This is caused by recent commit: commit 7ef1553dac8ef8dbe547b58d7420461a16be0eeb Author: Markus Armbruster Date: Tue May 5 17:29:25 2020 +0200 spapr_pci: Drop some dead error handling chassis_from_bus() uses object_property_get_uint() to get property "chassis_nr" of the bridge device. Failure would be a programming error. Pass &error_abort, and simplify its callers. Cc: David Gibson Cc: qemu-...@nongnu.org Signed-off-by: Markus Armbruster Acked-by: David Gibson Reviewed-by: Greg Kurz Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Paolo Bonzini Message-Id: <20200505152926.18877-18-arm...@redhat.com> Before that, we would simply print the "chassir_nr not found" error, and in case of a cold plugged device exit. The root cause is that the sPAPR PCI code assumes that a PCI bridge has a "chassir_nr" property, ie. it is a standard PCI bridge. Other PCI bridge types don't have that. Not sure yet why this information is required, I'll check LoPAPR. In the meantime, since we're in soft freeze, I guess we should revert Markus's patch and add a big fat comment to explain what's going on and maybe change the error message to something more informative, eg. "PCIE-to-PCI bridges are not supported". Thoughts ? > Thomas >
[Bug 1886793] [NEW] "go install" command fails while running inside s390x docker container on x86_64 host using qemu
Public bug reported:
Steps to reproduce the issue:
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
Build the following Docker Image using following Dockerfile.s390x using
command docker build -t test/crossbuild:latest-s390x -f Dockerfile.s390x
.
Dockerfile.s390x
##
FROM alpine:3.11 as qemu
ARG QEMU_VERSION=5.0.0-2
ARG QEMU_ARCHS="s390x"
RUN apk --update add curl
#Enable non-native runs on amd64 architecture hosts
RUN for i in ${QEMU_ARCHS}; do curl -L
https://github.com/multiarch/qemu-user-static/releases/download/v${QEMU_VERSION}/qemu-${i}-static.tar.gz
| tar zxvf - -C /usr/bin; done
RUN chmod +x /usr/bin/qemu-*
FROM s390x/golang:1.14.2-alpine3.11
MAINTAINER LoZ Open Source Ecosystem
(https://www.ibm.com/developerworks/community/groups/community/lozopensource)
ARG MANIFEST_TOOL_VERSION=v1.0.2
#Enable non-native builds of this image on an amd64 hosts.
#This must be the first RUN command in this file!
COPY --from=qemu /usr/bin/qemu-*-static /usr/bin/
#Install su-exec for use in the entrypoint.sh (so processes run as the right
user)
#Install bash for the entry script (and because it's generally useful)
#Install curl to download glide
#Install git for fetching Go dependencies
#Install ssh for fetching Go dependencies
#Install mercurial for fetching go dependencies
#Install wget since it's useful for fetching
#Install make for building things
#Install util-linux for column command (used for output formatting).
#Install grep and sed for use in some Makefiles (e.g. pulling versions out of
glide.yaml)
#Install shadow for useradd (it allows to use big UID)
RUN apk update && apk add --no-cache su-exec curl bash git openssh mercurial
make wget util-linux tini file grep sed shadow
RUN apk upgrade --no-cache
#Disable ssh host key checking
RUN echo 'Host *' >> /etc/ssh/ssh_config \
&& echo 'StrictHostKeyChecking no' >> /etc/ssh/ssh_config
#Disable cgo so that binaries we build will be fully static.
ENV CGO_ENABLED=0
#Recompile the standard library with cgo disabled. This prevents the standard
library from being
#marked stale, causing full rebuilds every time.
RUN go install -v std
#Install glide
RUN go get github.com/Masterminds/glide
ENV GLIDE_HOME /home/user/.glide
#Install dep
RUN go get github.com/golang/dep/cmd/dep
#Install ginkgo CLI tool for running tests
RUN go get github.com/onsi/ginkgo/ginkgo
#Install linting tools.
RUN wget -O - -q
https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s
v1.20.0
RUN golangci-lint --version
#Install license checking tool.
RUN go get github.com/pmezard/licenses
#Install tool to merge coverage reports.
RUN go get github.com/wadey/gocovmerge
#Install CLI tool for working with yaml files
RUN go get github.com/mikefarah/yaml
#Delete all the Go sources that were downloaded, we only rely on the binaries
RUN rm -rf /go/src/*
#Install vgo (should be removed once we take Go 1.11)
RUN go get -u golang.org/x/vgo
#Ensure that everything under the GOPATH is writable by everyone
RUN chmod -R 777 $GOPATH
RUN curl -sSL
https://github.com/estesp/manifest-tool/releases/download/${MANIFEST_TOOL_VERSION}/manifest-tool-linux-s390x
> manifest-tool && \
chmod +x manifest-tool && \
mv manifest-tool /usr/bin/
COPY entrypoint.sh /usr/local/bin/entrypoint.sh
ENTRYPOINT ["/sbin/tini", "--", "/usr/local/bin/entrypoint.sh"]
##
The build just hangs at RUN go install -v std
Also, while running the same command inside s390x container on x86_64
host, error Illegal instruction (core dumped) is thrown.
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
docker run -it -v /home/test/qemu-s390x-static:/usr/bin/qemu-s390x-
static s390x/golang:1.14.2-alpine3.11
Inside s390x container:
apk update && apk add --no-cache su-exec curl bash git openssh mercurial make
wget util-linux tini file grep sed shadow
apk upgrade --no-cache
#Disable cgo so that binaries we build will be fully static.
export CGO_ENABLED=0
go install -v std
This gives the following error:
Illegal instruction (core dumped)
Environment:
x86_64 Ub18.04 4.15.0-101-generic Ubuntu SMP x86_64 GNU/Linux
QEMU user static version: 5.0.0-2
Container application: Docker
Client: Docker Engine - Community
Version: 19.03.11
API version: 1.40
Go version:go1.13.10
Git commit:42e35e61f3
Built: Mon Jun 1 09:12:22 2020
OS/Arch: linux/amd64
Experimental: false
Server: Docker Engine - Community
Engine:
Version: 19.03.11
API version: 1.40 (minimum version 1.12)
Go version: go1.13.10
Git commit: 42e35e61f3
Built:Mon Jun 1 09:10:54 2020
OS/Arch: linux/amd64
Experimental: false
containerd:
Version:
[Bug 1886793] Re: "go install" command fails while running inside s390x docker container on x86_64 host using qemu
One more thing which I tried:
I installed qemu on x86 Ubuntu host with apt-get install.
Extracted s390x go 1.14.2 binaries on the same. Ran the following commands:
root:~ wget -q https://storage.googleapis.com/golang/go1.14.2.linux-s390x.tar.gz
root@:~# chmod ugo+r go1.14.2.linux-s390x.tar.gz
root@:~# rm -rf /usr/local/go /usr/bin/go
root@:~# tar -C /usr/local -xzf go1.14.2.linux-s390x.tar.gz
root@:~# ln -sf /usr/local/go/bin/go /usr/bin/
root@:~# ln -sf /usr/local/go/bin/gofmt /usr/bin/
root@:~# ln -sf /usr/bin/gcc /usr/bin/s390x-linux-gnu-gcc
root@:~# go version
/lib/ld64.so.1: No such file or directory
root@:~# qemu-s390x /usr/bin/go version
/lib/ld64.so.1: No such file or directory
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1886793
Title:
"go install" command fails while running inside s390x docker container
on x86_64 host using qemu
Status in QEMU:
New
Bug description:
Steps to reproduce the issue:
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
Build the following Docker Image using following Dockerfile.s390x
using command docker build -t test/crossbuild:latest-s390x -f
Dockerfile.s390x .
Dockerfile.s390x
##
FROM alpine:3.11 as qemu
ARG QEMU_VERSION=5.0.0-2
ARG QEMU_ARCHS="s390x"
RUN apk --update add curl
#Enable non-native runs on amd64 architecture hosts
RUN for i in ${QEMU_ARCHS}; do curl -L
https://github.com/multiarch/qemu-user-static/releases/download/v${QEMU_VERSION}/qemu-${i}-static.tar.gz
| tar zxvf - -C /usr/bin; done
RUN chmod +x /usr/bin/qemu-*
FROM s390x/golang:1.14.2-alpine3.11
MAINTAINER LoZ Open Source Ecosystem
(https://www.ibm.com/developerworks/community/groups/community/lozopensource)
ARG MANIFEST_TOOL_VERSION=v1.0.2
#Enable non-native builds of this image on an amd64 hosts.
#This must be the first RUN command in this file!
COPY --from=qemu /usr/bin/qemu-*-static /usr/bin/
#Install su-exec for use in the entrypoint.sh (so processes run as the right
user)
#Install bash for the entry script (and because it's generally useful)
#Install curl to download glide
#Install git for fetching Go dependencies
#Install ssh for fetching Go dependencies
#Install mercurial for fetching go dependencies
#Install wget since it's useful for fetching
#Install make for building things
#Install util-linux for column command (used for output formatting).
#Install grep and sed for use in some Makefiles (e.g. pulling versions out of
glide.yaml)
#Install shadow for useradd (it allows to use big UID)
RUN apk update && apk add --no-cache su-exec curl bash git openssh mercurial
make wget util-linux tini file grep sed shadow
RUN apk upgrade --no-cache
#Disable ssh host key checking
RUN echo 'Host *' >> /etc/ssh/ssh_config \
&& echo 'StrictHostKeyChecking no' >> /etc/ssh/ssh_config
#Disable cgo so that binaries we build will be fully static.
ENV CGO_ENABLED=0
#Recompile the standard library with cgo disabled. This prevents the
standard library from being
#marked stale, causing full rebuilds every time.
RUN go install -v std
#Install glide
RUN go get github.com/Masterminds/glide
ENV GLIDE_HOME /home/user/.glide
#Install dep
RUN go get github.com/golang/dep/cmd/dep
#Install ginkgo CLI tool for running tests
RUN go get github.com/onsi/ginkgo/ginkgo
#Install linting tools.
RUN wget -O - -q
https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s
v1.20.0
RUN golangci-lint --version
#Install license checking tool.
RUN go get github.com/pmezard/licenses
#Install tool to merge coverage reports.
RUN go get github.com/wadey/gocovmerge
#Install CLI tool for working with yaml files
RUN go get github.com/mikefarah/yaml
#Delete all the Go sources that were downloaded, we only rely on the binaries
RUN rm -rf /go/src/*
#Install vgo (should be removed once we take Go 1.11)
RUN go get -u golang.org/x/vgo
#Ensure that everything under the GOPATH is writable by everyone
RUN chmod -R 777 $GOPATH
RUN curl -sSL
https://github.com/estesp/manifest-tool/releases/download/${MANIFEST_TOOL_VERSION}/manifest-tool-linux-s390x
> manifest-tool && \
chmod +x manifest-tool && \
mv manifest-tool /usr/bin/
COPY entrypoint.sh /usr/local/bin/entrypoint.sh
ENTRYPOINT ["/sbin/tini", "--", "/usr/local/bin/entrypoint.sh"]
##
The build just hangs at RUN go install -v std
Also, while running the same command inside s390x container on x86_64
host, error Illegal instruction (core dumped) is thrown.
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
docker run -it -v /h
Re: [PULL v2 00/41] virtio,acpi: features, fixes, cleanups.
On Wed, 8 Jul 2020 at 07:45, Michael S. Tsirkin wrote: > > On Tue, Jul 07, 2020 at 06:50:55PM +0100, Peter Maydell wrote: > > Sure. (You can always just resend a new v2 cover letter without > > all the patches; that's what most people do for minor respins.) > > As a reply-to or as a new thread? New thread. Just like sending a complete new pullreq, only you only send the cover letter to the list, not all the patch mails. Some people send "cover letter plus the mail for the patch that changed" if they feel the changes were significant. -- PMM
Re: [PATCH] cpu: Add starts_halted() method
On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Thiago,
>
> On 7/8/20 1:28 AM, Thiago Jung Bauermann wrote:
> >
> > Hello Eduardo,
> >
> > Eduardo Habkost writes:
> >
> >> On Tue, Jul 07, 2020 at 05:43:33PM -0300, Thiago Jung Bauermann wrote:
> >>> PowerPC sPAPRs CPUs start in the halted state, but generic QEMU code
> >>> assumes that CPUs start in the non-halted state. spapr_reset_vcpu()
> >>> attempts to rectify this by setting CPUState::halted to 1. But that's too
> >>> late for hotplugged CPUs in a machine configured with 2 or mor threads per
> >>> core.
> >>>
> >>> By then, other parts of QEMU have already caused the vCPU to run in an
> >>> unitialized state a couple of times. For example, ppc_cpu_reset() calls
> >>> ppc_tlb_invalidate_all(), which ends up calling async_run_on_cpu(). This
> >>> kicks the new vCPU while it has CPUState::halted = 0, causing QEMU to
> >>> issue
> >>> a KVM_RUN ioctl on the new vCPU before the guest is able to make the
> >>> start-cpu RTAS call to initialize its register state.
> >>>
> >>> This doesn't seem to cause visible issues for regular guests, but on a
> >>> secure guest running under the Ultravisor it does. The Ultravisor relies
> >>> on
> >>> being able to snoop on the start-cpu RTAS call to map vCPUs to guests, and
> >>> this issue causes it to see a stray vCPU that doesn't belong to any guest.
> >>>
> >>> Fix by adding a starts_halted() method to the CPUState class, and making
> >>> it
> >>> return 1 if the machine is an sPAPR guest.
> >>>
> >>> Signed-off-by: Thiago Jung Bauermann
> >> [...]
> >>> +static uint32_t ppc_cpu_starts_halted(void)
> >>> +{
> >>> +SpaprMachineState *spapr =
> >>> +(SpaprMachineState *) object_dynamic_cast(qdev_get_machine(),
> >>> + TYPE_SPAPR_MACHINE);
> >>
> >> Wouldn't it be simpler to just implement this as a MachineClass
> >> boolean field? e.g.:
>
> Class boolean field certainly sounds better, but I am not sure this
> is a property of the machine. Rather the arch? So move the field
> to CPUClass? Maybe not, let's discuss :)
It is absolutely a property of the machine. e.g. I don't think we
want this for powernv. pseries is a bit of a special case since it is
explicitly a paravirt platform. But even for emulated hardware, the
board can absolutely strap things so that cpus do or don't start
immediately.
--
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: [PATCH] trivial: Remove trailing whitespaces
On Mon, Jul 06, 2020 at 06:23:00PM +0200, Christophe de Dinechin wrote: > There are a number of unnecessary trailing whitespaces that have > accumulated over time in the source code. They cause stray changes > in patches if you use tools that automatically remove them. > > Tested by doing a `git diff -w` after the change. > > This could probably be turned into a pre-commit hook. > For HVF bits, Reviewed-by: Roman Bolshakov Thanks, Roman
[Bug 1886811] [NEW] systemd complains Failed to enqueue loopback interface start request: Operation not supported
Public bug reported: This symptom seems similar to https://bugs.launchpad.net/qemu/+bug/1823790 Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture qemu version: latest git of git commit hash eb2c66b10efd2b914b56b20ae90655914310c925 compiled with "./configure --static --disable-system" Down stream bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 Bug report (closed) to systemd: https://github.com/systemd/systemd/issues/16359 systemd in armhf and armel (both little endian 32-bit) containers fail to start with Failed to enqueue loopback interface start request: Operation not supported How to reproduce on Debian (and probably Ubuntu): mmdebstrap --components="main contrib non-free" --architectures=armhf --variant=important bullseye /var/lib/machines/armhf-bullseye systemd-nspawn -D /var/lib/machines/armhf-bullseye -b When "armhf" architecture is replaced with "mips" (32-bit big endian) or "ppc64" (64-bit big endian), the container starts up fine. The same symptom is also observed with "powerpc" (32-bit big endian) architecture. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1886811 Title: systemd complains Failed to enqueue loopback interface start request: Operation not supported Status in QEMU: New Bug description: This symptom seems similar to https://bugs.launchpad.net/qemu/+bug/1823790 Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture qemu version: latest git of git commit hash eb2c66b10efd2b914b56b20ae90655914310c925 compiled with "./configure --static --disable-system" Down stream bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 Bug report (closed) to systemd: https://github.com/systemd/systemd/issues/16359 systemd in armhf and armel (both little endian 32-bit) containers fail to start with Failed to enqueue loopback interface start request: Operation not supported How to reproduce on Debian (and probably Ubuntu): mmdebstrap --components="main contrib non-free" --architectures=armhf --variant=important bullseye /var/lib/machines/armhf-bullseye systemd-nspawn -D /var/lib/machines/armhf-bullseye -b When "armhf" architecture is replaced with "mips" (32-bit big endian) or "ppc64" (64-bit big endian), the container starts up fine. The same symptom is also observed with "powerpc" (32-bit big endian) architecture. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1886811/+subscriptions
Re: [PATCH] hw/register: Document register_init_block @memory_size
Le 07/07/2020 à 08:23, Philippe Mathieu-Daudé a écrit :
> Document the 'memory_size' argument of register_init_block().
>
> Fixes: a74229597e ("register: Add block initialise helper")
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/hw/register.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/hw/register.h b/include/hw/register.h
> index 5d2c565ae0..fdac5e69b5 100644
> --- a/include/hw/register.h
> +++ b/include/hw/register.h
> @@ -181,6 +181,7 @@ uint64_t register_read_memory(void *opaque, hwaddr addr,
> unsigned size);
> * @data: Array to use for register data, must already be allocated
> * @ops: Memory region ops to access registers.
> * @debug enabled: turn on/off verbose debug information
> + * @memory_size: Size of the memory region
> * returns: A structure containing all of the registers and an initialized
> * memory region (r_array->mem) the caller should add to a
> container.
> */
>
Reviewed-by: Laurent Vivier
Re: [PATCH v4 04/11] target/ppc: convert vmuluwm to tcg_gen_gvec_mul
On Wed, Jul 01, 2020 at 06:43:39PM -0500, Lijun Pan wrote:
> Convert the original implementation of vmuluwm to the more generic
> tcg_gen_gvec_mul.
>
> Signed-off-by: Lijun Pan
Applied to ppc-for-5.2.
> ---
> Reviewed-by: Richard Henderson
> v3: newly introduced
>
> target/ppc/helper.h | 1 -
> target/ppc/int_helper.c | 13 -
> target/ppc/translate/vmx-impl.inc.c | 2 +-
> 3 files changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 2dfa1c6942..69416b6d7c 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -184,7 +184,6 @@ DEF_HELPER_3(vmulosw, void, avr, avr, avr)
> DEF_HELPER_3(vmuloub, void, avr, avr, avr)
> DEF_HELPER_3(vmulouh, void, avr, avr, avr)
> DEF_HELPER_3(vmulouw, void, avr, avr, avr)
> -DEF_HELPER_3(vmuluwm, void, avr, avr, avr)
> DEF_HELPER_3(vslo, void, avr, avr, avr)
> DEF_HELPER_3(vsro, void, avr, avr, avr)
> DEF_HELPER_3(vsrv, void, avr, avr, avr)
> diff --git a/target/ppc/int_helper.c b/target/ppc/int_helper.c
> index be53cd6f68..bd3e6d7cc7 100644
> --- a/target/ppc/int_helper.c
> +++ b/target/ppc/int_helper.c
> @@ -523,19 +523,6 @@ void helper_vprtybq(ppc_avr_t *r, ppc_avr_t *b)
> r->VsrD(0) = 0;
> }
>
> -#define VARITH_DO(name, op, element)\
> -void helper_v##name(ppc_avr_t *r, ppc_avr_t *a, ppc_avr_t *b) \
> -{ \
> -int i; \
> -\
> -for (i = 0; i < ARRAY_SIZE(r->element); i++) { \
> -r->element[i] = a->element[i] op b->element[i]; \
> -} \
> -}
> -VARITH_DO(muluwm, *, u32)
> -#undef VARITH_DO
> -#undef VARITH
> -
> #define VARITHFP(suffix, func) \
> void helper_v##suffix(CPUPPCState *env, ppc_avr_t *r, ppc_avr_t *a, \
>ppc_avr_t *b) \
> diff --git a/target/ppc/translate/vmx-impl.inc.c
> b/target/ppc/translate/vmx-impl.inc.c
> index 403ed3a01c..6e79ffa650 100644
> --- a/target/ppc/translate/vmx-impl.inc.c
> +++ b/target/ppc/translate/vmx-impl.inc.c
> @@ -801,7 +801,7 @@ static void trans_vclzd(DisasContext *ctx)
> GEN_VXFORM(vmuloub, 4, 0);
> GEN_VXFORM(vmulouh, 4, 1);
> GEN_VXFORM(vmulouw, 4, 2);
> -GEN_VXFORM(vmuluwm, 4, 2);
> +GEN_VXFORM_V(vmuluwm, MO_32, tcg_gen_gvec_mul, 4, 2);
> GEN_VXFORM_DUAL(vmulouw, PPC_ALTIVEC, PPC_NONE,
> vmuluwm, PPC_NONE, PPC2_ALTIVEC_207)
> GEN_VXFORM(vmulosb, 4, 4);
--
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: [PATCH] cpu: Add starts_halted() method
On Wed, 8 Jul 2020 at 12:12, David Gibson wrote: > On Wed, Jul 08, 2020 at 10:38:29AM +0200, Philippe Mathieu-Daudé wrote: > > Class boolean field certainly sounds better, but I am not sure this > > is a property of the machine. Rather the arch? So move the field > > to CPUClass? Maybe not, let's discuss :) > > It is absolutely a property of the machine. e.g. I don't think we > want this for powernv. pseries is a bit of a special case since it is > explicitly a paravirt platform. But even for emulated hardware, the > board can absolutely strap things so that cpus do or don't start > immediately. It's a property of the individual CPU, I think. One common setup for Arm systems is that the primary CPU starts powered up but the secondaries all start powered down. The original bug as described in the commit message sounds to me like something we should look to fix in the implementation of async_run_on_cpu() -- it shouldn't cause a CPU that's halfway through reset to do a KVM_RUN or otherwise run guest code, whether that CPU is going to start powered-up or powered-down. thanks -- PMM
Re: [RFC v2 1/6] cpus: extract out TCG-specific code to accel/tcg
On 7/7/20 6:58 PM, Paolo Bonzini wrote:
> On 07/07/20 15:58, Claudio Fontana wrote:
>> +
>> +CpusAccel tcg_cpus = {
>> +.create_vcpu_thread = tcg_start_vcpu_thread,
>> +.kick_vcpu_thread = tcg_kick_vcpu_thread,
>> +
>> +.synchronize_post_reset = tcg_cpu_synchronize_noop,
>> +.synchronize_post_init = tcg_cpu_synchronize_noop,
>> +.synchronize_state = tcg_cpu_synchronize_noop,
>> +.synchronize_pre_loadvm = tcg_cpu_synchronize_noop,
>> +};
>
> Could this struct reside in AccelClass instead, so that there's no need
> to register the operations explicitly?
This I tried a few times before, you can dig the history a bit of the comments.
The result of my attempts is that you end up replacing an "explicit
registration", with an explicit internal copy from a structure defined in
kvm-cpus.c
(for example) to the accelerator state, so you just switch one registration for
another.
The result is a pile of boilerplate of no use that is just confusing for the
reader.
> We could still cache it in a
> global variable in accel_init_machine, in order to avoid pointer chasing.
Indeed, the "cache" though ends up being the only useful thing,
and it can be even more confusing for the reader to see the whole machinery in
AccelClass.
>
> Thanks,
>
> Paolo
>
Still if you are interested to see what the result ends up being I can share
it, will need some fiddling..
Ciao,
Claudio
Re: [PATCH 3/3] cpu-timers, icount: new modules
On 7/8/20 4:34 PM, Paolo Bonzini wrote: > On 29/06/20 11:35, Claudio Fontana wrote: >> refactoring of cpus.c continues with cpu timer state extraction. >> >> cpu-timers: responsible for the cpu timers state, and for access to >> cpu clocks and ticks. >> >> icount: counts the TCG instructions executed. As such it is specific to >> the TCG accelerator. Therefore, it is built only under CONFIG_TCG. >> >> One complication is due to qtest, which misuses icount to warp time >> (qtest_clock_warp). In order to solve this problem, detach instead qtest >> from icount, and use a trivial separate counter for it. >> >> This requires fixing assumptions scattered in the code that >> qtest_enabled() implies icount_enabled(). >> >> No functionality change. >> >> Signed-off-by: Claudio Fontana >> Reviewed-by: Alex Bennée > > Claudio, > > this weirdly enough causes iotest 267 (i.e. basically vmstate > save/restore) to break on s390: > > +Unexpected storage key flag data: 0 > +error while loading state for instance 0x0 of device 's390-skeys' > +Error: Error -22 while loading VM state > > Bisectable, 100% failure rate, etc. :( Can you split the patch in > multiple parts, specifically separating any rename or introducing of > includes from the final file move? Hi Paolo, will take a look! Is this captured by some travis / cirrus-ci / anything I can easily see the result of? > > Also, the patch breaks --disable-tcg, which is easily fixed by changing > the prototype for icount_enabled() to > > #if defined CONFIG_TCG || !defined NEED_CPU_H > extern bool icount_enabled(void); > #else > #define icount_enabled() 0 > #endif > > (This way, more TCG-only code in cpus.c gets elided). You can integrate > this change in the next version. > > Paolo > Weird, I tested with --disable-tcg explicitly (but may be some time ago now, as I constantly rebased). Will take a look at the introduction of this #defines in place of variables, as this mechanisms will not work in the future for target-specific modules. Ciao, will let you know what I find, Claudio
Re: [PATCH 3/3] cpu-timers, icount: new modules
On 7/8/20 5:12 PM, Paolo Bonzini wrote: > On 08/07/20 17:07, Thomas Huth wrote: >>> Nope, unfortunately we don't have an s390 CI. But if you can get your >>> hands on one, just "./configure --target-list=s390x-softmmu && make && >>> make check-block" will show it. >> >> We've got a s390x builder on Travis ... or is this only about the s390x >> target? > > It is about s390 host. But Travis is not covering all submitted > patches, is it? > > Paolo > Probably something in-flight (in the queues), because for me it was all green for [s390] on travis, supposing that the tests run as part of travis CI include check-block. Thanks, CLaudio
Re: [PATCH 3/3] cpu-timers, icount: new modules
On 7/8/20 5:05 PM, Paolo Bonzini wrote: > On 08/07/20 17:00, Claudio Fontana wrote: >>> Bisectable, 100% failure rate, etc. :( Can you split the patch in >>> multiple parts, specifically separating any rename or introducing of >>> includes from the final file move? >> Hi Paolo, >> >> will take a look! >> >> Is this captured by some travis / cirrus-ci / anything I can easily see the >> result of? >> >> > > Nope, unfortunately we don't have an s390 CI. But if you can get your > hands on one, just "./configure --target-list=s390x-softmmu && make && > make check-block" will show it. > >>> >>> #if defined CONFIG_TCG || !defined NEED_CPU_H >>> extern bool icount_enabled(void); >>> #else >>> #define icount_enabled() 0 >>> #endif >>> >>> (This way, more TCG-only code in cpus.c gets elided). You can integrate >>> this change in the next version. >>> >>> Paolo >>> >> >> Weird, I tested with --disable-tcg explicitly (but may be some time ago now, >> as I constantly rebased). >> >> Will take a look at the introduction of this #defines in place of variables, >> as this mechanisms will not work in the future for target-specific modules. > > This is only done for per-target files so it should not be a problem. > > Paolo > K, I tried with latest master, disable-tcg build still works for me on x86, so it is something in queue I guess? Thanks, C
[PATCH v2 0/2] linux-user: fix print_syscall_err()
This function has been introduced to manage in a generic way the error
code of the syscall in the strace output.
But it has introduced a regression regarding two previous commits:
2a7e12455c1d ("linux-user/strace.c: Correct errno printing for mmap etc")
that intoduced the use of "-ret" rather than of "errno"
962b289ef350 ("linux-user: fix QEMU_STRACE=1 segfault")
that checks "-ret" is a valid error number
That series fixes that.
v2: add the patch to check "-ret" is valid
Laurent Vivier (2):
linux-user: fix the errno value in print_syscall_err()
linux-user: fix print_syscall_err() when syscall returned value is
negative
linux-user/strace.c | 36 +---
1 file changed, 13 insertions(+), 23 deletions(-)
--
2.26.2
[PATCH v2 1/2] linux-user: fix the errno value in print_syscall_err()
errno of the target is returned as a negative value by the syscall,
not in the host errno variable.
The emulation of the target syscall can return an error while the
host doesn't set an errno value. Target errnos and host errnos can
also differ in some cases.
Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument
printing after syscall execution")
Cc: filip.boz...@syrmia.com
Signed-off-by: Laurent Vivier
---
linux-user/strace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index 5235b2260cdd..b42664bbd180 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -731,7 +731,7 @@ print_syscall_err(abi_long ret)
qemu_log(" = ");
if (ret < 0) {
-qemu_log("-1 errno=%d", errno);
+qemu_log("-1 errno=%d", (int)-ret);
errstr = target_strerror(-ret);
if (errstr) {
qemu_log(" (%s)", errstr);
--
2.26.2
[PATCH v2 2/2] linux-user: fix print_syscall_err() when syscall returned value is negative
print_syscall_err() relies on the sign of the returned value to know
if it is an errno value or not.
But in some cases the returned value can have the most signicant bit
set without being an errno.
This patch restores previous behaviour that was also checking if
we can decode the errno to validate it.
This patch fixes this kind of problem (qemu-m68k):
root@sid:/# QEMU_STRACE= ls
3 brk(NULL) = -1 errno=21473607683 uname(0x407fff8a) = 0
to become:
root@sid:/# QEMU_STRACE= ls
3 brk(NULL) = 0x8001e000
3 uname(0xdf8a) = 0
Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument
printing after syscall execution")
Cc: filip.boz...@syrmia.com
Signed-off-by: Laurent Vivier
---
linux-user/strace.c | 36 +---
1 file changed, 13 insertions(+), 23 deletions(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index b42664bbd180..17f2554643f0 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -724,19 +724,20 @@ print_ipc(const struct syscallname *name,
* Variants for the return value output function
*/
-static void
+static bool
print_syscall_err(abi_long ret)
{
-const char *errstr = NULL;
+const char *errstr;
qemu_log(" = ");
if (ret < 0) {
-qemu_log("-1 errno=%d", (int)-ret);
errstr = target_strerror(-ret);
if (errstr) {
-qemu_log(" (%s)", errstr);
+qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
+return true;
}
}
+return false;
}
static void
@@ -744,11 +745,10 @@ print_syscall_ret_addr(const struct syscallname *name,
abi_long ret,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-print_syscall_err(ret);
-
-if (ret >= 0) {
-qemu_log("0x" TARGET_ABI_FMT_lx "\n", ret);
+if (!print_syscall_err(ret)) {
+qemu_log("0x" TARGET_ABI_FMT_lx, ret);
}
+qemu_log("\n");
}
#if 0 /* currently unused */
@@ -765,9 +765,7 @@ print_syscall_ret_newselect(const struct syscallname *name,
abi_long ret,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-print_syscall_err(ret);
-
-if (ret >= 0) {
+if (!print_syscall_err(ret)) {
qemu_log(" = 0x" TARGET_ABI_FMT_lx " (", ret);
print_fdset(arg0, arg1);
qemu_log(",");
@@ -796,9 +794,7 @@ print_syscall_ret_adjtimex(const struct syscallname *name,
abi_long ret,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-print_syscall_err(ret);
-
-if (ret >= 0) {
+if (!print_syscall_err(ret)) {
qemu_log(TARGET_ABI_FMT_ld, ret);
switch (ret) {
case TARGET_TIME_OK:
@@ -833,9 +829,7 @@ print_syscall_ret_listxattr(const struct syscallname *name,
abi_long ret,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-print_syscall_err(ret);
-
-if (ret >= 0) {
+if (!print_syscall_err(ret)) {
qemu_log(TARGET_ABI_FMT_ld, ret);
qemu_log(" (list = ");
if (arg1 != 0) {
@@ -866,9 +860,7 @@ print_syscall_ret_ioctl(const struct syscallname *name,
abi_long ret,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
-print_syscall_err(ret);
-
-if (ret >= 0) {
+if (!print_syscall_err(ret)) {
qemu_log(TARGET_ABI_FMT_ld, ret);
const IOCTLEntry *ie;
@@ -3189,9 +3181,7 @@ print_syscall_ret(int num, abi_long ret,
arg1, arg2, arg3,
arg4, arg5, arg6);
} else {
-print_syscall_err(ret);
-
-if (ret >= 0) {
+if (!print_syscall_err(ret)) {
qemu_log(TARGET_ABI_FMT_ld, ret);
}
qemu_log("\n");
--
2.26.2
Re: [PATCH 3/3] cpu-timers, icount: new modules
On 7/8/20 5:23 PM, Paolo Bonzini wrote: > On 08/07/20 17:17, Claudio Fontana wrote: >> On 7/8/20 5:05 PM, Paolo Bonzini wrote: >>> On 08/07/20 17:00, Claudio Fontana wrote: > Bisectable, 100% failure rate, etc. :( Can you split the patch in > multiple parts, specifically separating any rename or introducing of > includes from the final file move? Hi Paolo, will take a look! Is this captured by some travis / cirrus-ci / anything I can easily see the result of? >>> >>> Nope, unfortunately we don't have an s390 CI. But if you can get your >>> hands on one, just "./configure --target-list=s390x-softmmu && make && >>> make check-block" will show it. >>> > > #if defined CONFIG_TCG || !defined NEED_CPU_H > extern bool icount_enabled(void); > #else > #define icount_enabled() 0 > #endif > > (This way, more TCG-only code in cpus.c gets elided). You can integrate > this change in the next version. > > Paolo > Weird, I tested with --disable-tcg explicitly (but may be some time ago now, as I constantly rebased). Will take a look at the introduction of this #defines in place of variables, as this mechanisms will not work in the future for target-specific modules. >>> >>> This is only done for per-target files so it should not be a problem. >>> >>> Paolo >>> >> >> K, I tried with latest master, disable-tcg build still works for me on x86, >> so it is something in queue I guess? > > Peter reported the issue in the v1 pull request; there were two > different breakages but the cpus.c one was yours. > > Paolo > I see, since I don't manage to reproduce the problem at the moment, I will wait for master to be updated, after which I expect the issue to become apparent. As of now, I do configure --disable-tcg make -j120 which works for me, based on latest master. Ciao, C
Re: [PATCH] cpu: Add starts_halted() method
On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > > The original bug as described in the commit message sounds > > to me like something we should look to fix in the implementation > > of async_run_on_cpu() -- it shouldn't cause a CPU that's halfway > > through reset to do a KVM_RUN or otherwise run guest code, > > whether that CPU is going to start powered-up or powered-down. > > What "halfway through reset" means, exactly? Isn't halted==1 > enough to indicate the CPU is in that state? I mean "while we're in the middle of the CPU method that's called by cpu_reset()". "halted==1" says "the CPU is halted"; that's not the same thing. KVM_RUN happening as a side effect in the middle of that code is a bug whether the CPU happens to be intended to be put into the halted state or not. If the CPU is intended to be created not-halted then KVM_RUN can happen after cpu reset completes, but not before. thanks -- PMM
Re: [PULL 0/2] tcg patch queue
On Mon, 6 Jul 2020 at 19:52, Richard Henderson wrote: > > The following changes since commit eb6490f544388dd24c0d054a96dd304bc7284450: > > Merge remote-tracking branch > 'remotes/pmaydell/tags/pull-target-arm-20200703' into staging (2020-07-04 > 16:08:41 +0100) > > are available in the Git repository at: > > https://github.com/rth7680/qemu.git tags/pull-tcg-20200706 > > for you to fetch changes up to 852f933e482518797f7785a2e017a215b88df815: > > tcg: Fix do_nonatomic_op_* vs signed operations (2020-07-06 10:58:19 -0700) > > > Fix for ppc shifts > Fix for non-parallel atomic ops Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1 for any user-visible changes. -- PMM
Re: [PATCH 0/6] target/riscv: NaN-boxing for multiple precison
On 7/7/20 2:45 PM, LIU Zhiwei wrote:
>> On 2020/7/3 1:37, Richard Henderson wrote:
>>> I think it would be better to do all of the nan-boxing work inside of the
>>> helpers, including the return values.
>> Do you mean a helper function just for nan-boxing work?
No, that's not what I mean.
>> I don't think so.
>>
>> The inputs are flushed to canonical NAN only when they are
>> not legal nan-boxed values.
>>
>> The result is nan-boxed before writing to destination register.
>>
>> Both of them have some relations to nan-boxing, but they are not the same.
I mean
uint64_t helper_fadd_s(CPURISCVState *env, uint64_t frs1,
uint64_t frs2)
{
float32 in1 = check_nanbox(frs1);
float32 in2 = check_nanbox(frs2);
float32 res = float32_add(in1, in2, &env->fp_status);
return gen_nanbox(res);
}
I.e., always require nan-boxed inputs and return a nan-boxed output.
>>> If, for RVF && !RVD, we always maintain the invariant that the values are
>>> nanboxed anyway, then we do not even have to check for RVD at runtime.
>> Do you mean if FMV.X.S and FLW are nan-boxed, then we will not get the
>> invalid values？
No, I mean that if !RVD, there is no way to put an unboxed value into the fp
registers because...
>> First, FMV.X.D can transfer any 64 bits value to float register.
>> Second, users may set invalid values to float register by GDB.
... FMV.X.D does not exist for !RVD, nor does FLD.
The check_nanbox test will always succeed for !RVD, so we do not need to check
that RVD is set before performing check_nanbox.
Because the check is inexpensive, and because we expect !RVD to be an unusual
configuration, we do not bother to provide a second set of helpers that do not
perform the nan-boxing.
r~
Re: [PULL 07/15] hw/timer: RX62N 8-Bit timer (TMR)
2020-07-08 00:06 に Thomas Huth さんは書きました:
On 07/07/2020 17.02, Yoshinori Sato wrote:
On Mon, 29 Jun 2020 18:58:56 +0900,
Philippe Mathieu-Daudé wrote:
Hi Yoshinori,
On 6/25/20 11:25 AM, Peter Maydell wrote:
On Sun, 21 Jun 2020 at 13:54, Philippe Mathieu-Daudé
wrote:
From: Yoshinori Sato
renesas_tmr: 8bit timer modules.
Hi; the recent Coverity run reports a potential bug in this
code: (CID 1429976)
+static uint16_t read_tcnt(RTMRState *tmr, unsigned size, int ch)
+{
+int64_t delta, now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+int elapsed, ovf = 0;
+uint16_t tcnt[2];
Here we declare tcnt[] but do not initialize its contents...
+uint32_t ret;
+
+delta = (now - tmr->tick) * NANOSECONDS_PER_SECOND /
tmr->input_freq;
+if (delta > 0) {
+tmr->tick = now;
+
+if (FIELD_EX8(tmr->tccr[1], TCCR, CSS) == INTERNAL) {
+/* timer1 count update */
+elapsed = elapsed_time(tmr, 1, delta);
+if (elapsed >= 0x100) {
+ovf = elapsed >> 8;
+}
+tcnt[1] = tmr->tcnt[1] + (elapsed & 0xff);
+}
+switch (FIELD_EX8(tmr->tccr[0], TCCR, CSS)) {
+case INTERNAL:
+elapsed = elapsed_time(tmr, 0, delta);
+tcnt[0] = tmr->tcnt[0] + elapsed;
+break;
+case CASCADING:
+if (ovf > 0) {
+tcnt[0] = tmr->tcnt[0] + ovf;
+}
+break;
+}
...but not all cases here set both tcnt[0] and tcnt[1]
(for instance in the "case CASCADING:" if ovf <=0 we
won't set either of them)...
+} else {
+tcnt[0] = tmr->tcnt[0];
+tcnt[1] = tmr->tcnt[1];
+}
+if (size == 1) {
+return tcnt[ch];
+} else {
+ret = 0;
+ret = deposit32(ret, 0, 8, tcnt[1]);
+ret = deposit32(ret, 8, 8, tcnt[0]);
+return ret;
...and so here we will end up returning uninitialized
data. Presumably the spec says what value is actually
supposed to be returned in these cases?
PS: the "else" branch with the deposit32() calls could be
rewritten more simply as
return lduw_be_p(tcnt);
+static uint64_t tmr_read(void *opaque, hwaddr addr, unsigned size)
+{
In this function Coverity reports a missing "break" (CID 1429977):
+case A_TCORA:
+if (size == 1) {
+return tmr->tcora[ch];
+} else if (ch == 0) {
+return concat_reg(tmr->tcora);
+}
Here execution can fall through but there is no 'break' or '/*
fallthrough */'.
+case A_TCORB:
+if (size == 1) {
+return tmr->tcorb[ch];
+} else {
+return concat_reg(tmr->tcorb);
+}
Is it correct that the A_TCORA and A_TCORB code is different?
It looks odd, so if this is intentional then a comment describing
why it is so might be helpful to readers.
Can you address Peter's comments please?
This register can 8bit and 16bit access.
8bit case return separate single TCORA or TCORB.
16bit case return merged two channel's TCORA or TCORB.
high byte: channel 0 register.
low byte: channel 1 register
So could you please provide a patch that either adds the missing
"break;" statement between the cases here, or adds a "/* fallthrough
*/"
comment between the cases?
Thanks,
Thomas
OK.
This part will be cleaned up more.
Thanks.
build error of unused function as MACRO G_DEFINE_AUTOPTR_CLEANUP_FUNC expand
Hello all,
I build qemu with fuzzing enabled using clang and following error come.
nbd/server.c:1937:1: error: unused function
'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
^
/usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) {
g_list_free_full (*_l, (GDestroyNotify) func); } \
^
/usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro
'_GLIB_AUTOPTR_LIST_FUNC_NAME'
#define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)
glib_listautoptr_cleanup_##TypeName
^
:170:1: note: CC crypto/hash-glib.o
expanded from here
glib_listautoptr_cleanup_NBDExtentArray
^
nbd/server.c:1937:1: error: unused function
'glib_slistautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
/usr/include/glib-2.0/glib/gmacros.h:463:22: note: expanded from macro
'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
static inline void _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) (GSList **_l) {
g_slist_free_full (*_l, (GDestroyNotify) func); } \
^
/usr/include/glib-2.0/glib/gmacros.h:445:49: note: expanded from macro
'_GLIB_AUTOPTR_SLIST_FUNC_NAME'
#define _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName)
glib_slistautoptr_cleanup_##TypeName
^
:171:1: note: expanded from here
glib_slistautoptr_cleanup_NBDExtentArray
I see Eric’s patch 9bda600b083(“build: Silence clang warning on older glib
autoptr usage”)
So I know there should be a ‘-Wno-unused-function’ in CFLAGS. It is after
./configure:
CFLAGS-g -Wno-unused-function
QEMU_CFLAGS -I/usr/include/pixman-1 -Werror -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fPIE
-DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99
-Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides
-Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value
-Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition
-Wtype-limits -fstack-protector-strong -I$(SRC_PATH)/capstone/include
However while I ‘make V=1’ I see the build nbd/serer.c using following command:
clang-8 -iquote /home/test/qemu/nbd -iquote nbd -iquote
/home/test/qemu/tcg/i386 -isystem /home/test/qemu/linux-headers -isystem
/home/test/qemu/linux-headers -iquote . -iquote /home/test/qemu -iquote
/home/test/qemu/accel/tcg -iquote /home/test/qemu/include -iquote
/home/test/qemu/disas/libvixl -I/usr/include/pixman-1 -Werror -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fPIE
-DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -std=gnu99
-Wno-string-plus-int -Wno-typedef-redefinition -Wno-initializer-overrides
-Wexpansion-to-defined -Wendif-labels -Wno-shift-negative-value
-Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-definition
-Wtype-limits -fstack-protector-strong -I/home/test/qemu/capstone/include
-I/home/test/qemu/tests -I/home/test/qemu/tests/qtest -MMD -MP -MT nbd/server.o
-MF nbd/server.d -fsanitize=address,fuzzer-no-link -c -o nbd/server.o
nbd/server.c
There’s no CFLAGS ‘-Wno-unused-function’.
So I want to know:
1. Wha’t the relation of CFLAGS and QEMU_CFLAGS, it seems the CFLAGS doesn’t
work in this.
2. Any hits to solve this? My env error or needs a patch?
I use following command in Ubuntu 18.04.1.
CC=clang-8 CXX=clang++-8 ./configure --target-list="i386-softmmu"
--enable-debug --enable-debug --enable-kvm --enable-fuzzing
Thanks,
Li Qiang
Re: [PATCH v2 1/2] linux-user: fix the errno value in print_syscall_err()
On 7/8/20 8:24 AM, Laurent Vivier wrote:
> errno of the target is returned as a negative value by the syscall,
> not in the host errno variable.
>
> The emulation of the target syscall can return an error while the
> host doesn't set an errno value. Target errnos and host errnos can
> also differ in some cases.
>
> Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument
> printing after syscall execution")
> Cc: filip.boz...@syrmia.com
> Signed-off-by: Laurent Vivier
> ---
> linux-user/strace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson
r~
Re: [PATCH 1/5] block/io: introduce bdrv_try_mark_request_serialising
07.07.2020 18:56, Stefan Hajnoczi wrote: On Sat, Jun 20, 2020 at 05:36:45PM +0300, Vladimir Sementsov-Ogievskiy wrote: Introduce a function to mark the request serialising only if there are no conflicting request to wait for. The function is static, so mark it unused. The attribute is to be dropped in the next commit. Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/io.c | 58 +++--- 1 file changed, 51 insertions(+), 7 deletions(-) I found this patch difficult to understand because there are multiple levels of functions passing flags to ultimiately do different things in a common function. Here are some ideas if you have time to rework this patch: 1. Introduce a bdrv_find_overlapping_request() function that does most of bdrv_wait_serialising_requests_locked() but does not wait. Then bdrv_wait_serialising_requests_locked() can call that function in a loop and wait if an overlapping request is found. I thought about it, but splitting bdrv_find_overlapping_request is not so clear: it should include most of the logic inside "if (tracked_request_overlaps(..": an assertion, and checking !req->waiting_for. So the semantics of new functions becomes unclear, and it lead to splitting "->waiting_for" logic.. So, I decided to keep the whole function as is, not splitted. I just can't imagine reasonable split. 2. Pass overlap_offset/overlap_bytes arguments to bdrv_find_overlapping_request() instead of changing and restoring the value in bdrv_do_mark_request_serialising(). I'm not sure that it would be safe to not add a request to the list during the search. 3. Use consistent names for flags: wait/blocking, found/success I'm not sure if all these ideas will work, but I get the feeling this code can be refactored to make it easier to understand. Since I don't have a concrete suggestion and the code looks correct: Hmm. Unfortunately I didn't record the problems I faced on the way to resulting design, so I just don't remember now the details. So, I'll try to apply your suggestions, and remember the problems (or we'll get better patch :) Reviewed-by: Stefan Hajnoczi Thanks! -- Best regards, Vladimir
Re: [PATCH v2 2/2] linux-user: fix print_syscall_err() when syscall returned value is negative
On 7/8/20 8:24 AM, Laurent Vivier wrote:
> -static void
> +static bool
> print_syscall_err(abi_long ret)
> {
> -const char *errstr = NULL;
> +const char *errstr;
>
> qemu_log(" = ");
> if (ret < 0) {
This should be a target-specific test.
E.g. on most asm-generic I'm pretty sure this should be
if ((abi_ulong)ret > -(abi_ulong)512)
whereas for Alpha it should be
/*
* Syscall writes 0 to V0 to bypass error check, similar
* to how this is handled internal to Linux kernel.
*/
if (ret < 0 && env->ir[IR_V0] != 0)
r~
Re: [PATCH 2/5] block/io: introduce bdrv_co_range_try_lock
07.07.2020 19:10, Stefan Hajnoczi wrote:
On Sat, Jun 20, 2020 at 05:36:46PM +0300, Vladimir Sementsov-Ogievskiy wrote:
@@ -83,6 +84,12 @@ typedef struct BdrvTrackedRequest {
CoQueue wait_queue; /* coroutines blocked on this request */
struct BdrvTrackedRequest *waiting_for;
+
+/*
+ * If non-zero, the request is under lock, so it's allowed to intersect
+ * (actully it must be inside) the @lock request.
s/actully/actually/
@@ -745,15 +747,26 @@ static bool coroutine_fn
wait_or_find_conflicts(BdrvTrackedRequest *self,
if (tracked_request_overlaps(req, self->overlap_offset,
self->overlap_bytes))
{
-/* Hitting this means there was a reentrant request, for
- * example, a block driver issuing nested requests. This must
- * never happen since it means deadlock.
+if (self->lock == req) {
+/* This is atomic request under range_lock */
+assert(req->type == BDRV_TRACKED_LOCK);
+assert(self->offset >= req->offset);
+assert(self->bytes <= req->bytes);
These assertions do not catch requests that start within the locked
region but span beyond the end of the region. How about:
assert(self->offset + self->bytes - req->offset >= req->bytes);
+int coroutine_fn bdrv_co_pwrite_zeroes_locked(BdrvChild *child, int64_t offset,
+ int bytes, BdrvRequestFlags
flags,
+ BdrvTrackedRequest *lock)
The name is confusing because _locked() normally means that a mutex
should be held. Functions using that naming convention already exist in
block/io.c. It would be nice to distinguish between functions that need
BdrvTrackedRequest and functions that must be called with a mutex held.
How about bdrv_co_pwrite_zeroes_with_lock()?
OK.
May be _in_locked_range ? A bit longer, but more understandable.
--
Best regards,
Vladimir
Re: [PATCH 03/21] softfloat: add xtensa specialization for pickNaNMulAdd
On 7/6/20 4:47 PM, Max Filippov wrote: > pickNaNMulAdd logic on Xtensa is the same as pickNaN when applied to > the expression (a * b) + c. So with two pickNaN variants there must be > two pickNaNMulAdd variants. "Is the same as"? I question the non-use of the infzero parameter. When infzero, (a * b) = (Inf * 0), which will produce a default QNaN. Your sentence above would suggest that pickNaN is applied twice, so that if use_first_nan, the default nan is chosen above any nan within c. In addition, is the invalid flag raised for (Inf * 0) + NaN? Does that happen regardless of the use_first_nan setting, or does the whole operation short-circuit? r~
Re: Migrating custom qemu.org infrastructure to GitLab
On Wed, Jul 8, 2020 at 10:52 AM Stefan Hajnoczi wrote: Great, thanks for all the responses! Since others are interested in qemu-web.git, the wiki, and the bug tracker, I will investigate git repo hosting (mirrors). I'll send an update once I have more experience with GitLab and a proposal for how to perform the switch. If anyone objects we can discuss their concerns. Stefan
Re: [PATCH v2 2/2] linux-user: fix print_syscall_err() when syscall returned value is negative
Le 08/07/2020 à 17:52, Richard Henderson a écrit :
> On 7/8/20 8:24 AM, Laurent Vivier wrote:
>> -static void
>> +static bool
>> print_syscall_err(abi_long ret)
>> {
>> -const char *errstr = NULL;
>> +const char *errstr;
>>
>> qemu_log(" = ");
>> if (ret < 0) {
>
> This should be a target-specific test.
>
> E.g. on most asm-generic I'm pretty sure this should be
>
> if ((abi_ulong)ret > -(abi_ulong)512)
I think the test in target_strerror() gives the same result:
if ((err >= ERRNO_TABLE_SIZE) || (err < 0)) {
return NULL;
}
and it also ensures we don't overflow when we will access
target_to_host_errno_table[].
It's why we rely on errstr to know if the errno is valid or not
(we might also remove the "if (ret < 0)" in print_syscall_err).
> whereas for Alpha it should be
>
> /*
> * Syscall writes 0 to V0 to bypass error check, similar
> * to how this is handled internal to Linux kernel.
> */
> if (ret < 0 && env->ir[IR_V0] != 0)
We don't have access to "env" in strace.c.
it's an improvement regarding the code that has been modified.
If we want it I think it should be added in a separate patch.
Thanks,
Laurent
Re: [PATCH 05/21] target/xtensa: support copying registers up to 64 bits wide
On 7/6/20 4:47 PM, Max Filippov wrote:
> +if (arg_copy[i].arg->num_bits <= 32) {
> +temp = tcg_temp_local_new_i32();
> +tcg_gen_mov_i32(temp, arg_copy[i].arg->in);
> +} else if (arg_copy[i].arg->num_bits <= 64) {
> +temp = tcg_temp_local_new_i64();
> +tcg_gen_mov_i64(temp, arg_copy[i].arg->in);
This shouldn't compile.
You can't assign both TCGv_i32 and TCGv_i64 to the same variable.
What's going on here?
r~
Re: [PULL 00/53] Misc patches for QEMU 5.1 soft freeze
On 7/7/20 8:37 PM, Peter Maydell wrote:
> On Mon, 6 Jul 2020 at 17:48, Paolo Bonzini wrote:
>>
>> The following changes since commit fc1bff958998910ec8d25db86cd2f53ff125f7ab:
>>
>> hw/misc/pca9552: Add missing TypeInfo::class_size field (2020-06-29
>> 21:16:10 +0100)
>>
>> are available in the Git repository at:
>>
>> git://github.com/bonzini/qemu.git tags/for-upstream
>>
>> for you to fetch changes up to 80270507070ec73ea82741ce24cb7909a9258ea3:
>>
>> scripts: improve message when TAP based tests fail (2020-07-06 12:14:25
>> -0400)
>>
>>
>> * Make checkpatch say 'qemu' instead of 'kernel' (Aleksandar)
>> * Fix PSE guests with emulated NPT (Alexander B. #1)
>> * Fix leak (Alexander B. #2)
>> * HVF fixes (Roman, Cameron)
>> * New Sapphire Rapids CPUID bits (Cathy)
>> * cpus.c and softmmu/ cleanups (Claudio)
>> * TAP driver tweaks (Daniel, Havard)
>> * object-add bugfix and testcases (Eric A.)
>> * Fix Coverity MIN_CONST and MAX_CONST (Eric B.)
>> * SSE fixes (Joseph)
>> * "-msg guest-name" option (Mario)
>> * support for AMD nested live migration (myself)
>> * Small i386 TCG fixes (myself)
>> * improved error reporting for Xen (myself)
>> * fix "-cpu host -overcommit cpu-pm=on" (myself)
>> * Add accel/Kconfig (Philippe)
>> * KVM API cleanup (Philippe)
>> * iscsi sense handling fixes (Yongji)
>> * Misc bugfixes
>
> Hi; various build or test failures (5 total):
>
> 1) OSX:
>
> /Users/pm215/src/qemu-for-merges/ui/cocoa.m:1478:9: error: implicit
> declaration of function 'cpu_throttle_set' is invalid in C99 [-
> Werror,-Wimplicit-function-declaration]
> cpu_throttle_set(throttle_pct);
> ^
>
> 2) aarch64 and aarch32 linux:
>
> /home/pm/qemu/target/arm/kvm.c: In function ‘kvm_arch_init’:
> /home/pm/qemu/target/arm/kvm.c:248:29: error: passing argument 1 of
> ‘kvm_check_extension’ makes integer from pointer without a cast
> [-Werror=int-conversion]
> 248 | if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) {
> | ^
> | |
> | KVMState * {aka struct KVMState *}
> In file included from /home/pm/qemu/target/arm/kvm.c:23:
> /home/pm/qemu/include/sysemu/kvm.h:439:38: note: expected ‘unsigned
> int’ but argument is of type ‘KVMState *’ {aka ‘struct KVMState
> *’}
> 439 | int kvm_check_extension(unsigned int extension);
> | ~^
> /home/pm/qemu/target/arm/kvm.c:248:9: error: too many arguments to
> function ‘kvm_check_extension’
> 248 | if (kvm_check_extension(s, KVM_CAP_ARM_NISV_TO_USER)) {
> | ^~~
> In file included from /home/pm/qemu/target/arm/kvm.c:23:
> /home/pm/qemu/include/sysemu/kvm.h:439:5: note: declared here
> 439 | int kvm_check_extension(unsigned int extension);
> | ^~~
> /home/pm/qemu/target/arm/kvm.c:253:59: error: passing argument 1 of
> ‘kvm_check_extension’ makes integer from pointer without a cast
> [-Werror=int-conversion]
> 253 | cap_has_inject_ext_dabt = kvm_check_extension(s,
> | ^
> | |
> |
> KVMState * {aka struct KVMState *}
> In file included from /home/pm/qemu/target/arm/kvm.c:23:
> /home/pm/qemu/include/sysemu/kvm.h:439:38: note: expected ‘unsigned
> int’ but argument is of type ‘KVMState *’ {aka ‘struct KVMState
> *’}
> 439 | int kvm_check_extension(unsigned int extension);
> | ~^
> /home/pm/qemu/target/arm/kvm.c:253:39: error: too many arguments to
> function ‘kvm_check_extension’
> 253 | cap_has_inject_ext_dabt = kvm_check_extension(s,
> | ^~~
> In file included from /home/pm/qemu/target/arm/kvm.c:23:
> /home/pm/qemu/include/sysemu/kvm.h:439:5: note: declared here
> 439 | int kvm_check_extension(unsigned int extension);
> | ^~~
>
> 3) PPC64 had a failure on iotest 030 (though I think this may
> be an intermittent in master):
>
> TESTiotest-qcow2: 030 [fail]
> QEMU --
> "/home/pm215/qemu/build/all/tests/qemu-iotests/../../ppc64-softmmu/qemu-system-ppc64"
> -nodefaults -display none -accel qtest
> QEMU_IMG --
> "/home/pm215/qemu/build/all/tests/qemu-iotests/../../qemu-img"
> QEMU_IO --
> "/home/pm215/qemu/build/all/tests/qemu-iotests/../../qemu-io" --cache
> writeback --aio threads -f qcow2
> QEMU_NBD --
> "/home/pm215/qemu/build/all/tests/qemu-iotests/../../qemu-nbd"
> IMGFMT-- qcow2 (compat=1.1)
> IMGPROTO -- file
> PLATFORM -- Linux/ppc64 gcc1-power7 3.10.0-862.14.4.el7.ppc64
> TEST_DIR -- /home/pm215/qemu/build/all/tests/qemu-iotests/scratch
> SOCK_DIR -- /tmp/tmp.icAW30swbG
> SOCKET_SCM_HELPER --
> /home/pm215/
Re: [PATCH 06/21] target/xtensa: rename FPU2000 translators and helpers
On 7/6/20 4:47 PM, Max Filippov wrote: > Add _s suffix to all FPU2000 opcode translators and helpers that also > have double-precision variant to unify naming and allow adding DFPU > implementations. Add _fpu2k_ to the name of wur_fcr helper to make space > for the DFPU wur_fcr helper. > > Signed-off-by: Max Filippov > --- > target/xtensa/fpu_helper.c | 10 +- > target/xtensa/helper.h | 10 +- > target/xtensa/translate.c | 20 ++-- > 3 files changed, 20 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 3/5] block: introduce preallocate filter
08.07.2020 15:07, Stefan Hajnoczi wrote:
On Sat, Jun 20, 2020 at 05:36:47PM +0300, Vladimir Sementsov-Ogievskiy wrote:
It may be used for file-systems with slow allocation.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
qapi/block-core.json | 3 +-
block/preallocate.c | 255 +++
block/Makefile.objs | 1 +
3 files changed, 258 insertions(+), 1 deletion(-)
create mode 100644 block/preallocate.c
Please add documentation to docs/system/qemu-block-drivers.rst.inc
describing the purpose of this block driver and how to use it.
This implies adding new section "Filters", yes?
Since this filter grows the file I guess it's intended to be below an
image format?
Yes, between format and protocol nodes.
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0e1c6a59f2..a0bda399d6 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2805,7 +2805,7 @@
'cloop', 'compress', 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps',
'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi',
'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels',
-'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
+'preallocate', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'rbd',
{ 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },
'sheepdog',
'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs'
] }
@@ -3995,6 +3995,7 @@
'null-co':'BlockdevOptionsNull',
'nvme': 'BlockdevOptionsNVMe',
'parallels': 'BlockdevOptionsGenericFormat',
+ 'preallocate':'BlockdevOptionsGenericFormat',
'qcow2': 'BlockdevOptionsQcow2',
'qcow': 'BlockdevOptionsQcow',
'qed':'BlockdevOptionsGenericCOWFormat',
diff --git a/block/preallocate.c b/block/preallocate.c
new file mode 100644
index 00..c272a6e41d
--- /dev/null
+++ b/block/preallocate.c
@@ -0,0 +1,255 @@
+/*
+ * preallocate filter driver
+ *
+ * The driver performs preallocate operation: it is injected above
+ * some node, and before each write over EOF it does additional preallocating
+ * write-zeroes request.
+ *
+ * Copyright (c) 2020 Virtuozzo International GmbH.
+ *
+ * Author:
+ * Sementsov-Ogievskiy Vladimir
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see
Re: [PATCH 07/21] target/xtensa: move FSR/FCR register accessors
On 7/6/20 4:47 PM, Max Filippov wrote: > Move FSR/FCR register accessors from core opcodes to FPU2000 opcodes as > they are FPU2000-specific. > > Signed-off-by: Max Filippov > --- > target/xtensa/translate.c | 64 +++ > 1 file changed, 32 insertions(+), 32 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 08/21] target/xtensa: don't access BR regfile directly
On 7/6/20 4:47 PM, Max Filippov wrote: > BR registers used in FPU comparison opcodes are available as opcode > arguments for translators. Use them. This simplifies comparison helpers > interface and makes them usable in FLIX bundles. > > Signed-off-by: Max Filippov > --- > target/xtensa/fpu_helper.c | 42 +- > target/xtensa/helper.h | 14 ++--- > target/xtensa/translate.c | 20 ++ > 3 files changed, 42 insertions(+), 34 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 09/21] target/xtensa: add DFP option, registers and opcodes
On 7/6/20 4:47 PM, Max Filippov wrote:
> +float64 HELPER(add_d)(CPUXtensaState *env, float64 a, float64 b)
> +{
> +set_use_first_nan(true, &env->fp_status);
> +return float64_add(a, b, &env->fp_status);
> +}
> +
> float32 HELPER(add_s)(CPUXtensaState *env, float32 a, float32 b)
> {
> +set_use_first_nan(env->config->use_first_nan, &env->fp_status);
> return float32_add(a, b, &env->fp_status);
> }
I think you can do better than to set the use_first_nan flag before every
operation.
E.g. the translator could remember the previous setting within the TB, only
changing when necessary. E.g. if env->config->use_first_nan, then set it
during reset and never change it again. Similarly if DFP is not enabled.
r~
Memory callback
Sorry to bother you, I would like to ask a question.I want to use qemu to
monitor the information of the target thread reading and writing memory. I
see that qemu supports the tcg plugin, but I just find the following code
in plugin-gen.c .Do I just need to add a record function in it? How do I
read the current register values such as rip, rsp, how do I know the
current target address and data that the CPU read or written?Is there any
reference example?
/*
> * These helpers are stubs that get dynamically switched out for calls
> * direct to the plugin if they are subscribed to.
> */
> void HELPER(plugin_vcpu_udata_cb)(uint32_t cpu_index, void *udata)
> { }
> void HELPER(plugin_vcpu_mem_cb)(unsigned int vcpu_index,
> qemu_plugin_meminfo_t info, uint64_t vaddr,
> void *userdata)
> { }
Questions about online resizing a lun passthrough disk with virtio-scsi
Hi all, I have questions about online resizing a scsi lun passthrough disk. Here is my test scenario: host A: - the local 50GB sata /dev/sdd1 is configured as a physical volume, Created a 10GB logical volume. - exported the lv by iscsi target by targetcli. host B: - connected to the above iscsi target to obtain a 'local' scsi disk, say the /dev/sdb. - scsi lun passed through this /dev/sdb to a qemu-kvm linux guest. hostA:~ # lvresize -L +5G /dev/vg0/lv0 hostB:~ # rescan-scsi-bus.sh -s //this script is in sg3_utils, '-s' look for resized disks. hostB:~ # block_resize via qmp to notify qemu self and guest os. guest:~ # rescan-scsi-bus.sh -s guest:~ # use the extended disk space. My questions are: Is the 'block_resize' mandatory to notify guest os after online resizing a lun passed through disk? I'm curious it because I found there're couple of ways can make guest os realize the disk capacity change. e.g: * run 'block_resize' via qmp to let virtio-scsi notify the frontend about capacity change. * run 'rescan-scsi-bus.sh -s' inside guest. * run 'sg_readcap --16 /dev/sda' inside guest. I knew that the purpose of 'block_resize' is not only to notify guest os, but also to update some internal structure's member, say bs->total_sectors. What if I forgot to run 'block_resize', but run 'rescan-scsi-bus.sh -s' in guest? In this case, Although the qemu's bs->total_sectors value isn't updated due to missing 'block_resize', The guest os is still able to see the capacity change, I'm curious is there any potential risks while using the extended disk space in guest? What is the best practice for online resizing a lun passed through disk? Are all of below steps mandatory? OR may I omit either step 3 of 4? 1. online resize on storage side. 2. scsi rescan to realize the capacity change on hypervisor side. 3. block_resize via qmp. 4. scsi rescan to realize the capacity change in guest. 5. use the extended disk space in guest. Many thanks, Lin
Re: [PULL 00/53] Misc patches for QEMU 5.1 soft freeze
On 7/8/20 6:16 PM, Paolo Bonzini wrote: > On 08/07/20 18:13, Claudio Fontana wrote: >> so I don't get any tcg_get_icount_limit() compiled in, and no >> errors. >> >> I think that having comparable configure command line and compiler >> version/flags would help me pin down any related issue. > > Are you using link-time optimization by chance? > > Paolo > Yes unfortunately, nothing I have willingly enabled, but I found traces of LTO in C++. C++ is used to link the final qemu-system binary and on my system c++ has LTO: c++ -v Using built-in specs. COLLECT_GCC=c++ COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/7/lto-wrapper OFFLOAD_TARGET_NAMES=hsa:nvptx-none Target: x86_64-suse-linux Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,ada,go --enable-offload-targets=hsa,nvptx-none=/usr/nvptx-none, --without-cuda-driver --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/7 --enable-ssp --disable-libssp --disable-libvtv --disable-libcc1 --disable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-7 --without-system-libunwind --enable-multilib --with-arch-32=x86-64 --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux Thread model: posix gcc version 7.5.0 (SUSE Linux) I checked cc but did not think to check c++ . I will find a way to disable this thing and will correct the patch accordingly. Thanks a lot, and sorry for the inconvenience! Claudio
Re: [PATCH v2 1/2] linux-user: fix the errno value in print_syscall_err()
On 7/8/20 5:24 PM, Laurent Vivier wrote:
> errno of the target is returned as a negative value by the syscall,
> not in the host errno variable.
>
> The emulation of the target syscall can return an error while the
> host doesn't set an errno value. Target errnos and host errnos can
> also differ in some cases.
>
> Fixes: c84be71f6854 ("linux-user: Extend strace support to enable argument
> printing after syscall execution")
> Cc: filip.boz...@syrmia.com
> Signed-off-by: Laurent Vivier
> ---
> linux-user/strace.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 5235b2260cdd..b42664bbd180 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -731,7 +731,7 @@ print_syscall_err(abi_long ret)
>
> qemu_log(" = ");
> if (ret < 0) {
> -qemu_log("-1 errno=%d", errno);
> +qemu_log("-1 errno=%d", (int)-ret);
> errstr = target_strerror(-ret);
> if (errstr) {
> qemu_log(" (%s)", errstr);
>
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 01/12] npcm7xx: Add config symbol
On 7/7/20 8:47 PM, Havard Skinnemoen wrote: > Add a config symbol for the NPCM7xx BMC SoC family that subsequent > patches can use in Makefiles. > > Reviewed-by: Tyrone Ting > Acked-by: Joel Stanley > Signed-off-by: Havard Skinnemoen > --- > default-configs/arm-softmmu.mak | 1 + > hw/arm/Kconfig | 8 > 2 files changed, 9 insertions(+) > > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak > index 8fc09a4a51..9a94ebd0be 100644 > --- a/default-configs/arm-softmmu.mak > +++ b/default-configs/arm-softmmu.mak > @@ -27,6 +27,7 @@ CONFIG_GUMSTIX=y > CONFIG_SPITZ=y > CONFIG_TOSA=y > CONFIG_Z2=y > +CONFIG_NPCM7XX=y I'd squash this in patch 6: "Add two NPCM7xx-based machines" > CONFIG_COLLIE=y > CONFIG_ASPEED_SOC=y > CONFIG_NETDUINO2=y > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 4a224a6351..a31d0d282f 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -354,6 +354,14 @@ config XLNX_VERSAL > select VIRTIO_MMIO > select UNIMP > > +config NPCM7XX > +bool > +select A9MPCORE > +select ARM_GIC > +select PL310 # cache controller > +select SERIAL > +select UNIMP And this in patch 5: "Add NPCM730 and NPCM750 SoC models" > + > config FSL_IMX25 > bool > select IMX >
Re: [PATCH v4 01/12] npcm7xx: Add config symbol
On Wed, Jul 8, 2020 at 9:56 AM Philippe Mathieu-Daudé wrote: > > +config NPCM7XX > > +bool > > +select A9MPCORE > > +select ARM_GIC > > +select PL310 # cache controller > > +select SERIAL > > +select UNIMP > > And this in patch 5: "Add NPCM730 and NPCM750 SoC models" Is it still OK for earlier patches to use $(CONFIG_NPCM7XX) in Makefiles?
Re: [PATCH] MAINTAINERS: update nvme entry
On Tue, Jul 07, 2020 at 05:26:13PM +0200, Kevin Wolf wrote: > Am 06.07.2020 um 21:43 hat Keith Busch geschrieben: > > The nvme emulated device development pace has increased recently. Klaus > > has offered to co-maintain, and since we have many new contributions > > coming through, we're adding a repository to accumulate and test new > > features. > > > > Cc: Klaus Jensen > > Signed-off-by: Keith Busch > > I assume that you'll merge this as the first thing through your new > tree, so instead of applying, I'll just add: Thanks, I've started the new branch with this as the first commit. We'll wait at least a few more days to see if we've agreement on new patches before sending our pull request.
Re: [PULL 00/53] Misc patches for QEMU 5.1 soft freeze
On 7/8/20 6:55 PM, Paolo Bonzini wrote: > On 08/07/20 18:45, Claudio Fontana wrote: >> C++ is used to link the final qemu-system binary and on my system c++ has >> LTO: >> >> c++ -v >> Using built-in specs. >> COLLECT_GCC=c++ >> COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/7/lto-wrapper >> OFFLOAD_TARGET_NAMES=hsa:nvptx-none >> Target: x86_64-suse-linux >> Configured with: ../configure --prefix=/usr --infodir=/usr/share/info >> --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 >> --enable-languages=c,c++,objc,fortran,obj-c++,ada,go >> --enable-offload-targets=hsa,nvptx-none=/usr/nvptx-none, >> --without-cuda-driver --enable-checking=release --disable-werror >> --with-gxx-include-dir=/usr/include/c++/7 --enable-ssp --disable-libssp >> --disable-libvtv --disable-libcc1 --disable-plugin >> --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' >> --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new >> --disable-libstdcxx-pch --enable-version-specific-runtime-libs >> --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex >> --enable-gnu-indirect-function --program-suffix=-7 >> --without-system-libunwind --enable-multilib --with-arch-32=x86-64 >> --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux >> Thread model: posix >> gcc version 7.5.0 (SUSE Linux) >> >> >> I checked cc but did not think to check c++ . I will find a way to disable >> this thing and will correct the patch accordingly. > > Having LTO support is not the same thing as having it enabled. Are you > compiling and linking with "-flto"? > > Paolo > no, the compilation and link stage do not show this explicit parameter. I am puzzled. C
Re: [PATCH v4 08/12] hw/nvram: NPCM7xx OTP device model
On Wed, Jul 8, 2020 at 1:54 AM Philippe Mathieu-Daudé wrote:
>
> On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
> > +value = tswap32(nc->disabled_modules);
> > +npcm7xx_otp_array_write(&s->fuse_array, &value, 64, sizeof(value));
>
> What is magic offset 64 for?
Good point. I'll add some definitions based on
https://github.com/Nuvoton-Israel/bootblock/blob/master/Src/fuse_wrapper/fuse_wrapper.h#L23
> > +/* Preserve read-only and write-one-to-clear bits */
> > +value =
> > +(value & ~FST_RO_MASK) | (s->regs[NPCM7XX_OTP_FST] &
> > FST_RO_MASK);
>
> Trivial to review as:
>
>value &= ~FST_RO_MASK;
>value |= s->regs[NPCM7XX_OTP_FST] & FST_RO_MASK;
You're right, will do.
> > +/* Each OTP module holds 8192 bits of one-time programmable storage */
> > +#define NPCM7XX_OTP_ARRAY_BITS (8192)
> > +#define NPCM7XX_OTP_ARRAY_BYTES (NPCM7XX_OTP_ARRAY_BITS / 8)
>
> You could replace 8 by BITS_PER_BYTE.
Will do.
> > +typedef struct NPCM7xxOTPClass {
> > +SysBusDeviceClass parent;
> > +
> > +const MemoryRegionOps *mmio_ops;
> > +} NPCM7xxOTPClass;
> > +
> > +#define NPCM7XX_OTP_CLASS(klass) \
> > +OBJECT_CLASS_CHECK(NPCM7xxOTPClass, (klass), TYPE_NPCM7XX_OTP)
> > +#define NPCM7XX_OTP_GET_CLASS(obj) \
> > +OBJECT_GET_CLASS(NPCM7xxOTPClass, (obj), TYPE_NPCM7XX_OTP)
>
> If nothing outside of the model implementation requires accessing
> the class fields (which is certainly the case here, no code out of
> npcm7xx_otp.c should access mmio_ops directly), then I recommend
> to keep the class definitions local to the single source file where
> it is used. This also makes this header simpler to look at.
Good idea. Will do.
> Very high code quality, so I just made nitpicking comments.
> Reviewed-by: Philippe Mathieu-Daudé
Thank you. I'll incorporate your feedback and send out a v5 series shortly.
Havard
Re: [PATCH v4 01/12] npcm7xx: Add config symbol
On 7/8/20 6:58 PM, Havard Skinnemoen wrote: > On Wed, Jul 8, 2020 at 9:56 AM Philippe Mathieu-Daudé wrote: >>> +config NPCM7XX >>> +bool >>> +select A9MPCORE >>> +select ARM_GIC >>> +select PL310 # cache controller >>> +select SERIAL >>> +select UNIMP >> >> And this in patch 5: "Add NPCM730 and NPCM750 SoC models" > > Is it still OK for earlier patches to use $(CONFIG_NPCM7XX) in Makefiles? > I haven't reviewed them yet, so no. I'd do this way: - Add to the first peripheral that requires $(CONFIG_NPCM7XX): config NPCM7XX bool - Then when you add the SoC, complete with: select A9MPCORE select ARM_GIC select PL310 # cache controller select SERIAL select UNIMP
Re: [PATCH] cpu: Add starts_halted() method
On Wed, 8 Jul 2020 at 17:03, Eduardo Habkost wrote: > > On Wed, Jul 08, 2020 at 04:32:51PM +0100, Peter Maydell wrote: > > On Wed, 8 Jul 2020 at 16:25, Eduardo Habkost wrote: > > > On Wed, Jul 08, 2020 at 02:14:03PM +0100, Peter Maydell wrote: > > > > The original bug as described in the commit message sounds > > > > to me like something we should look to fix in the implementation > > > > of async_run_on_cpu() -- it shouldn't cause a CPU that's halfway > > > > through reset to do a KVM_RUN or otherwise run guest code, > > > > whether that CPU is going to start powered-up or powered-down. > > > > > > What "halfway through reset" means, exactly? Isn't halted==1 > > > enough to indicate the CPU is in that state? > > > > I mean "while we're in the middle of the CPU method that's > > called by cpu_reset()". "halted==1" says "the CPU is halted"; > > that's not the same thing. KVM_RUN happening > > as a side effect in the middle of that code is a bug > > whether the CPU happens to be intended to be put into the > > halted state or not. If the CPU is intended to be created > > not-halted then KVM_RUN can happen after cpu reset > > completes, but not before. > > Wait, I thought we already had mechanisms to prevent that from > happening. Otherwise, it would never be safe for cpu_reset() to > touch the CPU registers. Exactly. It appears that there's a bug in our mechanisms, which is why I'm suggesting that the right thing is to fix that bug rather than marking the CPU as halted earlier in the reset process so that the KVM_RUN happens to do nothing... -- PMM
Re: [PATCH 05/21] target/xtensa: support copying registers up to 64 bits wide
On Wed, Jul 8, 2020 at 9:14 AM Richard Henderson
wrote:
>
> On 7/6/20 4:47 PM, Max Filippov wrote:
> > +if (arg_copy[i].arg->num_bits <= 32) {
> > +temp = tcg_temp_local_new_i32();
> > +tcg_gen_mov_i32(temp, arg_copy[i].arg->in);
> > +} else if (arg_copy[i].arg->num_bits <= 64) {
> > +temp = tcg_temp_local_new_i64();
> > +tcg_gen_mov_i64(temp, arg_copy[i].arg->in);
>
> This shouldn't compile.
>
> You can't assign both TCGv_i32 and TCGv_i64 to the same variable.
>
> What's going on here?
temp is a void pointer, as well as OpcodeArg::in and OpcodeArg::out.
--
Thanks.
-- Max
[Bug 1886811] Re: systemd complains Failed to enqueue loopback interface start request: Operation not supported
** Bug watch added: Debian Bug tracker #964289 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 ** Also affects: qemu (Debian) via https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 Importance: Unknown Status: Unknown -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1886811 Title: systemd complains Failed to enqueue loopback interface start request: Operation not supported Status in QEMU: New Status in qemu package in Debian: Unknown Bug description: This symptom seems similar to https://bugs.launchpad.net/qemu/+bug/1823790 Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture qemu version: latest git of git commit hash eb2c66b10efd2b914b56b20ae90655914310c925 compiled with "./configure --static --disable-system" Down stream bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 Bug report (closed) to systemd: https://github.com/systemd/systemd/issues/16359 systemd in armhf and armel (both little endian 32-bit) containers fail to start with Failed to enqueue loopback interface start request: Operation not supported How to reproduce on Debian (and probably Ubuntu): mmdebstrap --components="main contrib non-free" --architectures=armhf --variant=important bullseye /var/lib/machines/armhf-bullseye systemd-nspawn -D /var/lib/machines/armhf-bullseye -b When "armhf" architecture is replaced with "mips" (32-bit big endian) or "ppc64" (64-bit big endian), the container starts up fine. The same symptom is also observed with "powerpc" (32-bit big endian) architecture. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1886811/+subscriptions
[Bug 1886811] Re: systemd complains Failed to enqueue loopback interface start request: Operation not supported
It would help to know which operation is not supported. Could you get the coredump? Is it possible to run the operation with "QEMU_STRACE" set in the environment? Normally loop ioctls are supported. But it seems the following ones are not implemented in QEMU: LOOP_SET_CAPACITY, LOOP_SET_DIRECT_IO, LOOP_SET_BLOCK_SIZE. ** Tags added: linux-user -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1886811 Title: systemd complains Failed to enqueue loopback interface start request: Operation not supported Status in QEMU: New Status in qemu package in Debian: Unknown Bug description: This symptom seems similar to https://bugs.launchpad.net/qemu/+bug/1823790 Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture qemu version: latest git of git commit hash eb2c66b10efd2b914b56b20ae90655914310c925 compiled with "./configure --static --disable-system" Down stream bug report at https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289 Bug report (closed) to systemd: https://github.com/systemd/systemd/issues/16359 systemd in armhf and armel (both little endian 32-bit) containers fail to start with Failed to enqueue loopback interface start request: Operation not supported How to reproduce on Debian (and probably Ubuntu): mmdebstrap --components="main contrib non-free" --architectures=armhf --variant=important bullseye /var/lib/machines/armhf-bullseye systemd-nspawn -D /var/lib/machines/armhf-bullseye -b When "armhf" architecture is replaced with "mips" (32-bit big endian) or "ppc64" (64-bit big endian), the container starts up fine. The same symptom is also observed with "powerpc" (32-bit big endian) architecture. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1886811/+subscriptions
Re: [PATCH v7 00/47] block: Deal with filters
On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" introduced with the commit a7399eb iotests: Make _filter_img_create more active Andrey
Re: [PATCH v7 02/47] block: Add chain helper functions
On 25.06.2020 18:21, Max Reitz wrote:
Add some helper functions for skipping filters in a chain of block
nodes.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 3 +++
block.c | 55 +++
2 files changed, 58 insertions(+)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index bb3457c5e8..5da793bfc3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1382,6 +1382,9 @@ BdrvChild *bdrv_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_filter_child(BlockDriverState *bs);
BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs);
BdrvChild *bdrv_primary_child(BlockDriverState *bs);
+BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs);
+BlockDriverState *bdrv_skip_filters(BlockDriverState *bs);
+BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs);
static inline BlockDriverState *child_bs(BdrvChild *child)
{
diff --git a/block.c b/block.c
index 5a42ef49fd..0a0b855261 100644
--- a/block.c
+++ b/block.c
@@ -7008,3 +7008,58 @@ BdrvChild *bdrv_primary_child(BlockDriverState *bs)
return NULL;
}
+
+static BlockDriverState *bdrv_do_skip_filters(BlockDriverState *bs,
+ bool stop_on_explicit_filter)
+{
+BdrvChild *c;
+
+if (!bs) {
+return NULL;
+}
+
+while (!(stop_on_explicit_filter && !bs->implicit)) {
+c = bdrv_filter_child(bs);
+if (!c) {
+break;
+}
+bs = c->bs;
Could it be child_bs(bs) ?
Andrey
+}
Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 01/47] block: Add child access functions
On 25.06.2020 18:21, Max Reitz wrote:
There are BDS children that the general block layer code can access,
namely bs->file and bs->backing. Since the introduction of filters and
external data files, their meaning is not quite clear. bs->backing can
be a COW source, or it can be a filtered child; bs->file can be a
filtered child, it can be data and metadata storage, or it can be just
metadata storage.
This overloading really is not helpful. This patch adds functions that
retrieve the correct child for each exact purpose. Later patches in
this series will make use of them. Doing so will allow us to handle
filter nodes in a meaningful way.
Signed-off-by: Max Reitz
---
include/block/block_int.h | 44 +--
block.c | 90 +++
2 files changed, 131 insertions(+), 3 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1b86b59af1..bb3457c5e8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -90,9 +90,17 @@ struct BlockDriver {
int instance_size;
...
Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 03/47] block: bdrv_cow_child() for bdrv_has_zero_init()
On 25.06.2020 18:21, Max Reitz wrote:
bdrv_has_zero_init() and the related bdrv_unallocated_blocks_are_zero()
should use bdrv_cow_child() if they want to check whether the given BDS
has a COW backing file.
Signed-off-by: Max Reitz
---
block.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 0a0b855261..f3e2aae49c 100644
--- a/block.c
+++ b/block.c
@@ -5394,7 +5394,7 @@ int bdrv_has_zero_init(BlockDriverState *bs)
/* If BS is a copy on write image, it is initialized to
the contents of the base image, which may not be zeroes. */
-if (bs->backing) {
+if (bdrv_cow_child(bs)) {
return 0;
}
if (bs->drv->bdrv_has_zero_init) {
@@ -5412,7 +5412,7 @@ bool bdrv_unallocated_blocks_are_zero(BlockDriverState
*bs)
{
BlockDriverInfo bdi;
-if (bs->backing) {
+if (bdrv_cow_child(bs)) {
return false;
}
Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 05/47] block: Include filters when freezing backing chain
On 25.06.2020 18:21, Max Reitz wrote: In order to make filters work in backing chains, the associated functions must be able to deal with them and freeze both COW and filter child links. While at it, add some comments that note which functions require their caller to ensure that a given child link is not frozen, and how the callers do so. Signed-off-by: Max Reitz --- block.c | 60 + 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/block.c b/block.c index d139ffb57d..b59bd776cd 100644 --- a/block.c +++ b/block.c Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 04/47] block: bdrv_set_backing_hd() is about bs->backing
On 25.06.2020 18:21, Max Reitz wrote:
bdrv_set_backing_hd() is a function that explicitly cares about the
bs->backing child. Highlight that in its description and use
child_bs(bs->backing) instead of backing_bs(bs) to make it more obvious.
Signed-off-by: Max Reitz
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index f3e2aae49c..d139ffb57d 100644
--- a/block.c
+++ b/block.c
@@ -2846,7 +2846,7 @@ static BdrvChildRole bdrv_backing_role(BlockDriverState
*bs)
}
/*
- * Sets the backing file link of a BDS. A new reference is created; callers
+ * Sets the bs->backing link of a BDS. A new reference is created; callers
* which don't need their own reference any more must call bdrv_unref().
*/
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd,
@@ -2855,7 +2855,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs,
BlockDriverState *backing_hd,
bool update_inherits_from = bdrv_chain_contains(bs, backing_hd) &&
bdrv_inherits_from_recursive(backing_hd, bs);
-if (bdrv_is_backing_chain_frozen(bs, backing_bs(bs), errp)) {
+if (bdrv_is_backing_chain_frozen(bs, child_bs(bs->backing), errp)) {
return;
}
Reviewed-by: Andrey Shinkevich
[Bug 1886793] Re: "go install" command fails while running inside s390x docker container on x86_64 host using qemu
Could you try to reproduce the problem with the latest version of QEMU
from git repo?
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1886793
Title:
"go install" command fails while running inside s390x docker container
on x86_64 host using qemu
Status in QEMU:
New
Bug description:
Steps to reproduce the issue:
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
Build the following Docker Image using following Dockerfile.s390x
using command docker build -t test/crossbuild:latest-s390x -f
Dockerfile.s390x .
Dockerfile.s390x
##
FROM alpine:3.11 as qemu
ARG QEMU_VERSION=5.0.0-2
ARG QEMU_ARCHS="s390x"
RUN apk --update add curl
#Enable non-native runs on amd64 architecture hosts
RUN for i in ${QEMU_ARCHS}; do curl -L
https://github.com/multiarch/qemu-user-static/releases/download/v${QEMU_VERSION}/qemu-${i}-static.tar.gz
| tar zxvf - -C /usr/bin; done
RUN chmod +x /usr/bin/qemu-*
FROM s390x/golang:1.14.2-alpine3.11
MAINTAINER LoZ Open Source Ecosystem
(https://www.ibm.com/developerworks/community/groups/community/lozopensource)
ARG MANIFEST_TOOL_VERSION=v1.0.2
#Enable non-native builds of this image on an amd64 hosts.
#This must be the first RUN command in this file!
COPY --from=qemu /usr/bin/qemu-*-static /usr/bin/
#Install su-exec for use in the entrypoint.sh (so processes run as the right
user)
#Install bash for the entry script (and because it's generally useful)
#Install curl to download glide
#Install git for fetching Go dependencies
#Install ssh for fetching Go dependencies
#Install mercurial for fetching go dependencies
#Install wget since it's useful for fetching
#Install make for building things
#Install util-linux for column command (used for output formatting).
#Install grep and sed for use in some Makefiles (e.g. pulling versions out of
glide.yaml)
#Install shadow for useradd (it allows to use big UID)
RUN apk update && apk add --no-cache su-exec curl bash git openssh mercurial
make wget util-linux tini file grep sed shadow
RUN apk upgrade --no-cache
#Disable ssh host key checking
RUN echo 'Host *' >> /etc/ssh/ssh_config \
&& echo 'StrictHostKeyChecking no' >> /etc/ssh/ssh_config
#Disable cgo so that binaries we build will be fully static.
ENV CGO_ENABLED=0
#Recompile the standard library with cgo disabled. This prevents the
standard library from being
#marked stale, causing full rebuilds every time.
RUN go install -v std
#Install glide
RUN go get github.com/Masterminds/glide
ENV GLIDE_HOME /home/user/.glide
#Install dep
RUN go get github.com/golang/dep/cmd/dep
#Install ginkgo CLI tool for running tests
RUN go get github.com/onsi/ginkgo/ginkgo
#Install linting tools.
RUN wget -O - -q
https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s
v1.20.0
RUN golangci-lint --version
#Install license checking tool.
RUN go get github.com/pmezard/licenses
#Install tool to merge coverage reports.
RUN go get github.com/wadey/gocovmerge
#Install CLI tool for working with yaml files
RUN go get github.com/mikefarah/yaml
#Delete all the Go sources that were downloaded, we only rely on the binaries
RUN rm -rf /go/src/*
#Install vgo (should be removed once we take Go 1.11)
RUN go get -u golang.org/x/vgo
#Ensure that everything under the GOPATH is writable by everyone
RUN chmod -R 777 $GOPATH
RUN curl -sSL
https://github.com/estesp/manifest-tool/releases/download/${MANIFEST_TOOL_VERSION}/manifest-tool-linux-s390x
> manifest-tool && \
chmod +x manifest-tool && \
mv manifest-tool /usr/bin/
COPY entrypoint.sh /usr/local/bin/entrypoint.sh
ENTRYPOINT ["/sbin/tini", "--", "/usr/local/bin/entrypoint.sh"]
##
The build just hangs at RUN go install -v std
Also, while running the same command inside s390x container on x86_64
host, error Illegal instruction (core dumped) is thrown.
Register x86_64 host with the latest qemu-user-static.
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
docker run -it -v /home/test/qemu-s390x-static:/usr/bin/qemu-s390x-
static s390x/golang:1.14.2-alpine3.11
Inside s390x container:
apk update && apk add --no-cache su-exec curl bash git openssh mercurial make
wget util-linux tini file grep sed shadow
apk upgrade --no-cache
#Disable cgo so that binaries we build will be fully static.
export CGO_ENABLED=0
go install -v std
This gives the following error:
Illegal instruction (core dumped)
Environment:
x86_64 Ub18.04 4.15.0-101-generic Ubuntu SMP x86_64 GNU/Linux
QEMU user static version: 5.0.0-2
Container application: Docker
Client: Do
Re: [PATCH v4 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models
Hi Havard,
On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> Management Controllers in servers. While the family includes four SoCs,
> this patch implements limited support for two of them: NPCM730 (targeted
> for Data Center applications) and NPCM750 (targeted for Enterprise
> applications).
>
> This patch includes little more than the bare minimum needed to boot a
> Linux kernel built with NPCM7xx support in direct-kernel mode:
>
> - Two Cortex-A9 CPU cores with built-in periperhals.
> - Global Configuration Registers.
> - Clock Management.
> - 3 Timer Modules with 5 timers each.
> - 4 serial ports.
>
> The chips themselves have a lot more features, some of which will be
> added to the model at a later stage.
>
> Reviewed-by: Tyrone Ting
> Reviewed-by: Joel Stanley
> Signed-off-by: Havard Skinnemoen
> ---
> hw/arm/Makefile.objs | 1 +
> hw/arm/npcm7xx.c | 328 +++
> include/hw/arm/npcm7xx.h | 81 ++
Please have a look at the scripts/git.orderfile, using it would
ease our reviews.
> 3 files changed, 410 insertions(+)
> create mode 100644 hw/arm/npcm7xx.c
> create mode 100644 include/hw/arm/npcm7xx.h
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 534a6a119e..13d163a599 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -41,6 +41,7 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
> obj-$(CONFIG_STM32F405_SOC) += stm32f405_soc.o
> obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
> obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
> +obj-$(CONFIG_NPCM7XX) += npcm7xx.o
> obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
> obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
> obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> new file mode 100644
> index 00..0a9e30f66f
> --- /dev/null
> +++ b/hw/arm/npcm7xx.c
> @@ -0,0 +1,328 @@
> +/*
> + * Nuvoton NPCM7xx SoC family.
> + *
> + * Copyright 2020 Google LLC
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> + * for more details.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "exec/address-spaces.h"
> +#include "hw/arm/npcm7xx.h"
> +#include "hw/char/serial.h"
> +#include "hw/loader.h"
> +#include "hw/misc/unimp.h"
> +#include "hw/qdev-properties.h"
> +#include "qapi/error.h"
> +#include "qemu/units.h"
> +#include "sysemu/sysemu.h"
> +
> +/* The first half of the address space is reserved for DDR4 DRAM. */
> +#define NPCM7XX_DRAM_BA (0x)
> +#define NPCM7XX_DRAM_SZ (2 * GiB)
> +
> +/*
> + * This covers the whole MMIO space. We'll use this to catch any MMIO
> accesses
> + * that aren't handled by any device.
> + */
> +#define NPCM7XX_MMIO_BA (0x8000)
> +#define NPCM7XX_MMIO_SZ (0x7FFD)
> +
> +/* Core system modules. */
> +#define NPCM7XX_L2C_BA (0xF03FC000)
> +#define NPCM7XX_CPUP_BA (0xF03FE000)
> +#define NPCM7XX_GCR_BA (0xF080)
> +#define NPCM7XX_CLK_BA (0xF0801000)
> +
> +/* Memory blocks at the end of the address space */
> +#define NPCM7XX_RAM2_BA (0xFFFD)
> +#define NPCM7XX_RAM2_SZ (128 * KiB)
> +#define NPCM7XX_ROM_BA (0x)
> +#define NPCM7XX_ROM_SZ (64 * KiB)
> +
> +/*
> + * Interrupt lines going into the GIC. This does not include internal
> Cortex-A9
> + * interrupts.
> + */
> +enum NPCM7xxInterrupt {
> +NPCM7XX_UART0_IRQ = 2,
> +NPCM7XX_UART1_IRQ,
> +NPCM7XX_UART2_IRQ,
> +NPCM7XX_UART3_IRQ,
> +NPCM7XX_TIMER0_IRQ = 32, /* Timer Module 0 */
> +NPCM7XX_TIMER1_IRQ,
> +NPCM7XX_TIMER2_IRQ,
> +NPCM7XX_TIMER3_IRQ,
> +NPCM7XX_TIMER4_IRQ,
> +NPCM7XX_TIMER5_IRQ, /* Timer Module 1 */
> +NPCM7XX_TIMER6_IRQ,
> +NPCM7XX_TIMER7_IRQ,
> +NPCM7XX_TIMER8_IRQ,
> +NPCM7XX_TIMER9_IRQ,
> +NPCM7XX_TIMER10_IRQ,/* Timer Module 2 */
> +NPCM7XX_TIMER11_IRQ,
> +NPCM7XX_TIMER12_IRQ,
> +NPCM7XX_TIMER13_IRQ,
> +NPCM7XX_TIMER14_IRQ,
> +};
> +
> +/* Total number of GIC interrupts, including internal Cortex-A9 interrupts.
> */
> +#define NPCM7XX_NUM_IRQ (160)
> +
> +/* Register base address for each Timer Module */
> +static const hwaddr npcm7xx_tim_addr[] = {
> +0xF0008000,
> +0xF0009000,
> +0xF000A000,
Here caps hex ...
> +};
> +
> +/* Register base address for each 16550 UART */
> +static cons
Re: [PATCH 09/21] target/xtensa: add DFP option, registers and opcodes
On Wed, Jul 8, 2020 at 9:25 AM Richard Henderson
wrote:
>
> On 7/6/20 4:47 PM, Max Filippov wrote:
> > +float64 HELPER(add_d)(CPUXtensaState *env, float64 a, float64 b)
> > +{
> > +set_use_first_nan(true, &env->fp_status);
> > +return float64_add(a, b, &env->fp_status);
> > +}
> > +
> > float32 HELPER(add_s)(CPUXtensaState *env, float32 a, float32 b)
> > {
> > +set_use_first_nan(env->config->use_first_nan, &env->fp_status);
> > return float32_add(a, b, &env->fp_status);
> > }
>
> I think you can do better than to set the use_first_nan flag before every
> operation.
And it was better, until I found that the rules for float64 are a
bit... peculiar.
> E.g. the translator could remember the previous setting within the TB, only
> changing when necessary. E.g. if env->config->use_first_nan, then set it
> during reset and never change it again. Similarly if DFP is not enabled.
This thought crossed my mind too, but then set_use_first_nan only
sets one variable in the float_status and gets inlined.
Is it worth the trouble?
--
Thanks.
-- Max
Re: [PATCH v7 06/47] block: Drop bdrv_is_encrypted()
On 25.06.2020 18:21, Max Reitz wrote:
The original purpose of bdrv_is_encrypted() was to inquire whether a BDS
can be used without the user entering a password or not. It has not
been used for that purpose for quite some time.
Actually, it is not even fit for that purpose, because to answer that
question, it would have recursively query all of the given node's
children.
So now we have to decide in which direction we want to fix
bdrv_is_encrypted(): Recursively query all children, or drop it and just
use bs->encrypted to get the current node's status?
Nowadays, its only purpose is to report through bdrv_query_image_info()
whether the given image is encrypted or not. For this purpose, it is
probably more interesting to see whether a given node itself is
encrypted or not (otherwise, a management application cannot discern for
certain which nodes are really encrypted and which just have encrypted
children).
Suggested-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz
---
include/block/block.h | 1 -
block.c | 8
block/qapi.c | 2 +-
3 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 86f9728f00..0080fe1311 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -538,7 +538,6 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it);
void bdrv_next_cleanup(BdrvNextIterator *it);
BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
-bool bdrv_is_encrypted(BlockDriverState *bs);
void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
void *opaque, bool read_only);
const char *bdrv_get_node_name(const BlockDriverState *bs);
diff --git a/block.c b/block.c
index b59bd776cd..76277ea4e0 100644
--- a/block.c
+++ b/block.c
@@ -5044,14 +5044,6 @@ bool bdrv_is_sg(BlockDriverState *bs)
return bs->sg;
}
-bool bdrv_is_encrypted(BlockDriverState *bs)
-{
-if (bs->backing && bs->backing->bs->encrypted) {
-return true;
-}
-return bs->encrypted;
-}
-
const char *bdrv_get_format_name(BlockDriverState *bs)
{
return bs->drv ? bs->drv->format_name : NULL;
diff --git a/block/qapi.c b/block/qapi.c
index afd9f3b4a7..4807a2b344 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -288,7 +288,7 @@ void bdrv_query_image_info(BlockDriverState *bs,
info->virtual_size= size;
info->actual_size = bdrv_get_allocated_file_size(bs);
info->has_actual_size = info->actual_size >= 0;
-if (bdrv_is_encrypted(bs)) {
+if (bs->encrypted) {
info->encrypted = true;
info->has_encrypted = true;
}
Reviewed-by: Andrey Shinkevich
Re: [PATCH 01/21] softfloat: make NO_SIGNALING_NANS runtime property
On 7/7/20 1:47 AM, Max Filippov wrote: > target/xtensa, the only user of NO_SIGNALING_NANS macro has FPU > implementations with and without the corresponding property. With > NO_SIGNALING_NANS being a macro they cannot be a part of the same QEMU > executable. > Replace macro with new property in float_status to allow cores with > different FPU implementations coexist. > > Cc: Peter Maydell > Cc: "Alex Bennée" > Signed-off-by: Max Filippov > --- > fpu/softfloat-specialize.inc.c | 228 > include/fpu/softfloat-helpers.h | 5 + > include/fpu/softfloat-types.h | 1 + > 3 files changed, 117 insertions(+), 117 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v7 07/47] block: Add bdrv_supports_compressed_writes()
On 25.06.2020 18:21, Max Reitz wrote:
Filters cannot compress data themselves but they have to implement
.bdrv_co_pwritev_compressed() still (or they cannot forward compressed
writes). Therefore, checking whether
bs->drv->bdrv_co_pwritev_compressed is non-NULL is not sufficient to
know whether the node can actually handle compressed writes. This
function looks down the filter chain to see whether there is a
non-filter that can actually convert the compressed writes into
compressed data (and thus normal writes).
Signed-off-by: Max Reitz
---
include/block/block.h | 1 +
block.c | 23 +++
2 files changed, 24 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index 0080fe1311..a905a5ec05 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -538,6 +538,7 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it);
void bdrv_next_cleanup(BdrvNextIterator *it);
BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs);
+bool bdrv_supports_compressed_writes(BlockDriverState *bs);
void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
void *opaque, bool read_only);
const char *bdrv_get_node_name(const BlockDriverState *bs);
diff --git a/block.c b/block.c
index 76277ea4e0..6449f3a11d 100644
--- a/block.c
+++ b/block.c
@@ -5044,6 +5044,29 @@ bool bdrv_is_sg(BlockDriverState *bs)
return bs->sg;
}
+/**
+ * Return whether the given node supports compressed writes.
+ */
+bool bdrv_supports_compressed_writes(BlockDriverState *bs)
+{
+BlockDriverState *filtered;
+
+if (!bs->drv || !block_driver_can_compress(bs->drv)) {
+return false;
+}
+
+filtered = bdrv_filter_bs(bs);
+if (filtered) {
+/*
+ * Filters can only forward compressed writes, so we have to
+ * check the child.
+ */
+return bdrv_supports_compressed_writes(filtered);
+}
+
+return true;
+}
+
const char *bdrv_get_format_name(BlockDriverState *bs)
{
return bs->drv ? bs->drv->format_name : NULL;
Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 08/47] throttle: Support compressed writes
On 25.06.2020 18:21, Max Reitz wrote:
Signed-off-by: Max Reitz
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
block/throttle.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/throttle.c b/block/throttle.c
index 0ebbad0743..f6e619aca2 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -154,6 +154,15 @@ static int coroutine_fn
throttle_co_pdiscard(BlockDriverState *bs,
return bdrv_co_pdiscard(bs->file, offset, bytes);
}
+static int coroutine_fn throttle_co_pwritev_compressed(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov)
+{
+return throttle_co_pwritev(bs, offset, bytes, qiov,
+ BDRV_REQ_WRITE_COMPRESSED);
+}
+
static int throttle_co_flush(BlockDriverState *bs)
{
return bdrv_co_flush(bs->file->bs);
@@ -246,6 +255,7 @@ static BlockDriver bdrv_throttle = {
.bdrv_co_pwrite_zeroes = throttle_co_pwrite_zeroes,
.bdrv_co_pdiscard = throttle_co_pdiscard,
+.bdrv_co_pwritev_compressed = throttle_co_pwritev_compressed,
.bdrv_attach_aio_context= throttle_attach_aio_context,
.bdrv_detach_aio_context= throttle_detach_aio_context,
Reviewed-by: Andrey Shinkevich
[RFC PATCH] configure: remove all dependencies on a (re)configure
The previous code was brittle and missed cases such as the mipn32
variants which for some reason has the 64 bit syscalls. This leads to
a number of binary targets having deps lines like:
all.clang-sanitizer/mipsn32el-linux-user/linux-user/signal.d
140: /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
455:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
all.clang-sanitizer/mipsn32el-linux-user/linux-user/syscall.d
146: /home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h \
485:/home/alex/lsrc/qemu.git/linux-user/mips64/syscall_nr.h:
which in turn would trigger the re-generation of syscall_nr.h in the
source tree (thanks to generic %/syscall_nr.h rules). The previous
code attempts to clean it out but misses edge cases but fails.
After spending a day trying to understand how this was happening I'm
unconvinced that there are not other such breakages possible with this
"caching". As we add more auto-generated code to the build it is likely
to trip up again. Apply a hammer to the problem.
Fixes: 91e5998f18 (which fixes 5f29856b852d and 4d6a835dea47)
Signed-off-by: Alex Bennée
---
configure | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index d2d93ae51e47..81ef123fb187 100755
--- a/configure
+++ b/configure
@@ -1950,23 +1950,20 @@ EOF
exit 0
fi
-# Remove old dependency files to make sure that they get properly regenerated
-rm -f */config-devices.mak.d
-
# Remove syscall_nr.h to be sure they will be regenerated in the build
# directory, not in the source directory
for arch in alpha hppa m68k xtensa sh4 microblaze arm ppc s390x sparc sparc64 \
i386 x86_64 mips mips64 ; do
# remove the file if it has been generated in the source directory
rm -f "${source_path}/linux-user/${arch}/syscall_nr.h"
-# remove the dependency files
-for target in ${arch}*-linux-user ; do
-test -d "${target}" && find "${target}" -type f -name "*.d" \
- -exec grep -q "${source_path}/linux-user/${arch}/syscall_nr.h" {}
\; \
- -print | while read file ; do rm "${file}" "${file%.d}.o" ; done
-done
done
+# Clean out all old dependency files. As more files are generated we
+# run the risk of old dependencies triggering generation in the wrong
+# places. Previous brittle attempts to be surgical tend to miss edge
+# cases leading to wasted time and much confusion.
+find -type f -name "*.d" -exec rm -f {} \;
+
if test -z "$python"
then
error_exit "Python not found. Use --python=/path/to/python"
--
2.20.1
Re: [PATCH v7 09/47] copy-on-read: Support compressed writes
On 25.06.2020 18:21, Max Reitz wrote:
Signed-off-by: Max Reitz
---
block/copy-on-read.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index a6e3c74a68..a6a864f147 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -107,6 +107,16 @@ static int coroutine_fn cor_co_pdiscard(BlockDriverState
*bs,
}
+static int coroutine_fn cor_co_pwritev_compressed(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov)
+{
+return bdrv_co_pwritev(bs->file, offset, bytes, qiov,
+ BDRV_REQ_WRITE_COMPRESSED);
+}
+
+
static void cor_eject(BlockDriverState *bs, bool eject_flag)
{
bdrv_eject(bs->file->bs, eject_flag);
@@ -131,6 +141,7 @@ static BlockDriver bdrv_copy_on_read = {
.bdrv_co_pwritev= cor_co_pwritev,
.bdrv_co_pwrite_zeroes = cor_co_pwrite_zeroes,
.bdrv_co_pdiscard = cor_co_pdiscard,
+.bdrv_co_pwritev_compressed = cor_co_pwritev_compressed,
.bdrv_eject = cor_eject,
.bdrv_lock_medium = cor_lock_medium,
Reviewed-by: Andrey Shinkevich
Re: [PATCH v4 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models
On 7/8/20 7:31 PM, Philippe Mathieu-Daudé wrote:
> Hi Havard,
>
> On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
>> The Nuvoton NPCM7xx SoC family are used to implement Baseboard
>> Management Controllers in servers. While the family includes four SoCs,
>> this patch implements limited support for two of them: NPCM730 (targeted
>> for Data Center applications) and NPCM750 (targeted for Enterprise
>> applications).
>>
>> This patch includes little more than the bare minimum needed to boot a
>> Linux kernel built with NPCM7xx support in direct-kernel mode:
>>
>> - Two Cortex-A9 CPU cores with built-in periperhals.
>> - Global Configuration Registers.
>> - Clock Management.
>> - 3 Timer Modules with 5 timers each.
>> - 4 serial ports.
>>
>> The chips themselves have a lot more features, some of which will be
>> added to the model at a later stage.
[...]
>> +static void npcm7xx_realize(DeviceState *dev, Error **errp)
>> +{
>> +NPCM7xxState *s = NPCM7XX(dev);
>> +NPCM7xxClass *nc = NPCM7XX_GET_CLASS(s);
>> +Error *err = NULL;
>> +int i;
>> +
>> +/* I/O space -- unimplemented unless overridden below. */
>> +create_unimplemented_device("npcm7xx.io", NPCM7XX_MMIO_BA,
>> NPCM7XX_MMIO_SZ);
>
> I still insist this is not the best, but as "The data sheet for these
> SoCs is not generally available" there is not much I can suggest to
> improve.
>From your other comment I found:
https://github.com/Nuvoton-Israel/bootblock/blob/master/SWC_HAL/Chips/npcm750/npcm750.h
In particular:
#define AHB1_BASE_ADDR 0xF000 /* AHB1
allocation (Including APB allocations) */
#define AHB18_BASE_ADDR 0x8000 /* AHB18
allocation */
#define AHB3_BASE_ADDR 0xA000 /* AHB3
allocation */
#define XBUSR_BASE_ADDR 0xC0002000 /* XBUS
registers */
#define AHB14_BASE_ADDR 0xE000 /* AHB14
Allocation */
#define APB14_BASE_ADDR 0xE000 /* APB14
Allocation */
#define VDMX_BASE_ADDR 0xE080 /* VDMX */
XBUS doesn't seem important.
If SPI flashes aren't connected, returning bus transaction sounds
correct:
#define SPI0CS0_BASE_ADDR 0x8000 /* SPI0 direct
access CS0 */
#define SPI0CS1_BASE_ADDR 0x8800 /* SPI0 direct
access CS1 */
#define SPI0CS2_BASE_ADDR 0x9000 /* SPI0 direct
access CS2 */
#define SPI0CS3_BASE_ADDR 0x9800 /* SPI0 direct
access CS3 */
#define SPI3CS0_BASE_ADDR 0xA000 /* SPI3 direct
access CS0 */
#define SPI3CS1_BASE_ADDR 0xA800 /* SPI3 direct
access CS1 */
#define SPI3CS2_BASE_ADDR 0xB000 /* SPI3 direct
access CS2 */
#define SPI3CS3_BASE_ADDR 0xB800 /* SPI3 direct
access CS3 */
So I'd prefer you use:
create_unimplemented_device("npcm7xx.AHB1", 0xf000, 256 * MiB);
Maybe for the PCI root complex:
create_unimplemented_device("npcm7xx.AHB14", 0xe000, 256 * MiB);
What do you think?
Regards,
Phil.
Re: [PATCH v7 10/47] mirror-top: Support compressed writes
On 25.06.2020 18:21, Max Reitz wrote:
Signed-off-by: Max Reitz
---
block/mirror.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/mirror.c b/block/mirror.c
index e8e8844afc..469acf4600 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1480,6 +1480,15 @@ static int coroutine_fn
bdrv_mirror_top_pdiscard(BlockDriverState *bs,
NULL, 0);
}
+static int coroutine_fn bdrv_mirror_top_pwritev_compressed(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov)
+{
+return bdrv_mirror_top_pwritev(bs, offset, bytes, qiov,
+ BDRV_REQ_WRITE_COMPRESSED);
+}
+
static void bdrv_mirror_top_refresh_filename(BlockDriverState *bs)
{
if (bs->backing == NULL) {
@@ -1526,6 +1535,7 @@ static BlockDriver bdrv_mirror_top = {
.bdrv_co_pwritev= bdrv_mirror_top_pwritev,
.bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
.bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
+.bdrv_co_pwritev_compressed = bdrv_mirror_top_pwritev_compressed,
.bdrv_co_flush = bdrv_mirror_top_flush,
.bdrv_co_block_status = bdrv_co_block_status_from_backing,
.bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
Reviewed-by: Andrey Shinkevich
Re: [PATCH v7 11/47] backup-top: Support compressed writes
On 25.06.2020 18:21, Max Reitz wrote:
Signed-off-by: Max Reitz
---
block/backup-top.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/block/backup-top.c b/block/backup-top.c
index af2f20f346..f304df8f26 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -99,6 +99,15 @@ static coroutine_fn int
backup_top_co_pwritev(BlockDriverState *bs,
return bdrv_co_pwritev(bs->backing, offset, bytes, qiov, flags);
}
+static coroutine_fn int backup_top_co_pwritev_compressed(BlockDriverState *bs,
+ uint64_t offset,
+ uint64_t bytes,
+ QEMUIOVector *qiov)
+{
+return backup_top_co_pwritev(bs, offset, bytes, qiov,
+ BDRV_REQ_WRITE_COMPRESSED);
+}
+
static int coroutine_fn backup_top_co_flush(BlockDriverState *bs)
{
if (!bs->backing) {
@@ -173,6 +182,7 @@ BlockDriver bdrv_backup_top_filter = {
.bdrv_co_pwritev= backup_top_co_pwritev,
.bdrv_co_pwrite_zeroes = backup_top_co_pwrite_zeroes,
.bdrv_co_pdiscard = backup_top_co_pdiscard,
+.bdrv_co_pwritev_compressed = backup_top_co_pwritev_compressed,
.bdrv_co_flush = backup_top_co_flush,
.bdrv_co_block_status = bdrv_co_block_status_from_backing,
Reviewed-by: Andrey Shinkevich
Re: [PATCH 03/21] softfloat: add xtensa specialization for pickNaNMulAdd
On Wed, Jul 8, 2020 at 9:07 AM Richard Henderson wrote: > > On 7/6/20 4:47 PM, Max Filippov wrote: > > pickNaNMulAdd logic on Xtensa is the same as pickNaN when applied to > > the expression (a * b) + c. So with two pickNaN variants there must be > > two pickNaNMulAdd variants. > > "Is the same as"? > > I question the non-use of the infzero parameter. > > When infzero, (a * b) = (Inf * 0), which will produce a default QNaN. Your > sentence above would suggest that pickNaN is applied twice, so that if > use_first_nan, the default nan is chosen above any nan within c. Apparently the description it's wrong: only NaNs in arguments are considered, default NaN produced as a result of (a * b) is never chosen when c is NaN. > In addition, is the invalid flag raised for (Inf * 0) + NaN? Does that happen > regardless of the use_first_nan setting, or does the whole operation > short-circuit? Flag setting happens regardless of which NaN is chosen. I'll post v3 with a fix for this and additional tests. -- Thanks. -- Max
Re: [PATCH v4 05/12] hw/arm: Add NPCM730 and NPCM750 SoC models
On Wed, Jul 8, 2020 at 10:31 AM Philippe Mathieu-Daudé wrote:
>
> Hi Havard,
>
> On 7/7/20 8:47 PM, Havard Skinnemoen wrote:
> > The Nuvoton NPCM7xx SoC family are used to implement Baseboard
> > Management Controllers in servers. While the family includes four SoCs,
> > this patch implements limited support for two of them: NPCM730 (targeted
> > for Data Center applications) and NPCM750 (targeted for Enterprise
> > applications).
> >
> > This patch includes little more than the bare minimum needed to boot a
> > Linux kernel built with NPCM7xx support in direct-kernel mode:
> >
> > - Two Cortex-A9 CPU cores with built-in periperhals.
> > - Global Configuration Registers.
> > - Clock Management.
> > - 3 Timer Modules with 5 timers each.
> > - 4 serial ports.
> >
> > The chips themselves have a lot more features, some of which will be
> > added to the model at a later stage.
> >
> > Reviewed-by: Tyrone Ting
> > Reviewed-by: Joel Stanley
> > Signed-off-by: Havard Skinnemoen
> > ---
> > hw/arm/Makefile.objs | 1 +
> > hw/arm/npcm7xx.c | 328 +++
> > include/hw/arm/npcm7xx.h | 81 ++
>
> Please have a look at the scripts/git.orderfile, using it would
> ease our reviews.
Oh cool.
git config diff.orderFile scripts/git.orderfile
>
> > 3 files changed, 410 insertions(+)
> > create mode 100644 hw/arm/npcm7xx.c
> > create mode 100644 include/hw/arm/npcm7xx.h
> >
> > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> > index 534a6a119e..13d163a599 100644
> > --- a/hw/arm/Makefile.objs
> > +++ b/hw/arm/Makefile.objs
> > @@ -41,6 +41,7 @@ obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
> > obj-$(CONFIG_STM32F405_SOC) += stm32f405_soc.o
> > obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
> > obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
> > +obj-$(CONFIG_NPCM7XX) += npcm7xx.o
> > obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
> > obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
> > obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
> > diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> > new file mode 100644
> > index 00..0a9e30f66f
> > --- /dev/null
> > +++ b/hw/arm/npcm7xx.c
> > @@ -0,0 +1,328 @@
> > +/*
> > + * Nuvoton NPCM7xx SoC family.
> > + *
> > + * Copyright 2020 Google LLC
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License as published by the
> > + * Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> > + * for more details.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "exec/address-spaces.h"
> > +#include "hw/arm/npcm7xx.h"
> > +#include "hw/char/serial.h"
> > +#include "hw/loader.h"
> > +#include "hw/misc/unimp.h"
> > +#include "hw/qdev-properties.h"
> > +#include "qapi/error.h"
> > +#include "qemu/units.h"
> > +#include "sysemu/sysemu.h"
> > +
> > +/* The first half of the address space is reserved for DDR4 DRAM. */
> > +#define NPCM7XX_DRAM_BA (0x)
> > +#define NPCM7XX_DRAM_SZ (2 * GiB)
> > +
> > +/*
> > + * This covers the whole MMIO space. We'll use this to catch any MMIO
> > accesses
> > + * that aren't handled by any device.
> > + */
> > +#define NPCM7XX_MMIO_BA (0x8000)
> > +#define NPCM7XX_MMIO_SZ (0x7FFD)
> > +
> > +/* Core system modules. */
> > +#define NPCM7XX_L2C_BA (0xF03FC000)
> > +#define NPCM7XX_CPUP_BA (0xF03FE000)
> > +#define NPCM7XX_GCR_BA (0xF080)
> > +#define NPCM7XX_CLK_BA (0xF0801000)
> > +
> > +/* Memory blocks at the end of the address space */
> > +#define NPCM7XX_RAM2_BA (0xFFFD)
> > +#define NPCM7XX_RAM2_SZ (128 * KiB)
> > +#define NPCM7XX_ROM_BA (0x)
> > +#define NPCM7XX_ROM_SZ (64 * KiB)
> > +
> > +/*
> > + * Interrupt lines going into the GIC. This does not include internal
> > Cortex-A9
> > + * interrupts.
> > + */
> > +enum NPCM7xxInterrupt {
> > +NPCM7XX_UART0_IRQ = 2,
> > +NPCM7XX_UART1_IRQ,
> > +NPCM7XX_UART2_IRQ,
> > +NPCM7XX_UART3_IRQ,
> > +NPCM7XX_TIMER0_IRQ = 32, /* Timer Module 0 */
> > +NPCM7XX_TIMER1_IRQ,
> > +NPCM7XX_TIMER2_IRQ,
> > +NPCM7XX_TIMER3_IRQ,
> > +NPCM7XX_TIMER4_IRQ,
> > +NPCM7XX_TIMER5_IRQ, /* Timer Module 1 */
> > +NPCM7XX_TIMER6_IRQ,
> > +NPCM7XX_TIMER7_IRQ,
> > +NPCM7XX_TIMER8_IRQ,
> > +NPCM7XX_TIMER9_IRQ,
> > +NPCM7XX_TIMER10_IRQ,/* Timer Module 2 */
> > +NPCM7XX_TIMER11_IRQ,
> > +NPCM7XX_TIMER12_IRQ,
> > +NPCM7XX_TIMER13_IRQ,
> > +NPCM7XX_TIMER14_IRQ,
> > +};
>
Re: [PULL 00/53] Misc patches for QEMU 5.1 soft freeze
On 7/8/20 7:03 PM, Claudio Fontana wrote: > On 7/8/20 6:55 PM, Paolo Bonzini wrote: >> On 08/07/20 18:45, Claudio Fontana wrote: >>> C++ is used to link the final qemu-system binary and on my system c++ has >>> LTO: >>> >>> c++ -v >>> Using built-in specs. >>> COLLECT_GCC=c++ >>> COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/7/lto-wrapper >>> OFFLOAD_TARGET_NAMES=hsa:nvptx-none >>> Target: x86_64-suse-linux >>> Configured with: ../configure --prefix=/usr --infodir=/usr/share/info >>> --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 >>> --enable-languages=c,c++,objc,fortran,obj-c++,ada,go >>> --enable-offload-targets=hsa,nvptx-none=/usr/nvptx-none, >>> --without-cuda-driver --enable-checking=release --disable-werror >>> --with-gxx-include-dir=/usr/include/c++/7 --enable-ssp --disable-libssp >>> --disable-libvtv --disable-libcc1 --disable-plugin >>> --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' >>> --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new >>> --disable-libstdcxx-pch --enable-version-specific-runtime-libs >>> --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex >>> --enable-gnu-indirect-function --program-suffix=-7 >>> --without-system-libunwind --enable-multilib --with-arch-32=x86-64 >>> --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux >>> Thread model: posix >>> gcc version 7.5.0 (SUSE Linux) >>> >>> >>> I checked cc but did not think to check c++ . I will find a way to disable >>> this thing and will correct the patch accordingly. >> >> Having LTO support is not the same thing as having it enabled. Are you >> compiling and linking with "-flto"? >> >> Paolo >> > > no, the compilation and link stage do not show this explicit parameter. I am > puzzled. > > C > This is really weird, as I cannot reproduce it. What I did notice is that all the code that directly or indirectly uses the functions is under an if (0) ( ) since tcg_enabled is the constant 0. By "indirectly" I mean that the static void qemu_tcg_cpu_thread_fn() function that calls those is referenced only by static void qemu_tcg_init_vcpu(), which is called only under an if (0), ie if (tcg_enabled()). For me this builds with --disable-tcg and with --enable-tcg . Maybe the issue is actually the name clash of the stub and the module? I admit I am not familiar with the rationale of why the stubs are all built regardless, could we have that icount.o from stubs/ is replacing softmmu/icount.o to cause this? Thanks, Claudio
Re: [PATCH v7 12/47] block: Use bdrv_filter_(bs|child) where obvious
On 25.06.2020 18:21, Max Reitz wrote:
Places that use patterns like
if (bs->drv->is_filter && bs->file) {
... something about bs->file->bs ...
}
should be
BlockDriverState *filtered = bdrv_filter_bs(bs);
if (filtered) {
... something about @filtered ...
}
instead.
Signed-off-by: Max Reitz
---
block.c| 31 ---
block/io.c | 7 +--
migration/block-dirty-bitmap.c | 8 +---
3 files changed, 26 insertions(+), 20 deletions(-)
...
diff --git a/block/io.c b/block/io.c
index df8f2a98d4..385176b331 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3307,6 +3307,7 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
Error **errp)
{
BlockDriverState *bs = child->bs;
+BdrvChild *filtered;
BlockDriver *drv = bs->drv;
BdrvTrackedRequest req;
int64_t old_size, new_bytes;
@@ -3358,6 +3359,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
goto out;
}
+filtered = bdrv_filter_child(bs);
+
Isn't better to have this initialization right before the relevant
if/else block?
Andrey
/*
* If the image has a backing file that is large enough that it would
* provide data for the new area, we cannot leave it unallocated because
@@ -3390,8 +3393,8 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
int64_t offset, bool exact,
goto out;
}
ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, flags, errp);
-} else if (bs->file && drv->is_filter) {
-ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp);
+} else if (filtered) {
+ret = bdrv_co_truncate(filtered, offset, exact, prealloc, flags, errp);
} else {
error_setg(errp, "Image format driver does not support resize");
ret = -ENOTSUP;
...
Reviewed-by: Andrey Shinkevich
Re: [PULL 00/53] Misc patches for QEMU 5.1 soft freeze
On 7/8/20 8:25 PM, Claudio Fontana wrote: > On 7/8/20 7:03 PM, Claudio Fontana wrote: >> On 7/8/20 6:55 PM, Paolo Bonzini wrote: >>> On 08/07/20 18:45, Claudio Fontana wrote: C++ is used to link the final qemu-system binary and on my system c++ has LTO: c++ -v Using built-in specs. COLLECT_GCC=c++ COLLECT_LTO_WRAPPER=/usr/lib64/gcc/x86_64-suse-linux/7/lto-wrapper OFFLOAD_TARGET_NAMES=hsa:nvptx-none Target: x86_64-suse-linux Configured with: ../configure --prefix=/usr --infodir=/usr/share/info --mandir=/usr/share/man --libdir=/usr/lib64 --libexecdir=/usr/lib64 --enable-languages=c,c++,objc,fortran,obj-c++,ada,go --enable-offload-targets=hsa,nvptx-none=/usr/nvptx-none, --without-cuda-driver --enable-checking=release --disable-werror --with-gxx-include-dir=/usr/include/c++/7 --enable-ssp --disable-libssp --disable-libvtv --disable-libcc1 --disable-plugin --with-bugurl=https://bugs.opensuse.org/ --with-pkgversion='SUSE Linux' --with-slibdir=/lib64 --with-system-zlib --enable-libstdcxx-allocator=new --disable-libstdcxx-pch --enable-version-specific-runtime-libs --with-gcc-major-version-only --enable-linker-build-id --enable-linux-futex --enable-gnu-indirect-function --program-suffix=-7 --without-system-libunwind --enable-multilib --with-arch-32=x86-64 --with-tune=generic --build=x86_64-suse-linux --host=x86_64-suse-linux Thread model: posix gcc version 7.5.0 (SUSE Linux) I checked cc but did not think to check c++ . I will find a way to disable this thing and will correct the patch accordingly. >>> >>> Having LTO support is not the same thing as having it enabled. Are you >>> compiling and linking with "-flto"? >>> >>> Paolo >>> >> >> no, the compilation and link stage do not show this explicit parameter. I am >> puzzled. >> >> C >> > > This is really weird, as I cannot reproduce it. > > What I did notice is that all the code that directly or indirectly uses the > functions is under an > > if (0) ( > ) > > since tcg_enabled is the constant 0. > > By "indirectly" I mean that the static void qemu_tcg_cpu_thread_fn() function > that calls those is referenced only by static void qemu_tcg_init_vcpu(), > which is called only under an if (0), > ie if (tcg_enabled()). > > For me this builds with --disable-tcg and with --enable-tcg . > > Maybe the issue is actually the name clash of the stub and the module? > > I admit I am not familiar with the rationale of why the stubs are all built > regardless, could we have that icount.o from stubs/ is replacing > softmmu/icount.o to cause this? > > Thanks, > > Claudio > > So I guess if you could Peter, are you building there with --disable-tcg or with --enable-tcg when you got that error? Ciao, Claudio
Re: [PATCH 7/7] block/io: improve loadvm performance
03.07.2020 20:35, Denis V. Lunev wrote:
This patch creates intermediate buffer for reading from block driver
state and performs read-ahead to this buffer. Snapshot code performs
reads sequentially and thus we know what offsets will be required
and when they will become not needed.
Results are fantastic. Switch to snapshot times of 2GB Fedora 31 VM
over NVME storage are the following:
original fixed
cached: 1.84s 1.16s
non-cached: 12.74s 1.27s
The difference over HDD would be more significant :)
[..]
+static coroutine_fn int bdrv_co_vmstate_load_task_entry(AioTask *task)
+{
+int err = 0;
+BdrvLoadVMStateTask *t = container_of(task, BdrvLoadVMStateTask, task);
+BdrvLoadVMChunk *c = t->chunk;
+BdrvLoadVMState *state = t->bs->loadvm_state;
+QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, c->buf, c->bytes);
+
+bdrv_inc_in_flight(t->bs);
+err = t->bs->drv->bdrv_load_vmstate(t->bs, &qiov, c->offset);
+bdrv_dec_in_flight(t->bs);
+
+qemu_co_mutex_lock(&state->lock);
In my understanding, we don't need this mutex. We need coroutine mutex, when
there
is a critical section with an yield inside, like accessing metadata in qcow2
code.
If all the critical sections doesn't have an yield inside they should not
intersect
anyway, as all these functions should be executed in coroutines in the aio
context
of this bs, i.e. not in parallel iothreads simultaneously.
+QLIST_REMOVE(c, list);
+if (err == 0) {
+QLIST_INSERT_HEAD(&state->chunks, c, list);
+} else {
+bdrv_free_loadvm_chunk(c);
+}
+qemu_co_mutex_unlock(&state->lock);
+qemu_co_queue_restart_all(&state->waiters);
+
+return err;
+}
[..]
+static int bdrv_co_do_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
+ int64_t pos)
+{
+BdrvLoadVMState *state = bs->loadvm_state;
+BdrvLoadVMChunk *c;
+size_t off;
+int64_t start_pos = pos;
+
+if (state == NULL) {
+if (pos != 0) {
+goto slow_path;
+}
+
+state = g_new(BdrvLoadVMState, 1);
+*state = (BdrvLoadVMState) {
+.pool = aio_task_pool_new(BDRV_VMSTATE_WORKERS_MAX),
+.chunks = QLIST_HEAD_INITIALIZER(state->chunks),
+.loading = QLIST_HEAD_INITIALIZER(state->loading),
+};
+qemu_co_mutex_init(&state->lock);
+qemu_co_queue_init(&state->waiters);
+
+bs->loadvm_state = state;
+
+bdrv_co_loadvmstate_start(bs);
+}
+
+if (state->offset != pos) {
+goto slow_path;
+}
+
+off = 0;
+
+qemu_co_mutex_lock(&state->lock);
+while (off < qiov->size && aio_task_pool_status(state->pool) == 0) {
+c = bdrv_co_find_loadvmstate_chunk(pos, QLIST_FIRST(&state->chunks));
+if (c != NULL) {
+ssize_t chunk_off = pos - c->offset;
+ssize_t to_copy = MIN(qiov->size - off, c->bytes - chunk_off);
+
+qemu_iovec_from_buf(qiov, off, c->buf + chunk_off, to_copy);
+
+off += to_copy;
+pos += to_copy;
+
+if (pos == c->offset + c->bytes) {
+state->chunk_count--;
+/* End of buffer, discard it from the list */
+QLIST_REMOVE(c, list);
+
+/*
+ * Start loading next chunk. The slot in the pool should
+ * always be free for this purpose at the moment.
+ *
+ * There is a problem with the end of the stream. This code
+ * starts to read the data beyond the end of the saved state.
+ * The code in low level should be ready to such behavior but
+ * we will have unnecessary BDRV_VMSTATE_WORKERS_MAX chunks
+ * fully zeroed. This is not good, but acceptable.
As I understand, we don't know where is the end of state, yes? So, we can't
avoid it anyway. Should not be a problem.
+ */
+bdrv_co_loadvmstate_next(bs, c);
+}
+
+state->offset += to_copy;
+continue;
+}
+
+c = bdrv_co_find_loadvmstate_chunk(pos, QLIST_FIRST(&state->loading));
+if (c != NULL) {
+qemu_co_queue_wait(&state->waiters, &state->lock);
+continue;
+}
+
+/*
+ * This should not happen normally. This point could be reached only
+ * if we have had some parallel requests. Fallback to slow load.
+ */
+qemu_co_mutex_unlock(&state->lock);
+
+slow_path:
A kind of taste, but I'd prefer to keep it in the end of the function, avoiding
gotos leading to loop body.
+return bs->drv->bdrv_load_vmstate(bs, qiov, start_pos);
+}
+qemu_co_mutex_unlock(&state->lock);
+
+return aio_task_pool_status(state->pool);
+}
+
static int coroutine_fn
bdrv_co_rw_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, int64_t pos,
Re: [PATCH v7 13/47] block: Use CAFs in block status functions
On 25.06.2020 18:21, Max Reitz wrote:
Use the child access functions in the block status inquiry functions as
appropriate.
Signed-off-by: Max Reitz
---
block/io.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/block/io.c b/block/io.c
index 385176b331..dc9891d6ce 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2407,11 +2407,12 @@ static int coroutine_fn
bdrv_co_block_status(BlockDriverState *bs,
if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
ret |= BDRV_BLOCK_ALLOCATED;
} else if (want_zero) {
+BlockDriverState *cow_bs = bdrv_cow_bs(bs);
+
if (bdrv_unallocated_blocks_are_zero(bs)) {
ret |= BDRV_BLOCK_ZERO;
-} else if (bs->backing) {
-BlockDriverState *bs2 = bs->backing->bs;
-int64_t size2 = bdrv_getlength(bs2);
+} else if (cow_bs) {
+int64_t size2 = bdrv_getlength(cow_bs);
if (size2 >= 0 && offset >= size2) {
ret |= BDRV_BLOCK_ZERO;
@@ -2477,7 +2478,7 @@ static int coroutine_fn
bdrv_co_block_status_above(BlockDriverState *bs,
bool first = true;
assert(bs != base);
-for (p = bs; p != base; p = backing_bs(p)) {
+for (p = bs; p != base; p = bdrv_filter_or_cow_bs(p)) {
ret = bdrv_co_block_status(p, want_zero, offset, bytes, pnum, map,
file);
if (ret < 0) {
@@ -2551,7 +2552,7 @@ int bdrv_block_status_above(BlockDriverState *bs,
BlockDriverState *base,
int bdrv_block_status(BlockDriverState *bs, int64_t offset, int64_t bytes,
int64_t *pnum, int64_t *map, BlockDriverState **file)
{
-return bdrv_block_status_above(bs, backing_bs(bs),
+return bdrv_block_status_above(bs, bdrv_filter_or_cow_bs(bs),
offset, bytes, pnum, map, file);
}
@@ -2561,9 +2562,9 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset,
int ret;
int64_t dummy;
-ret = bdrv_common_block_status_above(bs, backing_bs(bs), false, offset,
- bytes, pnum ? pnum : &dummy, NULL,
- NULL);
+ret = bdrv_common_block_status_above(bs, bdrv_filter_or_cow_bs(bs), false,
+ offset, bytes, pnum ? pnum : &dummy,
+ NULL, NULL);
if (ret < 0) {
return ret;
}
@@ -2626,7 +2627,7 @@ int bdrv_is_allocated_above(BlockDriverState *top,
break;
}
-intermediate = backing_bs(intermediate);
+intermediate = bdrv_filter_or_cow_bs(intermediate);
}
*pnum = n;
Reviewed-by: Andrey Shinkevich
[PATCH 2/2] util/qemu-sockets: make keep-alive enabled by default
Keep-alive won't hurt, let's try to enable it even if not requested by
user.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
util/qemu-sockets.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b961963472..f6851376f5 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -438,7 +438,8 @@ static struct addrinfo
*inet_parse_connect_saddr(InetSocketAddress *saddr,
*
* Handle keep_alive settings. If user specified settings explicitly, fail if
* can't set the settings. If user just enabled keep-alive, not specifying the
- * settings, try to set defaults but ignore failures.
+ * settings, try to set defaults but ignore failures. If keep-alive option is
+ * not specified, try to set it but ignore failures.
*/
static int inet_set_keepalive(int sock, bool has_keep_alive,
KeepAliveField *keep_alive, Error **errp)
@@ -447,8 +448,8 @@ static int inet_set_keepalive(int sock, bool has_keep_alive,
int val;
bool has_settings = has_keep_alive && keep_alive->type == QTYPE_QDICT;
-if (!has_keep_alive || (keep_alive->type == QTYPE_QBOOL &&
-!keep_alive->u.enabled))
+if (has_keep_alive &&
+keep_alive->type == QTYPE_QBOOL && !keep_alive->u.enabled)
{
return 0;
}
@@ -456,8 +457,12 @@ static int inet_set_keepalive(int sock, bool
has_keep_alive,
val = 1;
ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val));
if (ret < 0) {
-error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
-return -1;
+if (has_keep_alive) {
+error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
+return -1;
+} else {
+return 0;
+}
}
val = has_settings ? keep_alive->u.settings.idle : 30;
--
2.21.0
[PATCH 1/2] sockets: keep-alive settings
Introduce keep-alive settings (TCP_KEEPCNT, TCP_KEEPIDLE,
TCP_KEEPINTVL) and chose some defaults.
The linux default of 2 hours for /proc/tcp_keepalive_time
(corresponding to TCP_KEEPIDLE) makes keep-alive option almost
superfluous. Let's add a possibility to set the options by hand
and specify some defaults resulting in smaller total time to terminate
idle connection.
Do not document the default values in QAPI as they may be altered in
future (careful user will use explicit values).
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
Suggested default numbers are RFC, any better suggestion is welcome.
I just looked at /etc/libvirt/qemu.conf in my system and take values of
keepalive_interval and keepalive_count.
The only thing I'm sure in is that 2 hours is too long.
qapi/sockets.json | 33 +++-
util/qemu-sockets.c | 76 ++---
2 files changed, 97 insertions(+), 12 deletions(-)
diff --git a/qapi/sockets.json b/qapi/sockets.json
index cbd6ef35d0..73ff66a5d5 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -37,6 +37,37 @@
'host': 'str',
'port': 'str' } }
+##
+# @KeepAliveSettings:
+#
+# @idle: The time (in seconds) the connection needs to remain idle
+#before TCP starts sending keepalive probes (sets TCP_KEEPIDLE).
+# @interval: The time (in seconds) between individual keepalive probes
+#(sets TCP_KEEPINTVL).
+# @count: The maximum number of keepalive probes TCP should send before
+# dropping the connection (sets TCP_KEEPCNT).
+#
+# Since: 5.2
+##
+{ 'struct': 'KeepAliveSettings',
+ 'data': {
+'idle': 'int',
+'interval': 'int',
+'count': 'int' } }
+
+##
+# @KeepAliveField:
+#
+# @enabled: If true, enable keep-alive with some default settings
+# @settings: Enable keep-alive and use explicit settings
+#
+# Since: 5.2
+##
+{ 'alternate': 'KeepAliveField',
+ 'data': {
+'enabled': 'bool',
+'settings': 'KeepAliveSettings' } }
+
##
# @InetSocketAddress:
#
@@ -65,7 +96,7 @@
'*to': 'uint16',
'*ipv4': 'bool',
'*ipv6': 'bool',
-'*keep-alive': 'bool' } }
+'*keep-alive': 'KeepAliveField' } }
##
# @UnixSocketAddress:
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b37d288866..b961963472 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -433,6 +433,57 @@ static struct addrinfo
*inet_parse_connect_saddr(InetSocketAddress *saddr,
return res;
}
+/*
+ * inet_set_keepalive
+ *
+ * Handle keep_alive settings. If user specified settings explicitly, fail if
+ * can't set the settings. If user just enabled keep-alive, not specifying the
+ * settings, try to set defaults but ignore failures.
+ */
+static int inet_set_keepalive(int sock, bool has_keep_alive,
+ KeepAliveField *keep_alive, Error **errp)
+{
+int ret;
+int val;
+bool has_settings = has_keep_alive && keep_alive->type == QTYPE_QDICT;
+
+if (!has_keep_alive || (keep_alive->type == QTYPE_QBOOL &&
+!keep_alive->u.enabled))
+{
+return 0;
+}
+
+val = 1;
+ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val));
+if (ret < 0) {
+error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
+return -1;
+}
+
+val = has_settings ? keep_alive->u.settings.idle : 30;
+ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val));
+if (has_settings && ret < 0) {
+error_setg_errno(errp, errno, "Unable to set TCP_KEEPIDLE");
+return -1;
+}
+
+val = has_settings ? keep_alive->u.settings.interval : 30;
+ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val));
+if (has_settings && ret < 0) {
+error_setg_errno(errp, errno, "Unable to set TCP_KEEPINTVL");
+return -1;
+}
+
+val = has_settings ? keep_alive->u.settings.count : 20;
+ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val));
+if (has_settings && ret < 0) {
+error_setg_errno(errp, errno, "Unable to set TCP_KEEPCNT");
+return -1;
+}
+
+return 0;
+}
+
/**
* Create a socket and connect it to an address.
*
@@ -468,16 +519,11 @@ int inet_connect_saddr(InetSocketAddress *saddr, Error
**errp)
return sock;
}
-if (saddr->keep_alive) {
-int val = 1;
-int ret = qemu_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
- &val, sizeof(val));
-
-if (ret < 0) {
-error_setg_errno(errp, errno, "Unable to set KEEPALIVE");
-close(sock);
-return -1;
-}
+if (inet_set_keepalive(sock, saddr->has_keep_alive, saddr->keep_alive,
+ errp) < 0)
+{
+close(sock);
+return -1;
}
return sock;
@@ -677,12 +723,20 @@ int inet_parse(InetSocketAddress *addr, const char *str,
Error **errp)
}
begin = strstr(optstr, ",ke
Re: [PATCH v3 01/18] hw/block/nvme: bump spec data structures to v1.3
Looks good with a small nit (see below),
Reviewed-by: Dmitry Fomichev
>
On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Add missing fields in the Identify Controller and Identify Namespace
> data structures to bring them in line with NVMe v1.3.
>
> This also adds data structures and defines for SGL support which
> requires a couple of trivial changes to the nvme block driver as well.
>
> Signed-off-by: Klaus Jensen
> Acked-by: Fam Zheng
> Reviewed-by: Maxim Levitsky
> ---
> block/nvme.c | 18 ++---
> hw/block/nvme.c | 12 ++--
> include/block/nvme.h | 156 ++-
> 3 files changed, 154 insertions(+), 32 deletions(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index 374e26891573..c1c4c07ac6cc 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -518,7 +518,7 @@ static void nvme_identify(BlockDriverState *bs, int
> namespace, Error **errp)
> error_setg(errp, "Cannot map buffer for DMA");
> goto out;
> }
> -cmd.prp1 = cpu_to_le64(iova);
> +cmd.dptr.prp1 = cpu_to_le64(iova);
>
> if (nvme_cmd_sync(bs, s->queues[0], &cmd)) {
> error_setg(errp, "Failed to identify controller");
> @@ -629,7 +629,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error
> **errp)
> }
> cmd = (NvmeCmd) {
> .opcode = NVME_ADM_CMD_CREATE_CQ,
> -.prp1 = cpu_to_le64(q->cq.iova),
> +.dptr.prp1 = cpu_to_le64(q->cq.iova),
> .cdw10 = cpu_to_le32(((queue_size - 1) << 16) | (n & 0x)),
> .cdw11 = cpu_to_le32(0x3),
> };
> @@ -640,7 +640,7 @@ static bool nvme_add_io_queue(BlockDriverState *bs, Error
> **errp)
> }
> cmd = (NvmeCmd) {
> .opcode = NVME_ADM_CMD_CREATE_SQ,
> -.prp1 = cpu_to_le64(q->sq.iova),
> +.dptr.prp1 = cpu_to_le64(q->sq.iova),
> .cdw10 = cpu_to_le32(((queue_size - 1) << 16) | (n & 0x)),
> .cdw11 = cpu_to_le32(0x1 | (n << 16)),
> };
> @@ -988,16 +988,16 @@ try_map:
> case 0:
> abort();
> case 1:
> -cmd->prp1 = pagelist[0];
> -cmd->prp2 = 0;
> +cmd->dptr.prp1 = pagelist[0];
> +cmd->dptr.prp2 = 0;
> break;
> case 2:
> -cmd->prp1 = pagelist[0];
> -cmd->prp2 = pagelist[1];
> +cmd->dptr.prp1 = pagelist[0];
> +cmd->dptr.prp2 = pagelist[1];
> break;
> default:
> -cmd->prp1 = pagelist[0];
> -cmd->prp2 = cpu_to_le64(req->prp_list_iova + sizeof(uint64_t));
> +cmd->dptr.prp1 = pagelist[0];
> +cmd->dptr.prp2 = cpu_to_le64(req->prp_list_iova + sizeof(uint64_t));
> break;
> }
> trace_nvme_cmd_map_qiov(s, cmd, req, qiov, entries);
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 1aee042d4cb2..71b388aa0e20 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -397,8 +397,8 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns,
> NvmeCmd *cmd,
> NvmeRwCmd *rw = (NvmeRwCmd *)cmd;
> uint32_t nlb = le32_to_cpu(rw->nlb) + 1;
> uint64_t slba = le64_to_cpu(rw->slba);
> -uint64_t prp1 = le64_to_cpu(rw->prp1);
> -uint64_t prp2 = le64_to_cpu(rw->prp2);
> +uint64_t prp1 = le64_to_cpu(rw->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(rw->dptr.prp2);
>
> uint8_t lba_index = NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas);
> uint8_t data_shift = ns->id_ns.lbaf[lba_index].ds;
> @@ -795,8 +795,8 @@ static inline uint64_t nvme_get_timestamp(const NvmeCtrl
> *n)
>
> static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n, NvmeCmd *cmd)
> {
> -uint64_t prp1 = le64_to_cpu(cmd->prp1);
> -uint64_t prp2 = le64_to_cpu(cmd->prp2);
> +uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2);
>
> uint64_t timestamp = nvme_get_timestamp(n);
>
> @@ -834,8 +834,8 @@ static uint16_t nvme_set_feature_timestamp(NvmeCtrl *n,
> NvmeCmd *cmd)
> {
> uint16_t ret;
> uint64_t timestamp;
> -uint64_t prp1 = le64_to_cpu(cmd->prp1);
> -uint64_t prp2 = le64_to_cpu(cmd->prp2);
> +uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2);
>
> ret = nvme_dma_write_prp(n, (uint8_t *)×tamp,
> sizeof(timestamp), prp1, prp2);
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 1720ee1d5158..2a80d2a7ed89 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -377,15 +377,53 @@ enum NvmePmrmscMask {
> #define NVME_PMRMSC_SET_CBA(pmrmsc, val) \
> (pmrmsc |= (uint64_t)(val & PMRMSC_CBA_MASK) << PMRMSC_CBA_SHIFT)
>
> +enum NvmeSglDescriptorType {
> +NVME_SGL_DESCR_TYPE_DATA_BLOCK = 0x0,
> +NVME_SGL_DESCR_TYPE_BIT_BUCKET = 0x1,
> +NVME_SGL_DESCR_TYPE_SEGMENT = 0x2,
> +NVME_SGL_DESCR_TYPE_LAST_SEGMENT= 0x3,
> +NVME_SGL_DESCR_TYPE_KEYED_DATA_BLOCK= 0x4,
>
Re: [PATCH v3 02/18] hw/block/nvme: fix missing endian conversion
Looks good, Reviewed-by: Dmitry Fomichev > On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote: > From: Klaus Jensen > > Fix a missing cpu_to conversion by moving conversion to just before > returning instead. > > Signed-off-by: Klaus Jensen > Suggested-by: Philippe Mathieu-Daudé > --- > hw/block/nvme.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 71b388aa0e20..766cd5b33bb1 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -815,8 +815,8 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd > *cmd, NvmeRequest *req) > trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled"); > break; > case NVME_NUMBER_OF_QUEUES: > -result = cpu_to_le32((n->params.max_ioqpairs - 1) | > - ((n->params.max_ioqpairs - 1) << 16)); > +result = (n->params.max_ioqpairs - 1) | > +((n->params.max_ioqpairs - 1) << 16); > trace_pci_nvme_getfeat_numq(result); > break; > case NVME_TIMESTAMP: > @@ -826,7 +826,7 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd > *cmd, NvmeRequest *req) > return NVME_INVALID_FIELD | NVME_DNR; > } > > -req->cqe.result = result; > +req->cqe.result = cpu_to_le32(result); > return NVME_SUCCESS; > } >
Re: [PATCH v3 03/18] hw/block/nvme: additional tracing
Looks good,
Reviewed-by: Dmitry Fomichev
> On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Add various additional tracing and streamline nvme_identify_ns and
> nvme_identify_nslist (they do not need to repeat the command, it is
> already in the trace name).
>
> Signed-off-by: Klaus Jensen
> ---
> hw/block/nvme.c | 33 +
> hw/block/trace-events | 13 +++--
> 2 files changed, 44 insertions(+), 2 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 766cd5b33bb1..09ef54d771c4 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -69,6 +69,20 @@
>
> static void nvme_process_sq(void *opaque);
>
> +static uint16_t nvme_cid(NvmeRequest *req)
> +{
> +if (!req) {
> +return 0x;
> +}
> +
> +return le16_to_cpu(req->cqe.cid);
> +}
> +
> +static uint16_t nvme_sqid(NvmeRequest *req)
> +{
> +return le16_to_cpu(req->sq->sqid);
> +}
> +
> static bool nvme_addr_is_cmb(NvmeCtrl *n, hwaddr addr)
> {
> hwaddr low = n->ctrl_mem.addr;
> @@ -331,6 +345,8 @@ static void nvme_post_cqes(void *opaque)
> static void nvme_enqueue_req_completion(NvmeCQueue *cq, NvmeRequest *req)
> {
> assert(cq->cqid == req->sq->cqid);
> +trace_pci_nvme_enqueue_req_completion(nvme_cid(req), cq->cqid,
> + req->status);
> QTAILQ_REMOVE(&req->sq->out_req_list, req, entry);
> QTAILQ_INSERT_TAIL(&cq->req_list, req, entry);
> timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
> @@ -343,6 +359,8 @@ static void nvme_rw_cb(void *opaque, int ret)
> NvmeCtrl *n = sq->ctrl;
> NvmeCQueue *cq = n->cq[sq->cqid];
>
> +trace_pci_nvme_rw_cb(nvme_cid(req));
> +
> if (!ret) {
> block_acct_done(blk_get_stats(n->conf.blk), &req->acct);
> req->status = NVME_SUCCESS;
> @@ -378,6 +396,8 @@ static uint16_t nvme_write_zeros(NvmeCtrl *n,
> NvmeNamespace *ns, NvmeCmd *cmd,
> uint64_t offset = slba << data_shift;
> uint32_t count = nlb << data_shift;
>
> +trace_pci_nvme_write_zeroes(nvme_cid(req), slba, nlb);
> +
> if (unlikely(slba + nlb > ns->id_ns.nsze)) {
> trace_pci_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
> return NVME_LBA_RANGE | NVME_DNR;
> @@ -445,6 +465,8 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd,
> NvmeRequest *req)
> NvmeNamespace *ns;
> uint32_t nsid = le32_to_cpu(cmd->nsid);
>
> +trace_pci_nvme_io_cmd(nvme_cid(req), nsid, nvme_sqid(req), cmd->opcode);
> +
> if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
> trace_pci_nvme_err_invalid_ns(nsid, n->num_namespaces);
> return NVME_INVALID_NSID | NVME_DNR;
> @@ -876,6 +898,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd
> *cmd, NvmeRequest *req)
>
> static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> {
> +trace_pci_nvme_admin_cmd(nvme_cid(req), nvme_sqid(req), cmd->opcode);
> +
> switch (cmd->opcode) {
> case NVME_ADM_CMD_DELETE_SQ:
> return nvme_del_sq(n, cmd);
> @@ -1204,6 +1228,8 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr
> addr, unsigned size)
> uint8_t *ptr = (uint8_t *)&n->bar;
> uint64_t val = 0;
>
> +trace_pci_nvme_mmio_read(addr);
> +
> if (unlikely(addr & (sizeof(uint32_t) - 1))) {
> NVME_GUEST_ERR(pci_nvme_ub_mmiord_misaligned32,
> "MMIO read not 32-bit aligned,"
> @@ -1273,6 +1299,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr,
> int val)
> return;
> }
>
> +trace_pci_nvme_mmio_doorbell_cq(cq->cqid, new_head);
> +
> start_sqs = nvme_cq_full(cq) ? 1 : 0;
> cq->head = new_head;
> if (start_sqs) {
> @@ -1311,6 +1339,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr,
> int val)
> return;
> }
>
> +trace_pci_nvme_mmio_doorbell_sq(sq->sqid, new_tail);
> +
> sq->tail = new_tail;
> timer_mod(sq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
> }
> @@ -1320,6 +1350,9 @@ static void nvme_mmio_write(void *opaque, hwaddr addr,
> uint64_t data,
> unsigned size)
> {
> NvmeCtrl *n = (NvmeCtrl *)opaque;
> +
> +trace_pci_nvme_mmio_write(addr, data);
> +
> if (addr < sizeof(n->bar)) {
> nvme_write_bar(n, addr, data, size);
> } else if (addr >= 0x1000) {
> diff --git a/hw/block/trace-events b/hw/block/trace-events
> index 958fcc5508d1..c40c0d2e4b28 100644
> --- a/hw/block/trace-events
> +++ b/hw/block/trace-events
> @@ -33,19 +33,28 @@ pci_nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ
> vector %u"
> pci_nvme_irq_pin(void) "pulsing IRQ pin"
> pci_nvme_irq_masked(void) "IRQ is masked"
> pci_nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64"
> prp2=0x%"PRIx64""
> +pci_nvme_io_cmd(uint16_t cid, uint32_t nsid, uint16_t sqid, ui
Re: [PATCH v3 07/18] hw/block/nvme: add support for the get log page command
Looks good,
Reviewed-by: Dmitry Fomichev
On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Add support for the Get Log Page command and basic implementations of
> the mandatory Error Information, SMART / Health Information and Firmware
> Slot Information log pages.
>
> In violation of the specification, the SMART / Health Information log
> page does not persist information over the lifetime of the controller
> because the device has no place to store such persistent state.
>
> Note that the LPA field in the Identify Controller data structure
> intentionally has bit 0 cleared because there is no namespace specific
> information in the SMART / Health information log page.
>
> Required for compliance with NVMe revision 1.3d. See NVM Express 1.3d,
> Section 5.14 ("Get Log Page command").
>
> Signed-off-by: Klaus Jensen
> Signed-off-by: Klaus Jensen
> Acked-by: Keith Busch
> ---
> hw/block/nvme.c | 140 +-
> hw/block/nvme.h | 2 +
> hw/block/trace-events | 2 +
> include/block/nvme.h | 8 ++-
> 4 files changed, 149 insertions(+), 3 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index b6bc75eb61a2..7cb3787638f6 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -606,6 +606,140 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd
> *cmd)
> return NVME_SUCCESS;
> }
>
> +static uint16_t nvme_smart_info(NvmeCtrl *n, NvmeCmd *cmd, uint32_t buf_len,
> +uint64_t off, NvmeRequest *req)
> +{
> +uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2);
> +uint32_t nsid = le32_to_cpu(cmd->nsid);
> +
> +uint32_t trans_len;
> +time_t current_ms;
> +uint64_t units_read = 0, units_written = 0;
> +uint64_t read_commands = 0, write_commands = 0;
> +NvmeSmartLog smart;
> +BlockAcctStats *s;
> +
> +if (nsid && nsid != 0x) {
> +return NVME_INVALID_FIELD | NVME_DNR;
> +}
> +
> +s = blk_get_stats(n->conf.blk);
> +
> +units_read = s->nr_bytes[BLOCK_ACCT_READ] >> BDRV_SECTOR_BITS;
> +units_written = s->nr_bytes[BLOCK_ACCT_WRITE] >> BDRV_SECTOR_BITS;
> +read_commands = s->nr_ops[BLOCK_ACCT_READ];
> +write_commands = s->nr_ops[BLOCK_ACCT_WRITE];
> +
> +if (off > sizeof(smart)) {
> +return NVME_INVALID_FIELD | NVME_DNR;
> +}
> +
> +trans_len = MIN(sizeof(smart) - off, buf_len);
> +
> +memset(&smart, 0x0, sizeof(smart));
> +
> +smart.data_units_read[0] = cpu_to_le64(units_read / 1000);
> +smart.data_units_written[0] = cpu_to_le64(units_written / 1000);
> +smart.host_read_commands[0] = cpu_to_le64(read_commands);
> +smart.host_write_commands[0] = cpu_to_le64(write_commands);
> +
> +smart.temperature = cpu_to_le16(n->temperature);
> +
> +if ((n->temperature >= n->features.temp_thresh_hi) ||
> +(n->temperature <= n->features.temp_thresh_low)) {
> +smart.critical_warning |= NVME_SMART_TEMPERATURE;
> +}
> +
> +current_ms = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> +smart.power_on_hours[0] =
> +cpu_to_le64current_ms - n->starttime_ms) / 1000) / 60) / 60);
> +
> +return nvme_dma_read_prp(n, (uint8_t *) &smart + off, trans_len, prp1,
> + prp2);
> +}
> +
> +static uint16_t nvme_fw_log_info(NvmeCtrl *n, NvmeCmd *cmd, uint32_t buf_len,
> + uint64_t off, NvmeRequest *req)
> +{
> +uint32_t trans_len;
> +uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2);
> +NvmeFwSlotInfoLog fw_log = {
> +.afi = 0x1,
> +};
> +
> +strpadcpy((char *)&fw_log.frs1, sizeof(fw_log.frs1), "1.0", ' ');
> +
> +if (off > sizeof(fw_log)) {
> +return NVME_INVALID_FIELD | NVME_DNR;
> +}
> +
> +trans_len = MIN(sizeof(fw_log) - off, buf_len);
> +
> +return nvme_dma_read_prp(n, (uint8_t *) &fw_log + off, trans_len, prp1,
> + prp2);
> +}
> +
> +static uint16_t nvme_error_info(NvmeCtrl *n, NvmeCmd *cmd, uint32_t buf_len,
> +uint64_t off, NvmeRequest *req)
> +{
> +uint32_t trans_len;
> +uint64_t prp1 = le64_to_cpu(cmd->dptr.prp1);
> +uint64_t prp2 = le64_to_cpu(cmd->dptr.prp2);
> +NvmeErrorLog errlog;
> +
> +if (off > sizeof(errlog)) {
> +return NVME_INVALID_FIELD | NVME_DNR;
> +}
> +
> +memset(&errlog, 0x0, sizeof(errlog));
> +
> +trans_len = MIN(sizeof(errlog) - off, buf_len);
> +
> +return nvme_dma_read_prp(n, (uint8_t *)&errlog, trans_len, prp1, prp2);
> +}
> +
> +static uint16_t nvme_get_log(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> +{
> +uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> +uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> +uint32_t dw12 = le32_to_cpu(cmd->cdw12);
> +uint32_t dw13 = le32_to_cpu(cmd->cdw13);
> +uint8_t lid = dw1
Re: [PATCH v3 05/18] hw/block/nvme: add temperature threshold feature
Looks good,
Reviewed-by: Dmitry Fomichev
On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> It might seem weird to implement this feature for an emulated device,
> but it is mandatory to support and the feature is useful for testing
> asynchronous event request support, which will be added in a later
> patch.
>
> Signed-off-by: Klaus Jensen
> Acked-by: Keith Busch
> Reviewed-by: Maxim Levitsky
> ---
> hw/block/nvme.c | 48
> hw/block/nvme.h | 1 +
> include/block/nvme.h | 5 -
> 3 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 415d3b036897..a330ccf91620 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -59,6 +59,9 @@
> #define NVME_DB_SIZE 4
> #define NVME_CMB_BIR 2
> #define NVME_PMR_BIR 2
> +#define NVME_TEMPERATURE 0x143
> +#define NVME_TEMPERATURE_WARNING 0x157
> +#define NVME_TEMPERATURE_CRITICAL 0x175
>
> #define NVME_GUEST_ERR(trace, fmt, ...) \
> do { \
> @@ -841,9 +844,31 @@ static uint16_t nvme_get_feature_timestamp(NvmeCtrl *n,
> NvmeCmd *cmd)
> static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
> {
> uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> +uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> uint32_t result;
>
> switch (dw10) {
> +case NVME_TEMPERATURE_THRESHOLD:
> +result = 0;
> +
> +/*
> + * The controller only implements the Composite Temperature sensor,
> so
> + * return 0 for all other sensors.
> + */
> +if (NVME_TEMP_TMPSEL(dw11) != NVME_TEMP_TMPSEL_COMPOSITE) {
> +break;
> +}
> +
> +switch (NVME_TEMP_THSEL(dw11)) {
> +case NVME_TEMP_THSEL_OVER:
> +result = n->features.temp_thresh_hi;
> +break;
> +case NVME_TEMP_THSEL_UNDER:
> +result = n->features.temp_thresh_low;
> +break;
> +}
> +
> +break;
> case NVME_VOLATILE_WRITE_CACHE:
> result = blk_enable_write_cache(n->conf.blk);
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> @@ -888,6 +913,23 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd
> *cmd, NvmeRequest *req)
> uint32_t dw11 = le32_to_cpu(cmd->cdw11);
>
> switch (dw10) {
> +case NVME_TEMPERATURE_THRESHOLD:
> +if (NVME_TEMP_TMPSEL(dw11) != NVME_TEMP_TMPSEL_COMPOSITE) {
> +break;
> +}
> +
> +switch (NVME_TEMP_THSEL(dw11)) {
> +case NVME_TEMP_THSEL_OVER:
> +n->features.temp_thresh_hi = NVME_TEMP_TMPTH(dw11);
> +break;
> +case NVME_TEMP_THSEL_UNDER:
> +n->features.temp_thresh_low = NVME_TEMP_TMPTH(dw11);
> +break;
> +default:
> +return NVME_INVALID_FIELD | NVME_DNR;
> +}
> +
> +break;
> case NVME_VOLATILE_WRITE_CACHE:
> blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
> break;
> @@ -1468,6 +1510,7 @@ static void nvme_init_state(NvmeCtrl *n)
> n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);
> n->sq = g_new0(NvmeSQueue *, n->params.max_ioqpairs + 1);
> n->cq = g_new0(NvmeCQueue *, n->params.max_ioqpairs + 1);
> +n->features.temp_thresh_hi = NVME_TEMPERATURE_WARNING;
> }
>
> static void nvme_init_blk(NvmeCtrl *n, Error **errp)
> @@ -1625,6 +1668,11 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
> id->acl = 3;
> id->frmw = 7 << 1;
> id->lpa = 1 << 0;
> +
> +/* recommended default value (~70 C) */
> +id->wctemp = cpu_to_le16(NVME_TEMPERATURE_WARNING);
> +id->cctemp = cpu_to_le16(NVME_TEMPERATURE_CRITICAL);
> +
> id->sqes = (0x6 << 4) | 0x6;
> id->cqes = (0x4 << 4) | 0x4;
> id->nn = cpu_to_le32(n->num_namespaces);
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 1d30c0bca283..e3a2c907e210 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -107,6 +107,7 @@ typedef struct NvmeCtrl {
> NvmeSQueue admin_sq;
> NvmeCQueue admin_cq;
> NvmeIdCtrl id_ctrl;
> +NvmeFeatureVal features;
> } NvmeCtrl;
>
> /* calculate the number of LBAs that the namespace can accomodate */
> diff --git a/include/block/nvme.h b/include/block/nvme.h
> index 2a80d2a7ed89..d2c457695b38 100644
> --- a/include/block/nvme.h
> +++ b/include/block/nvme.h
> @@ -860,7 +860,10 @@ enum NvmeIdCtrlOncs {
> typedef struct NvmeFeatureVal {
> uint32_tarbitration;
> uint32_tpower_mgmt;
> -uint32_ttemp_thresh;
> +struct {
> +uint16_t temp_thresh_hi;
> +uint16_t temp_thresh_low;
> +};
> uint32_terr_rec;
> uint32_tvolatile_wc;
> uint32_tnum_queues;
Re: build error of unused function as MACRO G_DEFINE_AUTOPTR_CLEANUP_FUNC expand
Hi Li,
I usually build the fuzzer with "make i386-softmmu/fuzz", so I must have
missed the nbd issue... I could not reproduce this locally since:
alxndr@mozz:qemu(master)$ dpkg -l "*glib2.0-bin*"
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name Version Architecture Description
+++-==---=
ii libglib2.0-bin 2.60.6-2 amd64Programs for the GLib library
The issue described in 9bda600b083 only shows up in versions older than
2.57.2...
After some digging, in ./configure:
...
if test "$have_fuzzer" = "yes"; then
FUZZ_LDFLAGS=" -fsanitize=address,fuzzer"
FUZZ_CFLAGS=" -fsanitize=address,fuzzer"
CFLAGS=" -fsanitize=address,fuzzer-no-link"
Thats probably the issue. Should be
CFLAGS="$CFLAGS -fsanitize=address,fuzzer-no-link"
I'm also having trouble building, but for a different reason..
CC=clang-8 CXX=clang++-8 ./configure --target-list="i386-softmmu"
--enable-fuzzing
...
CC i386-softmmu/tests/qtest/fuzz/qtest_wrappers.o
/tmp/qemu/tests/qtest/fuzz/fuzz.c:215:5: error: implicit declaration of
function 'rcu_enable_atfork' is invalid in C99
[-Werror,-Wimplicit-function-declaration]
rcu_enable_atfork();
^
45222b9a9016488289a1938a528239c3b83eddb1 is the first bad commit
commit 45222b9a9016488289a1938a528239c3b83eddb1
Author: Alexander Bulekov
Date: Thu Jun 18 12:05:16 2020 -0400
fuzz: fix broken qtest check at rcu_disable_atfork
Looks like I forgot a header... I'll send fixes for both of these
issues.
Thank you
-Alex
On 200708 1850, Philippe Mathieu-Daudé wrote:
> Cc'ing the fuzzing maintainers.
>
> On 7/8/20 5:41 PM, Li Qiang wrote:
> > Hello all,
> >
> >
> >
> > I build qemu with fuzzing enabled using clang and following error come.
> >
> >
> >
> > nbd/server.c:1937:1: error: unused function
> > 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
> >
> > G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free);
> >
> > ^
> >
> > /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro
> > 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
> >
> > static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l)
> > { g_list_free_full (*_l, (GDestroyNotify) func); } \
> >
> > ^
> >
> > /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro
> > '_GLIB_AUTOPTR_LIST_FUNC_NAME'
> >
> > #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName)
> > glib_listautoptr_cleanup_##TypeName
> >
> > ^
> >
> > :170:1: note: CC crypto/hash-glib.o
> >
> > expanded from here
> >
> > glib_listautoptr_cleanup_NBDExtentArray
> >
> > ^
> >
> > nbd/server.c:1937:1: error: unused function
> > 'glib_slistautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function]
> >
> > /usr/include/glib-2.0/glib/gmacros.h:463:22: note: expanded from macro
> > 'G_DEFINE_AUTOPTR_CLEANUP_FUNC'
> >
> > static inline void _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName) (GSList
> > **_l) { g_slist_free_full (*_l, (GDestroyNotify) func); } \
> >
> > ^
> >
> > /usr/include/glib-2.0/glib/gmacros.h:445:49: note: expanded from macro
> > '_GLIB_AUTOPTR_SLIST_FUNC_NAME'
> >
> > #define _GLIB_AUTOPTR_SLIST_FUNC_NAME(TypeName)
> > glib_slistautoptr_cleanup_##TypeName
> >
> > ^
> >
> > :171:1: note: expanded from here
> >
> > glib_slistautoptr_cleanup_NBDExtentArray
> >
> >
> >
> >
> >
> > I see Eric’s patch 9bda600b083(“build: Silence clang warning on older
> > glib autoptr usage”)
> >
> > So I know there should be a ‘-Wno-unused-function’ in CFLAGS. It is
> > after ./configure:
> >
> >
> >
> > CFLAGS -g -Wno-unused-function
> >
> > QEMU_CFLAGS -I/usr/include/pixman-1 -Werror -pthread
> > -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
> > -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
> > -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef
> > -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common
> > -fwrapv -std=gnu99 -Wno-string-plus-int -Wno-typedef-redefinition
> > -Wno-initializer-overrides -Wexpansion-to-defined -Wendif-labels
> > -Wno-shift-negative-value -Wno-missing-include-dirs -Wempty-body
> > -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self
> > -Wignored-qualifiers -Wold-style-definition -Wtype-limits
> > -fstack-protector-strong -I$(SRC_PATH)/capstone/include
> >
> >
> >
> > However while I ‘make V=1’ I see the build nbd/serer.c using following
> > command:
> >
> > clang-8 -iquote /home/test/qemu/nbd -iquote nbd -iquote
> > /home/test/qemu/tcg/i386 -isystem /home/test/qemu/linux-headers -isystem
> > /home/test/qemu/linux-headers -iquote . -iquote /home/
Re: [PATCH v3 12/18] hw/block/nvme: support the get/set features select and save fields
Looks good,
Reviewed-by: Dmitry Fomichev
On Mon, 2020-07-06 at 08:12 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Since the device does not have any persistent state storage, no
> features are "saveable" and setting the Save (SV) field in any Set
> Features command will result in a Feature Identifier Not Saveable status
> code.
>
> Similarly, if the Select (SEL) field is set to request saved values, the
> devices will (as it should) return the default values instead.
>
> Since this also introduces "Supported Capabilities", the nsid field is
> now also checked for validity wrt. the feature being get/set'ed.
>
> Signed-off-by: Klaus Jensen
> ---
> hw/block/nvme.c | 103 +-
> hw/block/trace-events | 4 +-
> include/block/nvme.h | 27 ++-
> 3 files changed, 119 insertions(+), 15 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 2d85e853403f..df8b786e4875 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -85,6 +85,14 @@ static const bool nvme_feature_support[NVME_FID_MAX] = {
> [NVME_TIMESTAMP]= true,
> };
>
> +static const uint32_t nvme_feature_cap[NVME_FID_MAX] = {
> +[NVME_TEMPERATURE_THRESHOLD]= NVME_FEAT_CAP_CHANGE,
> +[NVME_VOLATILE_WRITE_CACHE] = NVME_FEAT_CAP_CHANGE,
> +[NVME_NUMBER_OF_QUEUES] = NVME_FEAT_CAP_CHANGE,
> +[NVME_ASYNCHRONOUS_EVENT_CONF] = NVME_FEAT_CAP_CHANGE,
> +[NVME_TIMESTAMP]= NVME_FEAT_CAP_CHANGE,
> +};
> +
> static void nvme_process_sq(void *opaque);
>
> static uint16_t nvme_cid(NvmeRequest *req)
> @@ -1083,20 +1091,47 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd
> *cmd, NvmeRequest *req)
> {
> uint32_t dw10 = le32_to_cpu(cmd->cdw10);
> uint32_t dw11 = le32_to_cpu(cmd->cdw11);
> +uint32_t nsid = le32_to_cpu(cmd->nsid);
> uint32_t result;
> uint8_t fid = NVME_GETSETFEAT_FID(dw10);
> +NvmeGetFeatureSelect sel = NVME_GETFEAT_SELECT(dw10);
> uint16_t iv;
>
> static const uint32_t nvme_feature_default[NVME_FID_MAX] = {
> [NVME_ARBITRATION] = NVME_ARB_AB_NOLIMIT,
> };
>
> -trace_pci_nvme_getfeat(nvme_cid(req), fid, dw11);
> +trace_pci_nvme_getfeat(nvme_cid(req), fid, sel, dw11);
>
> if (!nvme_feature_support[fid]) {
> return NVME_INVALID_FIELD | NVME_DNR;
> }
>
> +if (nvme_feature_cap[fid] & NVME_FEAT_CAP_NS) {
> +if (!nsid || nsid > n->num_namespaces) {
> +/*
> + * The Reservation Notification Mask and Reservation Persistence
> + * features require a status code of Invalid Field in Command
> when
> + * NSID is 0x. Since the device does not support those
> + * features we can always return Invalid Namespace or Format as
> we
> + * should do for all other features.
> + */
> +return NVME_INVALID_NSID | NVME_DNR;
> +}
> +}
> +
> +switch (sel) {
> +case NVME_GETFEAT_SELECT_CURRENT:
> +break;
> +case NVME_GETFEAT_SELECT_SAVED:
> +/* no features are saveable by the controller; fallthrough */
> +case NVME_GETFEAT_SELECT_DEFAULT:
> +goto defaults;
> +case NVME_GETFEAT_SELECT_CAP:
> +result = nvme_feature_cap[fid];
> +goto out;
> +}
> +
> switch (fid) {
> case NVME_TEMPERATURE_THRESHOLD:
> result = 0;
> @@ -1106,22 +1141,45 @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd
> *cmd, NvmeRequest *req)
> * return 0 for all other sensors.
> */
> if (NVME_TEMP_TMPSEL(dw11) != NVME_TEMP_TMPSEL_COMPOSITE) {
> -break;
> +goto out;
> }
>
> switch (NVME_TEMP_THSEL(dw11)) {
> case NVME_TEMP_THSEL_OVER:
> result = n->features.temp_thresh_hi;
> -break;
> +goto out;
> case NVME_TEMP_THSEL_UNDER:
> result = n->features.temp_thresh_low;
> -break;
> +goto out;
> }
>
> -break;
> +return NVME_INVALID_FIELD | NVME_DNR;
> case NVME_VOLATILE_WRITE_CACHE:
> result = blk_enable_write_cache(n->conf.blk);
> trace_pci_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
> +goto out;
> +case NVME_ASYNCHRONOUS_EVENT_CONF:
> +result = n->features.async_config;
> +goto out;
> +case NVME_TIMESTAMP:
> +return nvme_get_feature_timestamp(n, cmd);
> +default:
> +break;
> +}
> +
> +defaults:
> +switch (fid) {
> +case NVME_TEMPERATURE_THRESHOLD:
> +result = 0;
> +
> +if (NVME_TEMP_TMPSEL(dw11) != NVME_TEMP_TMPSEL_COMPOSITE) {
> +break;
> +}
> +
> +if (NVME_TEMP_THSEL(dw11) == NVME_TEMP_THSEL_OVER) {
> +result = NVME_TEMPERATURE_WARNING;
> +}
> +
> break;
> case NVME_NUMBER_OF_QUEUES:
Re: [PATCH v3 17/18] hw/block/nvme: provide the mandatory subnqn field
Looks good,
Reviewed-by: Dmitry Fomichev
On Mon, 2020-07-06 at 08:13 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> The SUBNQN field is mandatory in NVM Express 1.3.
>
> Signed-off-by: Klaus Jensen
> ---
> hw/block/nvme.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 07d58aa945f2..e3984157926b 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -2141,6 +2141,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
> {
> NvmeIdCtrl *id = &n->id_ctrl;
> uint8_t *pci_conf = pci_dev->config;
> +char *subnqn;
>
> id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
> id->ssvid = cpu_to_le16(pci_get_word(pci_conf +
> PCI_SUBSYSTEM_VENDOR_ID));
> @@ -2179,6 +2180,10 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice
> *pci_dev)
> id->oncs = cpu_to_le16(NVME_ONCS_WRITE_ZEROS | NVME_ONCS_TIMESTAMP |
> NVME_ONCS_FEATURES);
>
> +subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
> +strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
> +g_free(subnqn);
> +
> id->psd[0].mp = cpu_to_le16(0x9c4);
> id->psd[0].enlat = cpu_to_le32(0x10);
> id->psd[0].exlat = cpu_to_le32(0x4);
Re: [PATCH v3 15/18] hw/block/nvme: reject invalid nsid values in active namespace id list
Looks good,
Reviewed-by: Dmitry Fomichev
On Mon, 2020-07-06 at 08:13 +0200, Klaus Jensen wrote:
> From: Klaus Jensen
>
> Reject the nsid broadcast value (0x) and 0xfffe in the
> Active Namespace ID list.
>
> Signed-off-by: Klaus Jensen
> ---
> hw/block/nvme.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index fc58f3d76530..af39126cd8d1 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -992,6 +992,16 @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n,
> NvmeIdentify *c)
>
> trace_pci_nvme_identify_nslist(min_nsid);
>
> +/*
> + * Both 0x (NVME_NSID_BROADCAST) and 0xfffe are invalid
> values
> + * since the Active Namespace ID List should return namespaces with ids
> + * *higher* than the NSID specified in the command. This is also
> specified
> + * in the spec (NVM Express v1.3d, Section 5.15.4).
> + */
> +if (min_nsid >= NVME_NSID_BROADCAST - 1) {
> +return NVME_INVALID_NSID | NVME_DNR;
> +}
> +
> list = g_malloc0(data_len);
> for (i = 0; i < n->num_namespaces; i++) {
> if (i < min_nsid) {
[PATCH 0/2] keepalive default
Hi all! We understood, that keepalive is almost superfluous with default 2 hours in /proc/tcp_keepalive_time. Forcing user to setup keepalive for the whole system doesn't seem right, better setup it per-socket. These series suggests to set some defaults for keepalive settings as well as set keepalive itself by default. keepalive helps to not hang on io other side is down. Vladimir Sementsov-Ogievskiy (2): sockets: keep-alive settings util/qemu-sockets: make keep-alive enabled by default qapi/sockets.json | 33 +- util/qemu-sockets.c | 81 +++-- 2 files changed, 102 insertions(+), 12 deletions(-) -- 2.21.0
Re: [PATCH 0/2] keepalive default
Patchew URL: https://patchew.org/QEMU/20200708191540.28455-1-vsement...@virtuozzo.com/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === CC hw/hppa/trace.o In file included from /tmp/qemu-test/src/util/qemu-sockets.c:24: /tmp/qemu-test/src/util/qemu-sockets.c: In function 'inet_set_keepalive': /tmp/qemu-test/src/util/qemu-sockets.c:469:46: error: 'TCP_KEEPIDLE' undeclared (first use in this function) 469 | ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)); | ^~~~ /tmp/qemu-test/src/include/qemu-common.h:48:31: note: in definition of macro 'qemu_setsockopt' --- /tmp/qemu-test/src/include/qemu-common.h:48:31: note: in definition of macro 'qemu_setsockopt' 48 | setsockopt(sockfd, level, optname, (const void *)optval, optlen) | ^~~ /tmp/qemu-test/src/util/qemu-sockets.c:476:46: error: 'TCP_KEEPINTVL' undeclared (first use in this function) 476 | ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)); | ^ /tmp/qemu-test/src/include/qemu-common.h:48:31: note: in definition of macro 'qemu_setsockopt' 48 | setsockopt(sockfd, level, optname, (const void *)optval, optlen) | ^~~ /tmp/qemu-test/src/util/qemu-sockets.c:483:46: error: 'TCP_KEEPCNT' undeclared (first use in this function) 483 | ret = qemu_setsockopt(sock, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val)); | ^~~ /tmp/qemu-test/src/include/qemu-common.h:48:31: note: in definition of macro 'qemu_setsockopt' 48 | setsockopt(sockfd, level, optname, (const void *)optval, optlen) | ^~~ make: *** [/tmp/qemu-test/src/rules.mak:69: util/qemu-sockets.o] Error 1 make: *** Waiting for unfinished jobs Traceback (most recent call last): File "./tests/docker/docker.py", line 669, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=bab255245d94432aabefeb4189c6aced', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-mck04b5u/src/docker-src.2020-07-08-15.38.08.17376:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=bab255245d94432aabefeb4189c6aced make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-mck04b5u/src' make: *** [docker-run-test-mingw@fedora] Error 2 real3m11.264s user0m8.976s The full log is available at http://patchew.org/logs/20200708191540.28455-1-vsement...@virtuozzo.com/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v7 00/47] block: Deal with filters
On 08.07.2020 20:32, Eric Blake wrote: On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" Arrgh. If I'm reading bash's changelog correctly, readarray -d was introduced in bash 4.4, so I'm guessing you're still on 4.3 or earlier? What bash version and platform are you using? My bash version is 4.2.46. It is the latest in the virtuozzolinux-base repository. I should install the 4.4 package manually. Thank you Eric for your hint! Andrey introduced with the commit a7399eb iotests: Make _filter_img_create more active Andrey
[Bug 1886811] Re: systemd complains Failed to enqueue loopback interface start request: Operation not supported
It seems systemd is trying to use RTM_SETLINK.
Could you try this patch:
diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
index c0687c52e62b..b09b5b7c13e0 100644
--- a/linux-user/fd-trans.c
+++ b/linux-user/fd-trans.c
@@ -1200,6 +1200,7 @@ static abi_long target_to_host_data_route(struct nlmsghdr
*nlh)
break;
case RTM_NEWLINK:
case RTM_DELLINK:
+case RTM_SETLINK:
if (nlh->nlmsg_len >= NLMSG_LENGTH(sizeof(*ifi))) {
ifi = NLMSG_DATA(nlh);
ifi->ifi_type = tswap16(ifi->ifi_type);
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1886811
Title:
systemd complains Failed to enqueue loopback interface start request:
Operation not supported
Status in QEMU:
New
Status in qemu package in Debian:
Unknown
Bug description:
This symptom seems similar to
https://bugs.launchpad.net/qemu/+bug/1823790
Host Linux: Debian 11 Bullseye (testing) on x84-64 architecture
qemu version: latest git of git commit hash
eb2c66b10efd2b914b56b20ae90655914310c925
compiled with "./configure --static --disable-system"
Down stream bug report at
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=964289
Bug report (closed) to systemd:
https://github.com/systemd/systemd/issues/16359
systemd in armhf and armel (both little endian 32-bit) containers fail to
start with
Failed to enqueue loopback interface start request: Operation not supported
How to reproduce on Debian (and probably Ubuntu):
mmdebstrap --components="main contrib non-free" --architectures=armhf
--variant=important bullseye /var/lib/machines/armhf-bullseye
systemd-nspawn -D /var/lib/machines/armhf-bullseye -b
When "armhf" architecture is replaced with "mips" (32-bit big endian) or
"ppc64"
(64-bit big endian), the container starts up fine.
The same symptom is also observed with "powerpc" (32-bit big endian)
architecture.
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1886811/+subscriptions
