Re: [Qemu-devel] [PATCH v6 01/10] machine: export register_compat_prop()
On Tue, Jun 27, 2017 at 10:05:54AM -0500, Eric Blake wrote: > On 06/27/2017 09:55 AM, Eduardo Habkost wrote: > > >> + * > >> + * The property values set using this function must be always valid and > >> + * never report setter errors, as they property have > > > > Typo (my fault, sorry). It should be "the property have", or "the > > property will have". > > > > "the property will have" Thanks Eduardo & Eric for reviewing. I suppose this series will go through Juan's tree if finally proper? Anyway, please anyone let me know if we need a repost. > > > With this fixed (which may be done manually by the maintainer when > > applying): > > > > Reviewed-by: Eduardo Habkost -- Peter Xu
Re: [Qemu-devel] [PATCH v6 06/10] migration: move only_migratable to MigrationState
On Tue, Jun 27, 2017 at 10:36:44AM -0300, Eduardo Habkost wrote: > On Tue, Jun 27, 2017 at 06:15:37AM -0500, Eric Blake wrote: > > On 06/26/2017 11:10 PM, Peter Xu wrote: > > > One less global variable, and it does only matter with migration. > > > > > > We keep the old "--only-migratable" option, but also now we support: > > > > > > -global migration.only-migratable=true > > > > Does command line introspection work to see that this new spelling is > > available? If not, what else can we do to let libvirt learn that the > > new form is preferred? > > "only-migratable" will appear on the output of > "device-list-properties typename=migration". But although I think > implementing this using global properties internally is nice, I'm unsure > about making -global the recommended interface to configure this. Could I ask why it may not good to use "-global" to configure it? Actually if we can merge this series, there is planned work to expose more migration parameters to the user, like: migration.postcopy, migration.compression, migration.speed, etc. So before I post similar patches like this to export more things, I would like to know whether there is possible issue with it in general. Thanks, -- Peter Xu
Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning
On Tue, Jun 27, 2017 at 05:17:58PM -0700, Alistair Francis wrote: > On Tue, Jun 27, 2017 at 2:10 PM, Edgar E. Iglesias > wrote: > > On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: > >> When QEMU is waiting for a TCP socket connection it reports that message as > >> an error. This isn't an error it is a warnign so let's change the report to > >> use warning_report() instead. > > > > Hi Alistair, > > > > Isn't this more like an informational message rather than a warning? > > > > A warning would indicate to me that something unexpected and perhaps wrong > > happened. > > In this case, there's nothing wrong going on. > > > > We may need more classes, like 'info:' and/or maybe others... > > Hey Edgar, > > That is a good point. I can add a info_report() as well then that > copies the warning_report() function but prefixes with 'info: '. Hi, Yes, or another way is to pass the class of message as an argument, e.g like qemu_log_mask does it. Best regards, Edgar > > Thanks, > Alistair > > > > > Cheers, > > Edgar > > > > > > > >> > >> Signed-off-by: Alistair Francis > >> --- > >> > >> chardev/char-socket.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c > >> index ccc499cfa1..5a56628e74 100644 > >> --- a/chardev/char-socket.c > >> +++ b/chardev/char-socket.c > >> @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error > >> **errp) > >> * in TLS and telnet cases, only wait for an accepted socket */ > >> while (!s->ioc) { > >> if (s->is_listen) { > >> -error_report("QEMU waiting for connection on: %s", > >> +warning_report("QEMU waiting for connection on: %s", > >> chr->filename); > >> qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, > >> NULL); > >> tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); > >> -- > >> 2.11.0 > >> > >>
Re: [Qemu-devel] [PATCH 00/10] Block layer thread-safety, part 2
Hi, This series failed build test on s390x host. Please find the details below. Message-id: 20170627152248.10446-1-pbonz...@redhat.com Subject: [Qemu-devel] [PATCH 00/10] Block layer thread-safety, part 2 Type: series === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo "=== ENV ===" env echo "=== PACKAGES ===" rpm -qa echo "=== TEST BEGIN ===" CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build echo -n "Using CC: " realpath $CC mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/1498629200-35463-1-git-send-email-wei.w.w...@intel.com -> patchew/1498629200-35463-1-git-send-email-wei.w.w...@intel.com Switched to a new branch 'test' 6629a48 ssh: support I/O from any AioContext 4eb83dc sheepdog: add queue_lock e34f4a1 qed: protect table cache with CoMutex 5875314 block: invoke .bdrv_drain callback in coroutine context and from AioContext e965a1d qed: move tail of qed_aio_write_main to qed_aio_write_{cow, alloc} f263535 vvfat: make it thread-safe 298a2ea vpc: make it thread-safe e524d42 vdi: make it thread-safe 104e601 coroutine-lock: add qemu_co_rwlock_downgrade and qemu_co_rwlock_upgrade 7980c13 qcow2: call CoQueue APIs under CoMutex === OUTPUT BEGIN === === ENV === XDG_SESSION_ID=123106 SHELL=/bin/sh USER=fam PATCHEW=/home/fam/patchew/patchew-cli -s http://patchew.org --nodebug PATH=/usr/bin:/bin PWD=/var/tmp/patchew-tester-tmp-wwed8rs5/src LANG=en_US.UTF-8 HOME=/home/fam SHLVL=2 LOGNAME=fam DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1012/bus XDG_RUNTIME_DIR=/run/user/1012 _=/usr/bin/env === PACKAGES === gpg-pubkey-873529b8-54e386ff xz-libs-5.2.2-2.fc24.s390x libxshmfence-1.2-3.fc24.s390x giflib-4.1.6-15.fc24.s390x trousers-lib-0.3.13-6.fc24.s390x ncurses-base-6.0-6.20160709.fc25.noarch gmp-6.1.1-1.fc25.s390x libidn-1.33-1.fc25.s390x slang-2.3.0-7.fc25.s390x libsemanage-2.5-8.fc25.s390x pkgconfig-0.29.1-1.fc25.s390x alsa-lib-1.1.1-2.fc25.s390x yum-metadata-parser-1.1.4-17.fc25.s390x python3-slip-dbus-0.6.4-4.fc25.noarch python2-cssselect-0.9.2-1.fc25.noarch python-fedora-0.8.0-2.fc25.noarch createrepo_c-libs-0.10.0-6.fc25.s390x initscripts-9.69-1.fc25.s390x wget-1.18-2.fc25.s390x dhcp-client-4.3.5-1.fc25.s390x parted-3.2-21.fc25.s390x flex-2.6.0-3.fc25.s390x colord-libs-1.3.4-1.fc25.s390x python-osbs-client-0.33-3.fc25.noarch perl-Pod-Simple-3.35-1.fc25.noarch python2-simplejson-3.10.0-1.fc25.s390x brltty-5.4-2.fc25.s390x librados2-10.2.4-2.fc25.s390x tcp_wrappers-7.6-83.fc25.s390x libcephfs_jni1-10.2.4-2.fc25.s390x nettle-devel-3.3-1.fc25.s390x bzip2-devel-1.0.6-21.fc25.s390x libuuid-2.28.2-2.fc25.s390x pango-1.40.4-1.fc25.s390x python3-dnf-1.1.10-6.fc25.noarch cryptsetup-libs-1.7.4-1.fc25.s390x texlive-kpathsea-doc-svn41139-33.fc25.1.noarch netpbm-10.77.00-3.fc25.s390x openssh-7.4p1-4.fc25.s390x texlive-kpathsea-bin-svn40473-33.20160520.fc25.1.s390x texlive-graphics-svn41015-33.fc25.1.noarch texlive-dvipdfmx-def-svn40328-33.fc25.1.noarch texlive-mfware-svn40768-33.fc25.1.noarch texlive-texlive-scripts-svn41433-33.fc25.1.noarch texlive-euro-svn22191.1.1-33.fc25.1.noarch texlive-etex-svn37057.0-33.fc25.1.noarch texlive-iftex-svn29654.0.2-33.fc25.1.noarch texlive-palatino-svn31835.0-33.fc25.1.noarch texlive-texlive-docindex-svn41430-33.fc25.1.noarch texlive-xunicode-svn30466.0.981-33.fc25.1.noarch texlive-koma-script-svn41508-33.fc25.1.noarch texlive-pst-grad-svn15878.1.06-33.fc25.1.noarch texlive-pst-blur-svn15878.2.0-33.fc25.1.noarch texlive-jknapltx-svn19440.0-33.fc25.1.noarch netpbm-progs-10.77.00-3.fc25.s390x texinfo-6.1-4.fc25.s390x openssl-devel-1.0.2k-1.fc25.s390x python2-sssdconfig-1.15.2-1.fc25.noarch gdk-pixbuf2-2.36.6-1.fc25.s390x mesa-libEGL-13.0.4-3.fc25.s390x pcre-cpp-8.40-6.fc25.s390x pcre-utf16-8.40-6.fc25.s390x glusterfs-extra-xlators-3.10.1-1.fc25.s390x mesa-libGL-devel-13.0.4-3.fc25.s390x nss-devel-3.29.3-1.1.fc25.s390x libaio-0.3.110-6.fc24.s390x libfontenc-1.1.3-3.fc24.s390x lzo-2.08-8.fc24.s390x isl-0.14-5.fc24.s390x libXau-1.0.8-6.fc24.s390x linux-atm-libs-2.5.1-14.fc24.s390x libXext-1.3.3-4.fc24.s390x libXxf86vm-1.1.4-3.fc24.s390x bison-3.0.4-4.fc24.s390x perl-srpm-macros-1-20.fc25.noarch gawk-4.1.3-8.fc25.s390x libwayland-client-1.12.0-1.fc25.s390x perl-Exporter-5.72-366.fc25.noarch perl-version-0.99.17-1.fc25.s390x fftw-libs-double-3.3.5-3.fc25.s390x libssh2-1.8.0-1.fc25.s390x ModemManager-glib-1.6.4-1.fc25.s390x newt-python3-0.52.19-2.fc25.s390x python-munch-2.0.4-3.fc25.noarch python-bugzilla-1.2.2-4.fc25.noarch libedit-3.1-16.20160618cvs.fc25.s390x python-pycurl-7.43.0-4.fc25.s390x createrepo_c-0.10.0-6.fc25.s390x device-mapper-multipath-libs-0.4.9-83.fc25.s390x yum-3.4.3-510.
Re: [Qemu-devel] [RFC 5/5] vifo: introduce new VFIO ioctl VFIO_DEVICE_PCI_GET_DIRTY_BITMAP
> From: Alex Williamson [mailto:alex.william...@redhat.com] > Sent: Wednesday, June 28, 2017 3:45 AM > > On Tue, 27 Jun 2017 08:56:01 + > "Zhang, Yulei" wrote: > > > > -Original Message- > > > From: Qemu-devel [mailto:qemu-devel- > > > bounces+yulei.zhang=intel@nongnu.org] On Behalf Of Alex > Williamson > > > Sent: Tuesday, June 27, 2017 4:19 AM > > > To: Zhang, Yulei > > > Cc: Tian, Kevin ; joonas.lahti...@linux.intel.com; > > > qemu-devel@nongnu.org; zhen...@linux.intel.com; Zheng, Xiao > > > ; Wang, Zhi A > > > Subject: Re: [Qemu-devel] [RFC 5/5] vifo: introduce new VFIO ioctl > > > VFIO_DEVICE_PCI_GET_DIRTY_BITMAP > > > > > > On Tue, 4 Apr 2017 18:28:04 +0800 > > > Yulei Zhang wrote: > > > > > > > New VFIO ioctl VFIO_DEVICE_PCI_GET_DIRTY_BITMAP is used to sync > the > > > > pci device dirty pages during the migration. > > > > > > If this needs to exist, it needs a lot more documentation. Why is this > > > a PCI specific device ioctl? Couldn't any vfio device need this? > > > > > > > Signed-off-by: Yulei Zhang > > > > --- > > > > hw/vfio/pci.c | 32 > > > > hw/vfio/pci.h | 2 ++ > > > > linux-headers/linux/vfio.h | 14 ++ > > > > 3 files changed, 48 insertions(+) > > > > > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > > > > index 833cd90..64c851f 100644 > > > > --- a/hw/vfio/pci.c > > > > +++ b/hw/vfio/pci.c > > > > @@ -32,6 +32,7 @@ > > > > #include "pci.h" > > > > #include "trace.h" > > > > #include "qapi/error.h" > > > > +#include "exec/ram_addr.h" > > > > > > > > #define MSIX_CAP_LENGTH 12 > > > > > > > > @@ -39,6 +40,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice > > > *vdev); > > > > static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool > enabled); > > > > static VMStateDescription vfio_pci_vmstate; > > > > static void vfio_vm_change_state_handler(void *pv, int running, > RunState > > > state); > > > > +static void vfio_log_sync(MemoryListener *listener, > > > MemoryRegionSection *section); > > > > > > > > /* > > > > * Disabling BAR mmaping can be slow, but toggling it around INTx can > > > > @@ -2869,6 +2871,11 @@ static void vfio_realize(PCIDevice *pdev, > Error > > > **errp) > > > > vfio_setup_resetfn_quirk(vdev); > > > > > qemu_add_vm_change_state_handler(vfio_vm_change_state_handler, > > > vdev); > > > > > > > > +vdev->vfio_memory_listener = (MemoryListener) { > > > > + .log_sync = vfio_log_sync, > > > > +}; > > > > +memory_listener_register(&vdev->vfio_memory_listener, > > > &address_space_memory); > > > > + > > > > return; > > > > > > > > out_teardown: > > > > @@ -2964,6 +2971,7 @@ static void > vfio_vm_change_state_handler(void > > > *pv, int running, RunState state) > > > > if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_STATUS_SET, > vfio_status)) > > > { > > > > error_report("vfio: Failed to %s device\n", running ? "start" : > "stop"); > > > > } > > > > +vdev->device_stop = running ? false : true; > > > > g_free(vfio_status); > > > > } > > > > > > > > @@ -3079,6 +3087,30 @@ static int vfio_device_get(QEMUFile *f, void > *pv, > > > size_t size, VMStateField *fie > > > > return 0; > > > > } > > > > > > > > +static void vfio_log_sync(MemoryListener *listener, > > > MemoryRegionSection *section) > > > > +{ > > > > +VFIOPCIDevice *vdev = container_of(listener, struct VFIOPCIDevice, > > > vfio_memory_listener); > > > > + > > > > +if (vdev->device_stop) { > > > > +struct vfio_pci_get_dirty_bitmap *d; > > > > +ram_addr_t size = int128_get64(section->size); > > > > +unsigned long page_nr = size >> TARGET_PAGE_BITS; > > > > +unsigned long bitmap_size = (BITS_TO_LONGS(page_nr) + 1) * > > > sizeof(unsigned long); > > > > +d = g_malloc0(sizeof(*d) + bitmap_size); > > > > +d->start_addr = section->offset_within_address_space; > > > > +d->page_nr = page_nr; > > > > + > > > > +if (ioctl(vdev->vbasedev.fd, > VFIO_DEVICE_PCI_GET_DIRTY_BITMAP, d)) > > > { > > > > +error_report("vfio: Failed to fetch dirty pages for > > > > migration\n"); > > > > +goto exit; > > > > +} > > > > +cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d- > > > >dirty_bitmap, d->start_addr, d->page_nr); > > > > + > > > > +exit: > > > > +g_free(d); > > > > +} > > > > +} > > > > + > > > > static void vfio_instance_init(Object *obj) > > > > { > > > > PCIDevice *pci_dev = PCI_DEVICE(obj); > > > > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > > > > index bd98618..984391d 100644 > > > > --- a/hw/vfio/pci.h > > > > +++ b/hw/vfio/pci.h > > > > @@ -144,6 +144,8 @@ typedef struct VFIOPCIDevice { > > > > bool no_kvm_intx; > > > > bool no_kvm_msi; > > > > bool no_kvm_msix; > > > > +bool device_stop; > > > > +MemoryListener vfio_memory_listener; > > > > } VFIOPCIDevice; > > > > > > > > uint32_t v
Re: [Qemu-devel] [Qemu devel v5 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit.
Hi Alistair, On Tue, Jun 27, 2017 at 4:19 AM, Alistair Francis wrote: > On Mon, Jun 26, 2017 at 9:01 AM, sundeep subbaraya > wrote: > > Hi Alistair, > > > > On Wed, May 31, 2017 at 4:02 AM, Alistair Francis > > wrote: > >> > >> On Sun, May 28, 2017 at 10:26 PM, sundeep subbaraya > >> wrote: > >> > Hi Alistair, > >> > > >> > On Sat, May 27, 2017 at 5:30 AM, Alistair Francis < > alistai...@gmail.com> > >> > wrote: > >> >> > >> >> On Tue, May 16, 2017 at 8:38 AM, Subbaraya Sundeep > >> >> wrote: > >> >> > Emulated Emcraft's Smartfusion2 System On Module starter > >> >> > kit. > >> >> > > >> >> > Signed-off-by: Subbaraya Sundeep > >> >> > --- > >> >> > hw/arm/Makefile.objs | 1 + > >> >> > hw/arm/msf2-som.c| 89 > >> >> > > >> >> > 2 files changed, 90 insertions(+) > >> >> > create mode 100644 hw/arm/msf2-som.c > >> >> > > >> >> > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > >> >> > index c828061..4b02093 100644 > >> >> > --- a/hw/arm/Makefile.objs > >> >> > +++ b/hw/arm/Makefile.objs > >> >> > @@ -5,6 +5,7 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o > >> >> > stellaris.o > >> >> > obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o > >> >> > obj-$(CONFIG_ACPI) += virt-acpi-build.o > >> >> > obj-y += netduino2.o > >> >> > +obj-y += msf2-som.o > >> >> > >> >> This should be obj-$(CONFIG_MSF2). > >> > > >> > > >> > Ok will change it. > >> >> > >> >> > >> >> > obj-y += sysbus-fdt.o > >> >> > > >> >> > obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o > >> >> > diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c > >> >> > new file mode 100644 > >> >> > index 000..cd2b759 > >> >> > --- /dev/null > >> >> > +++ b/hw/arm/msf2-som.c > >> >> > @@ -0,0 +1,89 @@ > >> >> > +/* > >> >> > + * SmartFusion2 SOM starter kit(from Emcraft) emulation. > >> >> > + * > >> >> > + * Copyright (c) 2017 Subbaraya Sundeep > >> >> > + * > >> >> > + * Permission is hereby granted, free of charge, to any person > >> >> > obtaining a copy > >> >> > + * of this software and associated documentation files (the > >> >> > "Software"), to deal > >> >> > + * in the Software without restriction, including without > limitation > >> >> > the rights > >> >> > + * to use, copy, modify, merge, publish, distribute, sublicense, > >> >> > and/or > >> >> > sell > >> >> > + * copies of the Software, and to permit persons to whom the > >> >> > Software > >> >> > is > >> >> > + * furnished to do so, subject to the following conditions: > >> >> > + * > >> >> > + * The above copyright notice and this permission notice shall be > >> >> > included in > >> >> > + * all copies or substantial portions of the Software. > >> >> > + * > >> >> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > >> >> > EXPRESS OR > >> >> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > >> >> > MERCHANTABILITY, > >> >> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > EVENT > >> >> > SHALL > >> >> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > DAMAGES > >> >> > OR > >> >> > OTHER > >> >> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > >> >> > ARISING FROM, > >> >> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > >> >> > DEALINGS IN > >> >> > + * THE SOFTWARE. > >> >> > + */ > >> >> > + > >> >> > +#include "qemu/osdep.h" > >> >> > +#include "qapi/error.h" > >> >> > +#include "hw/boards.h" > >> >> > +#include "hw/arm/msf2-soc.h" > >> >> > +#include "hw/arm/arm.h" > >> >> > +#include "exec/address-spaces.h" > >> >> > + > >> >> > +#define DDR_BASE_ADDRESS 0xA000 > >> >> > +#define DDR_SIZE (64 * M_BYTE) > >> >> > + > >> >> > +static void emcraft_sf2_init(MachineState *machine) > >> >> > +{ > >> >> > +DeviceState *dev; > >> >> > +DeviceState *spi_flash; > >> >> > +MSF2State *soc; > >> >> > +DriveInfo *dinfo = drive_get_next(IF_MTD); > >> >> > +qemu_irq cs_line; > >> >> > +SSIBus *spi_bus; > >> >> > +MemoryRegion *sysmem = get_system_memory(); > >> >> > +MemoryRegion *ddr = g_new(MemoryRegion, 1); > >> >> > + > >> >> > +memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE, > >> >> > + &error_fatal); > >> >> > +vmstate_register_ram_global(ddr); > >> >> > +memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr); > >> >> > >> >> The user can use -m to specify the amount of RAM to create in the > >> >> machine. Unless this board only ever includes 64MB of RAM you should > >> >> use that option (you will need to sanity check it though). If the > >> >> board only ever has 64MB it might be worth printing a warning to the > >> >> user if they specify an something. Although there might be a default > >> >> if they don't use -m, which makes it hard to print out a warning > >> >> message. > >> > > >> > > >> > This -m confuses me. Why is it necessary for an embedded board
[Qemu-devel] [PATCH v1] virtio-net: code cleanup
Use is_power_of_2(), and remove trailing "." from error_setg(). Signed-off-by: Wei Wang --- hw/net/virtio-net.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 91eddaf..144169c 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -1918,9 +1918,9 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) */ if (n->net_conf.rx_queue_size < VIRTIO_NET_RX_QUEUE_MIN_SIZE || n->net_conf.rx_queue_size > VIRTQUEUE_MAX_SIZE || -(n->net_conf.rx_queue_size & (n->net_conf.rx_queue_size - 1))) { +!is_power_of_2(n->net_conf.rx_queue_size)) { error_setg(errp, "Invalid rx_queue_size (= %" PRIu16 "), " - "must be a power of 2 between %d and %d.", + "must be a power of 2 between %d and %d", n->net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_MIN_SIZE, VIRTQUEUE_MAX_SIZE); virtio_cleanup(vdev); @@ -1930,7 +1930,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->max_queues = MAX(n->nic_conf.peers.queues, 1); if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) { error_setg(errp, "Invalid number of queues (= %" PRIu32 "), " - "must be a positive integer less than %d.", + "must be a positive integer less than %d", n->max_queues, (VIRTIO_QUEUE_MAX - 1) / 2); virtio_cleanup(vdev); return; -- 2.7.4
Re: [Qemu-devel] [PATCH v5 3/4] net/net: Convert parse_host_port() to Error
Mao Zhongyi writes: > Hi, Markus > > On 06/27/2017 04:04 PM, Markus Armbruster wrote: >> Mao Zhongyi writes: >> >>> Cc: berra...@redhat.com >>> Cc: kra...@redhat.com >>> Cc: pbonz...@redhat.com >>> Cc: jasow...@redhat.com >>> Cc: arm...@redhat.com >>> Signed-off-by: Mao Zhongyi >>> --- >>> include/qemu/sockets.h | 3 ++- >>> net/net.c | 22 +- >>> net/socket.c | 19 ++- >>> 3 files changed, 33 insertions(+), 11 deletions(-) >>> >>> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h >>> index 5c326db..78e2b30 100644 >>> --- a/include/qemu/sockets.h >>> +++ b/include/qemu/sockets.h >>> @@ -53,7 +53,8 @@ void socket_listen_cleanup(int fd, Error **errp); >>> int socket_dgram(SocketAddress *remote, SocketAddress *local, Error >>> **errp); >>> >>> /* Old, ipv4 only bits. Don't use for new code. */ >>> -int parse_host_port(struct sockaddr_in *saddr, const char *str); >>> +int parse_host_port(struct sockaddr_in *saddr, const char *str, >>> +Error **errp); >>> int socket_init(void); >>> >>> /** >>> diff --git a/net/net.c b/net/net.c >>> index 6235aab..884e3ac 100644 >>> --- a/net/net.c >>> +++ b/net/net.c >>> @@ -100,7 +100,8 @@ static int get_str_sep(char *buf, int buf_size, const >>> char **pp, int sep) >>> return 0; >>> } >>> >>> -int parse_host_port(struct sockaddr_in *saddr, const char *str) >>> +int parse_host_port(struct sockaddr_in *saddr, const char *str, >>> +Error **errp) >>> { >>> char buf[512]; >>> struct hostent *he; >>> @@ -108,24 +109,35 @@ int parse_host_port(struct sockaddr_in *saddr, const >>> char *str) >>> int port; >>> >>> p = str; >>> -if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) >>> +if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { >>> +error_setg(errp, "The address should contain ':', for example: " >>> + "=230.0.0.1:1234"); >> >> Suggest "Host address '%s' should ..." like you do in the next error message. >> >> The = is confusing. Do we need an example here? The other error >> messages in this function apparently don't. >> >> What about "host address '%s' doesn't contain ':' separating host from >> port" or "can't find ':' separating host from port in host address >> '%s'"? >> > > Yes, my description message is really confusing. > Both of your prompt message are well understood. > > Thank you very much. > >> >>> return -1; >>> +} >>> saddr->sin_family = AF_INET; >>> if (buf[0] == '\0') { >>> saddr->sin_addr.s_addr = 0; >>> } else { >>> if (qemu_isdigit(buf[0])) { >>> -if (!inet_aton(buf, &saddr->sin_addr)) >>> +if (!inet_aton(buf, &saddr->sin_addr)) { >>> +error_setg(errp, "Host address '%s' is not a valid " >>> + "IPv4 address", buf); >>> return -1; >>> +} >>> } else { >>> -if ((he = gethostbyname(buf)) == NULL) >>> +he = gethostbyname(buf); >>> +if (he == NULL) { >>> +error_setg(errp, "Specified hostname is error."); >> >> No. Suggest "can't resolve host address '%s'. This error message still >> lacks detail, but I'm not sure hstrerror() is sufficiently portable. >> > > The gethostbyname() return a null pointer if an error occurs, and the h_errno > variable holds an error number. herror() and hstrerror() can prints the error > message associated with the current value of h_errno, but hstrerror() returns > the string type is good for passing the error message to Error. So I'd prefer > the hstrerror. > > As for the portability of hstrerror(), sorry, I'm also not sure, but in this > case I tested, it's OK. so I want to use hstrerror() for a while, if there are > any problem that can be fixed later. Do you think it can be done? Standard first portability question: does Windows provide it? >> Outside this patch's scope: gethostbyname() is obsolete. Applications >> should use getaddrinfo() instead. Comes with gai_strerror(). > > Can I try to fix it later? Sure. By "outside this patch's scope" I mean it's a separate matter that clearly doesn't have to be addressed to get this patch accepted.
[Qemu-devel] [PATCH 2/2] [TEST] Collect TLB and victim TLB hit/miss stats
I used the following patch to collect hit/miss TLB ratios for a few benchmarks. The results can be found here: http://imgur.com/a/gee1o Please note that these results also include boot/shutdown as the per-region instrumentation patch came later. Signed-off-by: Pranith Kumar --- accel/tcg/cputlb.c| 12 cpus.c| 26 ++ include/exec/cpu-defs.h | 4 include/sysemu/cpus.h | 2 ++ target/arm/helper.c | 6 +- tcg/i386/tcg-target.inc.c | 16 ++-- vl.c | 3 +++ 7 files changed, 66 insertions(+), 3 deletions(-) diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c index ef52a7e5e0..2ac2397431 100644 --- a/accel/tcg/cputlb.c +++ b/accel/tcg/cputlb.c @@ -864,12 +864,19 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry, } } +extern bool enable_instrumentation; + /* Return true if ADDR is present in the victim tlb, and has been copied back to the main tlb. */ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index, size_t elt_ofs, target_ulong page) { size_t vidx; + +if (enable_instrumentation) { +env->tlb_access_victim++; +} + for (vidx = 0; vidx < CPU_VTLB_SIZE; ++vidx) { CPUTLBEntry *vtlb = &env->tlb_v_table[mmu_idx][vidx]; target_ulong cmp = *(target_ulong *)((uintptr_t)vtlb + elt_ofs); @@ -885,6 +892,11 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index, CPUIOTLBEntry tmpio, *io = &env->iotlb[mmu_idx][index]; CPUIOTLBEntry *vio = &env->iotlb_v[mmu_idx][vidx]; tmpio = *io; *io = *vio; *vio = tmpio; + +if (enable_instrumentation) { +env->tlb_access_victim_hit++; +} + return true; } } diff --git a/cpus.c b/cpus.c index 14bb8d552e..14669b3469 100644 --- a/cpus.c +++ b/cpus.c @@ -1602,6 +1602,32 @@ static bool all_vcpus_paused(void) return true; } +void print_tlb_stats(void) +{ +CPUState *cpu; +CPU_FOREACH(cpu) { +CPUArchState *cs = cpu->env_ptr; + +fprintf(stderr, "TLB accesses %lu, hits %lu, victim accesses %lu, hits %lu\n", +cs->tlb_access_total, cs->tlb_access_hit, cs->tlb_access_victim, +cs->tlb_access_victim_hit); +} +} + +void clear_tlb_stats(void) +{ +CPUState *cpu; +CPU_FOREACH(cpu) { +CPUArchState *cs = cpu->env_ptr; + +cs->tlb_access_total= 0; +cs->tlb_access_hit = 0; +cs->tlb_access_victim = 0; +cs->tlb_access_victim = 0; +cs->tlb_access_victim_hit = 0; +} +} + void pause_all_vcpus(void) { CPUState *cpu; diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h index 5f4e303635..29b3c2ada8 100644 --- a/include/exec/cpu-defs.h +++ b/include/exec/cpu-defs.h @@ -138,6 +138,10 @@ typedef struct CPUIOTLBEntry { target_ulong tlb_flush_addr;\ target_ulong tlb_flush_mask;\ target_ulong vtlb_index;\ +target_ulong tlb_access_hit;\ +target_ulong tlb_access_total; \ +target_ulong tlb_access_victim; \ +target_ulong tlb_access_victim_hit; \ #else diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index 731756d948..7d8d92646c 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -10,6 +10,8 @@ void resume_all_vcpus(void); void pause_all_vcpus(void); void cpu_stop_current(void); void cpu_ticks_init(void); +void print_tlb_stats(void); +void clear_tlb_stats(void); void configure_icount(QemuOpts *opts, Error **errp); extern int use_icount; diff --git a/target/arm/helper.c b/target/arm/helper.c index dfbf03676c..d2e75b0f20 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1124,7 +1124,9 @@ static uint64_t pmxevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri) } } -bool enable_instrumentation; +extern bool enable_instrumentation; +extern void print_tlb_stats(void); +extern void clear_tlb_stats(void); static void pmuserenr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) @@ -1139,6 +1141,8 @@ static void pmuserenr_write(CPUARMState *env, const ARMCPRegInfo *ri, } else if (value == 0xfa11dead) { printf("Disabling instrumentation\n"); enable_instrumentation = false; +print_tlb_stats(); +clear_tlb_stats(); tb_flush(cs); } diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c index 9d7d25c017..b75bd54c35 100644 --- a/tcg/i386/tcg-target.inc.c +++ b/tcg/i386/tcg-target.inc.c @@ -1250,6 +1250,8 @@ static void * const qemu
[Qemu-devel] [PATCH 1/2] [TEST] aarch64: Use pmuserenr_el0 register for instrumentation
We need a way for the benchmark running in the guest to indicate us to start/stop our instrumentation. On x86, we could use the 'cpuid' instruction along with an appropriately populated 'eax' register. However, no such dummy instruction exists for aarch64. So we modify the permission bits for 'pmuserenr_el0' register and tap that to instrument the guest code. You can use the following annotations on your region-of-interest to instrument the code. #define magic_enable() \ asm volatile ("msr pmuserenr_el0, %0" :: "r" (0x)); #define magic_disable() \ asm volatile ("msr pmuserenr_el0, %0" :: "r" (0xfa11dead)); Signed-off-by: Pranith Kumar --- target/arm/helper.c | 19 +-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/target/arm/helper.c b/target/arm/helper.c index 2594faa9b8..dfbf03676c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -1124,9 +1124,24 @@ static uint64_t pmxevtyper_read(CPUARMState *env, const ARMCPRegInfo *ri) } } +bool enable_instrumentation; + static void pmuserenr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value) { +ARMCPU *cpu = arm_env_get_cpu(env); +CPUState *cs = CPU(cpu); + +if (value == 0x) { +printf("Enabling instrumentation\n"); +enable_instrumentation = true; +tb_flush(cs); +} else if (value == 0xfa11dead) { +printf("Disabling instrumentation\n"); +enable_instrumentation = false; +tb_flush(cs); +} + if (arm_feature(env, ARM_FEATURE_V8)) { env->cp15.c9_pmuserenr = value & 0xf; } else { @@ -1316,13 +1331,13 @@ static const ARMCPRegInfo v7_cp_reginfo[] = { .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0, .accessfn = pmreg_access_xevcntr }, { .name = "PMUSERENR", .cp = 15, .crn = 9, .crm = 14, .opc1 = 0, .opc2 = 0, - .access = PL0_R | PL1_RW, .accessfn = access_tpm, + .access = PL0_RW | PL1_RW, .accessfn = access_tpm, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr), .resetvalue = 0, .writefn = pmuserenr_write, .raw_writefn = raw_write }, { .name = "PMUSERENR_EL0", .state = ARM_CP_STATE_AA64, .opc0 = 3, .opc1 = 3, .crn = 9, .crm = 14, .opc2 = 0, - .access = PL0_R | PL1_RW, .accessfn = access_tpm, .type = ARM_CP_ALIAS, + .access = PL0_RW | PL1_RW, .accessfn = access_tpm, .type = ARM_CP_ALIAS, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmuserenr), .resetvalue = 0, .writefn = pmuserenr_write, .raw_writefn = raw_write }, -- 2.13.0
[Qemu-devel] [TEST PATCH 0/2] Instrumentation and TLB stats
The following two patches are what I use to instrument guest code and collect TLB hit/miss information. These patches are for informational and discussion purposes only. Pranith Kumar (2): [TEST] aarch64: Use pmuserenr_el0 register for instrumentation [TEST] Collect TLB stats along with victim TLB accel/tcg/cputlb.c| 12 cpus.c| 26 ++ include/exec/cpu-defs.h | 4 include/sysemu/cpus.h | 2 ++ target/arm/helper.c | 23 +-- tcg/i386/tcg-target.inc.c | 16 ++-- vl.c | 3 +++ 7 files changed, 82 insertions(+), 4 deletions(-) -- 2.13.0
Re: [Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic
On Tue, Jun 27, 2017 at 5:45 PM, Alistair Francis wrote: > Rename the error_print_loc() function in preperation for using it to preparation > print warnings as well. > > Signed-off-by: Alistair Francis Reviewed-by: Philippe Mathieu-Daudé > --- > > util/qemu-error.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/util/qemu-error.c b/util/qemu-error.c > index b331f8f4a4..1c5e35ecdb 100644 > --- a/util/qemu-error.c > +++ b/util/qemu-error.c > @@ -146,7 +146,7 @@ const char *error_get_progname(void) > /* > * Print current location to current monitor if we have one, else to stderr. > */ > -static void error_print_loc(void) > +static void print_loc(void) > { > const char *sep = ""; > int i; > @@ -197,7 +197,7 @@ void error_vreport(const char *fmt, va_list ap) > g_free(timestr); > } > > -error_print_loc(); > +print_loc(); > error_vprintf(fmt, ap); > error_printf("\n"); > } > -- > 2.11.0 > >
Re: [Qemu-devel] [PATCH qemu v8 2/2] memory/iommu: introduce IOMMUMemoryRegionClass
On 21/06/17 17:26, David Gibson wrote: > On Wed, Jun 21, 2017 at 12:05:08PM +1000, Alexey Kardashevskiy wrote: >> On 14/06/17 16:36, Alexey Kardashevskiy wrote: >>> This finishes QOM'fication of IOMMUMemoryRegion by introducing >>> a IOMMUMemoryRegionClass. This also provides a fastpath analog for >>> IOMMU_MEMORY_REGION_GET_CLASS(). >> >> >> Ping? > > Busy.. sick.. busy & sick.. > > I'll get to it eventually. > Well, if would be nice to hear from anyone... -- Alexey signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
On Tue, Jun 27, 2017 at 8:57 PM, Alistair Francis wrote: > There is no way nhandles can be zero in this section so that part of the > if statement will always be false. Let's just remove it to make the code > easier to read. > > Signed-off-by: Alistair Francis > Acked-by: Edgar E. Iglesias Reviewed-by: Philippe Mathieu-Daudé > --- > > util/oslib-win32.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index 80e4668935..7ec0f8e083 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, > gint nhandles, > /* If we have a timeout, or no handles to poll, be satisfied > * with just noticing we have messages waiting. > */ > -if (timeout != 0 || nhandles == 0) { > +if (timeout != 0) { > return 1; > } > > -- > 2.11.0 > >
Re: [Qemu-devel] [PATCH v5 3/4] net/net: Convert parse_host_port() to Error
Hi, Markus On 06/27/2017 04:04 PM, Markus Armbruster wrote: Mao Zhongyi writes: Cc: berra...@redhat.com Cc: kra...@redhat.com Cc: pbonz...@redhat.com Cc: jasow...@redhat.com Cc: arm...@redhat.com Signed-off-by: Mao Zhongyi --- include/qemu/sockets.h | 3 ++- net/net.c | 22 +- net/socket.c | 19 ++- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 5c326db..78e2b30 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -53,7 +53,8 @@ void socket_listen_cleanup(int fd, Error **errp); int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp); /* Old, ipv4 only bits. Don't use for new code. */ -int parse_host_port(struct sockaddr_in *saddr, const char *str); +int parse_host_port(struct sockaddr_in *saddr, const char *str, +Error **errp); int socket_init(void); /** diff --git a/net/net.c b/net/net.c index 6235aab..884e3ac 100644 --- a/net/net.c +++ b/net/net.c @@ -100,7 +100,8 @@ static int get_str_sep(char *buf, int buf_size, const char **pp, int sep) return 0; } -int parse_host_port(struct sockaddr_in *saddr, const char *str) +int parse_host_port(struct sockaddr_in *saddr, const char *str, +Error **errp) { char buf[512]; struct hostent *he; @@ -108,24 +109,35 @@ int parse_host_port(struct sockaddr_in *saddr, const char *str) int port; p = str; -if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) +if (get_str_sep(buf, sizeof(buf), &p, ':') < 0) { +error_setg(errp, "The address should contain ':', for example: " + "=230.0.0.1:1234"); Suggest "Host address '%s' should ..." like you do in the next error message. The = is confusing. Do we need an example here? The other error messages in this function apparently don't. What about "host address '%s' doesn't contain ':' separating host from port" or "can't find ':' separating host from port in host address '%s'"? Yes, my description message is really confusing. Both of your prompt message are well understood. Thank you very much. return -1; +} saddr->sin_family = AF_INET; if (buf[0] == '\0') { saddr->sin_addr.s_addr = 0; } else { if (qemu_isdigit(buf[0])) { -if (!inet_aton(buf, &saddr->sin_addr)) +if (!inet_aton(buf, &saddr->sin_addr)) { +error_setg(errp, "Host address '%s' is not a valid " + "IPv4 address", buf); return -1; +} } else { -if ((he = gethostbyname(buf)) == NULL) +he = gethostbyname(buf); +if (he == NULL) { +error_setg(errp, "Specified hostname is error."); No. Suggest "can't resolve host address '%s'. This error message still lacks detail, but I'm not sure hstrerror() is sufficiently portable. The gethostbyname() return a null pointer if an error occurs, and the h_errno variable holds an error number. herror() and hstrerror() can prints the error message associated with the current value of h_errno, but hstrerror() returns the string type is good for passing the error message to Error. So I'd prefer the hstrerror. As for the portability of hstrerror(), sorry, I'm also not sure, but in this case I tested, it's OK. so I want to use hstrerror() for a while, if there are any problem that can be fixed later. Do you think it can be done? Outside this patch's scope: gethostbyname() is obsolete. Applications should use getaddrinfo() instead. Comes with gai_strerror(). Can I try to fix it later? Thanks, Mao return - 1; +} saddr->sin_addr = *(struct in_addr *)he->h_addr; } } port = strtol(p, (char **)&r, 0); -if (r == p) +if (r == p) { +error_setg(errp, "The port number is illegal"); Suggest "Port number '%s' is invalid". return -1; +} saddr->sin_port = htons(port); return 0; } diff --git a/net/socket.c b/net/socket.c index e136f87..a875205 100644 --- a/net/socket.c +++ b/net/socket.c @@ -501,9 +501,12 @@ static int net_socket_listen_init(NetClientState *peer, NetSocketState *s; struct sockaddr_in saddr; int fd, ret; +Error *err = NULL; -if (parse_host_port(&saddr, host_str) < 0) +if (parse_host_port(&saddr, host_str, &err) < 0) { +error_report_err(err); return -1; +} fd = qemu_socket(PF_INET, SOCK_STREAM, 0); if (fd < 0) { @@ -548,8 +551,10 @@ static int net_socket_connect_init(NetClientState *peer, struct sockaddr_in saddr; Error *err = NULL; -if (parse_host_port(&saddr, host_str) < 0) +if (parse_host_port(&saddr, host_str, &err) < 0) { +error_report_err(err); return -1; +} fd = qemu_socket(PF_INET, SOCK_STREAM, 0); if (fd < 0) { @@ -601
Re: [Qemu-devel] [PATCH v5 2/4] net/socket: Convert error message to Error
Hi, Markus On 06/27/2017 03:43 PM, Markus Armbruster wrote: Suggest "net/socket: Convert several helper functions to Error". OK, I see. Mao Zhongyi writes: Currently, net_socket_mcast_create(), net_socket_fd_init_dgram() and net_socket_fd_init() use the function such as fprintf(), perror() to report an error message. Now, convert these functions to Error. Cc: jasow...@redhat.com Cc: arm...@redhat.com Signed-off-by: Mao Zhongyi --- net/socket.c | 76 ++-- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/net/socket.c b/net/socket.c index 53765bd..e136f87 100644 --- a/net/socket.c +++ b/net/socket.c @@ -209,7 +209,9 @@ static void net_socket_send_dgram(void *opaque) } } -static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr *localaddr) +static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, + struct in_addr *localaddr, + Error **errp) { struct ip_mreq imr; int fd; @@ -221,8 +223,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr #endif if (!IN_MULTICAST(ntohl(mcastaddr->sin_addr.s_addr))) { -fprintf(stderr, "qemu: error: specified mcastaddr \"%s\" (0x%08x) " -"does not contain a multicast address\n", +error_setg(errp, "qemu: error: specified mcastaddr %s (0x%08x) " +"does not contain a multicast address", Please drop the "qemu: error: " prefix. More of the same below. OK, I will inet_ntoa(mcastaddr->sin_addr), (int)ntohl(mcastaddr->sin_addr.s_addr)); return -1; @@ -230,7 +232,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr } fd = qemu_socket(PF_INET, SOCK_DGRAM, 0); if (fd < 0) { -perror("socket(PF_INET, SOCK_DGRAM)"); +error_setg_errno(errp, errno, ")" + " failed"); I'm not a native speaker, but "Create FOO failed" doesn't feel right to me. Where's the subject? "Create" is a verb. "Creation of FOO failed" has a subject, but doesn't feel right. What about "can't create datagram socket"? Thanks for your sharing. My English is really poor, I will continue to improve. Same for many of the other new error messages. return -1; } @@ -242,13 +245,15 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr val = 1; ret = qemu_setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)); if (ret < 0) { -perror("setsockopt(SOL_SOCKET, SO_REUSEADDR)"); +error_setg_errno(errp, errno, "Set the socket fd=%d attribute" + " SO_REUSEADDR failed", fd); Why would the user want to know the value of @fd here? Same for many of the other new error messages. It really doesn't make sense. I will drop it. goto fail; } ret = bind(fd, (struct sockaddr *)mcastaddr, sizeof(*mcastaddr)); if (ret < 0) { -perror("bind"); +error_setg_errno(errp, errno, "Bind ip=%s to fd=%d failed", + inet_ntoa(mcastaddr->sin_addr), fd); goto fail; } @@ -263,7 +268,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr ret = qemu_setsockopt(fd, IPPROTO_IP, IP_ADD_MEMBERSHIP, &imr, sizeof(struct ip_mreq)); if (ret < 0) { -perror("setsockopt(IP_ADD_MEMBERSHIP)"); +error_setg_errno(errp, errno, "Add socket fd=%d to multicast group %s" + " failed", fd, inet_ntoa(imr.imr_multiaddr)); goto fail; } @@ -272,7 +278,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_LOOP, &loop, sizeof(loop)); if (ret < 0) { -perror("setsockopt(SOL_IP, IP_MULTICAST_LOOP)"); +error_setg_errno(errp, errno, "Force multicast message to loopback" + " failed"); goto fail; } @@ -281,7 +288,8 @@ static int net_socket_mcast_create(struct sockaddr_in *mcastaddr, struct in_addr ret = qemu_setsockopt(fd, IPPROTO_IP, IP_MULTICAST_IF, localaddr, sizeof(*localaddr)); if (ret < 0) { -perror("setsockopt(IP_MULTICAST_IF)"); +error_setg_errno(errp, errno, "Set the default network send " + "interfaec failed"); s/interfaec/interface/ OK, I see. Thanks, Mao goto fail; } } @@ -320,7 +328,8 @@ static NetClientInfo net_dgram_socket_info = { static NetSocketState *net_socket_fd_init_dgram(NetClientState *peer, const char *model, const char *name,
[Qemu-devel] [PATCH v4] virtio-net: enable configurable tx queue size
This patch enables the virtio-net tx queue size to be configurable between 256 (the default queue size) and 1024 by the user when the vhost-user backend is used. Currently, the maximum tx queue size for other backends is 512 due to the following limitations: - QEMU backend: the QEMU backend implementation in some cases may send 1024+1 iovs to writev. - Vhost_net backend: there are possibilities that the guest sends a vring_desc of memory which crosses a MemoryRegion thereby generating more than 1024 iovs after translation from guest-physical address in the backend. Signed-off-by: Wei Wang --- hw/net/virtio-net.c| 32 ++-- include/hw/virtio/virtio-net.h | 1 + 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 91eddaf..a1fc0db 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -34,8 +34,11 @@ /* previously fixed value */ #define VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE 256 +#define VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE 256 + /* for now, only allow larger queues; with virtio-1, guest can downsize */ #define VIRTIO_NET_RX_QUEUE_MIN_SIZE VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE +#define VIRTIO_NET_TX_QUEUE_MIN_SIZE VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE /* * Calculate the number of bytes up to and including the given 'field' of @@ -1508,15 +1511,18 @@ static void virtio_net_add_queue(VirtIONet *n, int index) n->vqs[index].rx_vq = virtio_add_queue(vdev, n->net_conf.rx_queue_size, virtio_net_handle_rx); + if (n->net_conf.tx && !strcmp(n->net_conf.tx, "timer")) { n->vqs[index].tx_vq = -virtio_add_queue(vdev, 256, virtio_net_handle_tx_timer); +virtio_add_queue(vdev, n->net_conf.tx_queue_size, + virtio_net_handle_tx_timer); n->vqs[index].tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, virtio_net_tx_timer, &n->vqs[index]); } else { n->vqs[index].tx_vq = -virtio_add_queue(vdev, 256, virtio_net_handle_tx_bh); +virtio_add_queue(vdev, n->net_conf.tx_queue_size, + virtio_net_handle_tx_bh); n->vqs[index].tx_bh = qemu_bh_new(virtio_net_tx_bh, &n->vqs[index]); } @@ -1927,6 +1933,17 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) return; } +if (n->net_conf.tx_queue_size < VIRTIO_NET_TX_QUEUE_MIN_SIZE || +n->net_conf.tx_queue_size > VIRTQUEUE_MAX_SIZE || +!is_power_of_2(n->net_conf.tx_queue_size)) { +error_setg(errp, "Invalid tx_queue_size (= %" PRIu16 "), " + "must be a power of 2 between %d and %d", + n->net_conf.tx_queue_size, VIRTIO_NET_TX_QUEUE_MIN_SIZE, + VIRTQUEUE_MAX_SIZE); +virtio_cleanup(vdev); +return; +} + n->max_queues = MAX(n->nic_conf.peers.queues, 1); if (n->max_queues * 2 + 1 > VIRTIO_QUEUE_MAX) { error_setg(errp, "Invalid number of queues (= %" PRIu32 "), " @@ -1947,6 +1964,15 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) error_report("Defaulting to \"bh\""); } +/* + * Currently, backends other than vhost-user don't support 1024 queue + * size. + */ +if (n->net_conf.tx_queue_size == VIRTQUEUE_MAX_SIZE && +n->nic_conf.peers.ncs[0]->info->type != NET_CLIENT_DRIVER_VHOST_USER) { +n->net_conf.tx_queue_size = VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE; +} + for (i = 0; i < n->max_queues; i++) { virtio_net_add_queue(n, i); } @@ -2106,6 +2132,8 @@ static Property virtio_net_properties[] = { DEFINE_PROP_STRING("tx", VirtIONet, net_conf.tx), DEFINE_PROP_UINT16("rx_queue_size", VirtIONet, net_conf.rx_queue_size, VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE), +DEFINE_PROP_UINT16("tx_queue_size", VirtIONet, net_conf.tx_queue_size, + VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE), DEFINE_PROP_UINT16("host_mtu", VirtIONet, net_conf.mtu, 0), DEFINE_PROP_BOOL("x-mtu-bypass-backend", VirtIONet, mtu_bypass_backend, true), diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 602b486..b81b6a4 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -36,6 +36,7 @@ typedef struct virtio_net_conf int32_t txburst; char *tx; uint16_t rx_queue_size; +uint16_t tx_queue_size; uint16_t mtu; } virtio_net_conf; -- 2.7.4
[Qemu-devel] [PATCH] exec: fix access to ram_list.dirty_memory when sync dirty bitmap
In cpu_physical_memory_sync_dirty_bitmap(rb, start, ...), the 2nd argument 'start' is relative to the start of the ramblock 'rb'. When it's used to access the dirty memory bitmap of ram_list (i.e. ram_list.dirty_memory[DIRTY_MEMORY_MIGRATION]->blocks[]), an offset to the start of all RAM (i.e. rb->offset) should be added to it, which has however been missed since c/s 6b6712efcc. For a ramblock of host memory backend whose offset is not zero, cpu_physical_memory_sync_dirty_bitmap() synchronizes the incorrect part of the dirty memory bitmap of ram_list to the per ramblock dirty bitmap. As a result, a guest with host memory backend may crash after migration. Fix it by adding the offset of ramblock when accessing the dirty memory bitmap of ram_list in cpu_physical_memory_sync_dirty_bitmap(). Reported-by: Stefan Hajnoczi Signed-off-by: Haozhong Zhang --- include/exec/ram_addr.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h index 73d1bea8b6..cbc797ed05 100644 --- a/include/exec/ram_addr.h +++ b/include/exec/ram_addr.h @@ -377,6 +377,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, uint64_t *real_dirty_pages) { ram_addr_t addr; +ram_addr_t offset = rb->offset; unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS); uint64_t num_dirty = 0; unsigned long *dest = rb->bmap; @@ -386,8 +387,9 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, int k; int nr = BITS_TO_LONGS(length >> TARGET_PAGE_BITS); unsigned long * const *src; -unsigned long idx = (page * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; -unsigned long offset = BIT_WORD((page * BITS_PER_LONG) % +unsigned long word = BIT_WORD((start + offset) >> TARGET_PAGE_BITS); +unsigned long idx = (word * BITS_PER_LONG) / DIRTY_MEMORY_BLOCK_SIZE; +unsigned long offset = BIT_WORD((word * BITS_PER_LONG) % DIRTY_MEMORY_BLOCK_SIZE); rcu_read_lock(); @@ -416,7 +418,7 @@ uint64_t cpu_physical_memory_sync_dirty_bitmap(RAMBlock *rb, } else { for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) { if (cpu_physical_memory_test_and_clear_dirty( -start + addr, +start + addr + offset, TARGET_PAGE_SIZE, DIRTY_MEMORY_MIGRATION)) { *real_dirty_pages += 1; -- 2.11.0
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1
On Fri, Jun 23, 2017 at 04:10:55PM +0200, Laurent Vivier wrote: > On 23/06/2017 11:21, David Gibson wrote: > > On Thu, Jun 22, 2017 at 01:31:24PM +0200, Thomas Huth wrote: > >> On 22.06.2017 13:26, Laurent Vivier wrote: > >>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0. > >>> > >>> When we run qemu on a POWER9 DD1 host, we must use either > >>> "-cpu host" or "-cpu POWER9", but in the latter case it fails with > >>> > >>> Unable to find sPAPR CPU Core definition > >>> > >>> because POWER9 DD1 doesn't appear in the list of known CPUs. > >>> > >>> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 > >>> PVR instead of CPU_POWERPC_POWER9_BASE. > >>> > >>> Signed-off-by: Laurent Vivier > >>> --- > >>> target/ppc/cpu-models.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c > >>> index 4d3e635..a22363c 100644 > >>> --- a/target/ppc/cpu-models.c > >>> +++ b/target/ppc/cpu-models.c > >>> @@ -1144,7 +1144,7 @@ > >>> POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22,970, > >>> "PowerPC 970 v2.2") > >>> > >>> -POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_BASE, > >>> POWER9, > >>> +POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_DD1, > >>> POWER9, > >>> "POWER9 v1.0") > >>> > >>> POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10, 970, > >>> > >> > >> I think this also makes sense for running in TCG mode to get a valid > >> real PVR there. > > > > I'm not so convinced. > > > > IIUC, this will make TCG default (for now) to a DD1 POWER9. That's a) > > probably not what anyone wants - who'd select a buggy prototype and b) > > not accurate - TCG does not implement DD1's bugs. > > According to the POWER8 user manual (I didn't fine the POWER9 one): > > "3.6.3.1 Processor Version Register (PVR) > > The processor revision level (PVR[16:31]) starts at x‘0100’, indicating > revision ‘1.0’. As revisions are made, bits [29:31] will indicate minor > revisions. Similarly, bits [20:23] indicate major changes." > > POWER9 DD1 PVR is 0x004E0100, so this is really version 1.0 of the POWER9. > > Perhaps we can define POWER9_v1.0 as CPU_POWERPC_POWER9_DD1, and > introduce a POWER9_v0.0 set to CPU_POWERPC_POWER9_BASE and define it as > the default one? I like the suggestion to set a v0.0 to CPU_POWERPC_POWER9_BASE. But, I think we could have only that option, removing the CPU_POWERPC_POWER9_DD1 entry. Then, we add the v2.0 (when ready) as the CPU emulated by TCG. To TCG, DD1 wouldn't be listed because it cannot be emulated today. What do you think? Ziviani > > Laurent > >
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1
On Fri, 2017-06-23 at 18:05 +0200, Thomas Huth wrote: > On 23.06.2017 11:21, David Gibson wrote: > > On Thu, Jun 22, 2017 at 01:31:24PM +0200, Thomas Huth wrote: > > > On 22.06.2017 13:26, Laurent Vivier wrote: > > > > CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 > > > > v1.0. > > > > > > > > When we run qemu on a POWER9 DD1 host, we must use either > > > > "-cpu host" or "-cpu POWER9", but in the latter case it fails > > > > with > > > > > > > > Unable to find sPAPR CPU Core definition > > > > > > > > because POWER9 DD1 doesn't appear in the list of known CPUs. > > > > > > > > This patch fixes this by defining POWER9_v1.0 with POWER9 DD1 > > > > PVR instead of CPU_POWERPC_POWER9_BASE. > > > > > > > > Signed-off-by: Laurent Vivier > > > > --- > > > > target/ppc/cpu-models.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c > > > > index 4d3e635..a22363c 100644 > > > > --- a/target/ppc/cpu-models.c > > > > +++ b/target/ppc/cpu-models.c > > > > @@ -1144,7 +1144,7 @@ > > > > POWERPC_DEF("970_v2.2", CPU_POWERPC_970_v22, > > > > 970, > > > > "PowerPC 970 v2.2") > > > > > > > > -POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_BASE, > > > > POWER9, > > > > +POWERPC_DEF("POWER9_v1.0", CPU_POWERPC_POWER9_DD1, > > > > POWER9, > > > > "POWER9 v1.0") > > > > > > > > POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10, > > > > 970, > > > > > > > > > > I think this also makes sense for running in TCG mode to get a > > > valid > > > real PVR there. > > > > I'm not so convinced. > > > > IIUC, this will make TCG default (for now) to a DD1 POWER9. That's > > a) > > probably not what anyone wants - who'd select a buggy prototype and > > b) > > not accurate - TCG does not implement DD1's bugs. > > But DD1 = v1.0. There is no real chip which is using > CPU_POWERPC_POWER9_BASE as PVR ... so using that is also not > accurate, > and also likely not what users expect when the select "POWER9_v1.0", > and > it just does not work as soon as KVM is enabled. So I think Laurent's > patch is the right way to go. Or do you have a better suggestion? > I guess we have do decide what we want to be the primary behaviour when someone puts -cpu POWER9 on the command line. Does that mean that they want an ISAv3.0 compliant cpu or do they want a POWER9 cpu... FWIW: Currently POWER8 is aliased to POWER8_v2.0 which afaik is the last POWER8 revision. Should POWER9 just alias to the most recent POWER9 cpu (currently DD1)? Then how would you specify an ISAv3.0 compliant processor? TCG only implements a ISAv3 compliant POWER9 cpu so it doesn't make sense, and should even refuse to boot with POWER9_DD1 since I think it will fail to boot if we let it. My personal preference would be to have POWER9 alias to the base and the user have to explicitly select if they want a DD* version. But that might frustrate/confuse people. Maybe we should have some table in KVM_HV of allowable processor combinations so that if the user is on ISA compliant hardware then they can specify any such cpu on the command line and KVM_HV still start successfully. > Thomas >
Re: [Qemu-devel] [PATCH v3 20/20] block: Make bdrv_is_allocated_above() byte-based
在 6/28/2017 3:24 AM, Eric Blake 写道: We are gradually moving away from sector-based interfaces, towards byte-based. In the common case, allocation is unlikely to ever use values that are not naturally sector-aligned, but it is possible that byte-based values will let us be more precise about allocation at the end of an unaligned file that can do byte-based access. Changing the signature of the function to use int64_t *pnum ensures that the compiler enforces that all callers are updated. For now, the io.c layer still assert()s that all callers are sector-aligned, but that can be relaxed when a later patch implements byte-based block status. Therefore, for the most part this patch is just the addition of scaling at the callers followed by inverse scaling at bdrv_is_allocated(). But some code, particularly stream_run(), gets a lot simpler because it no longer has to mess with sectors. For ease of review, bdrv_is_allocated() was tackled separately. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: tweak function comments, favor bdrv_getlength() over ->total_sectors --- include/block/block.h | 2 +- block/commit.c| 20 block/io.c| 42 -- block/mirror.c| 5 - block/replication.c | 17 - For replication part: Reviewed-by: Xie Changlong block/stream.c| 21 + qemu-img.c| 10 +++--- 7 files changed, 61 insertions(+), 56 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 9b9d87b..13022d5 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -428,7 +428,7 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs, int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum); int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, -int64_t sector_num, int nb_sectors, int *pnum); +int64_t offset, int64_t bytes, int64_t *pnum); bool bdrv_is_read_only(BlockDriverState *bs); bool bdrv_is_writable(BlockDriverState *bs); diff --git a/block/commit.c b/block/commit.c index 241aa95..774a8a5 100644 --- a/block/commit.c +++ b/block/commit.c @@ -146,7 +146,7 @@ static void coroutine_fn commit_run(void *opaque) int64_t offset; uint64_t delay_ns = 0; int ret = 0; -int n = 0; /* sectors */ +int64_t n = 0; /* bytes */ void *buf = NULL; int bytes_written = 0; int64_t base_len; @@ -171,7 +171,7 @@ static void coroutine_fn commit_run(void *opaque) buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE); -for (offset = 0; offset < s->common.len; offset += n * BDRV_SECTOR_SIZE) { +for (offset = 0; offset < s->common.len; offset += n) { bool copy; /* Note that even when no rate limit is applied we need to yield @@ -183,15 +183,12 @@ static void coroutine_fn commit_run(void *opaque) } /* Copy if allocated above the base */ ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base), - offset / BDRV_SECTOR_SIZE, - COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE, - &n); + offset, COMMIT_BUFFER_SIZE, &n); copy = (ret == 1); -trace_commit_one_iteration(s, offset, n * BDRV_SECTOR_SIZE, ret); +trace_commit_one_iteration(s, offset, n, ret); if (copy) { -ret = commit_populate(s->top, s->base, offset, - n * BDRV_SECTOR_SIZE, buf); -bytes_written += n * BDRV_SECTOR_SIZE; +ret = commit_populate(s->top, s->base, offset, n, buf); +bytes_written += n; } if (ret < 0) { BlockErrorAction action = @@ -204,11 +201,10 @@ static void coroutine_fn commit_run(void *opaque) } } /* Publish progress */ -s->common.offset += n * BDRV_SECTOR_SIZE; +s->common.offset += n; if (copy && s->common.speed) { -delay_ns = ratelimit_calculate_delay(&s->limit, - n * BDRV_SECTOR_SIZE); +delay_ns = ratelimit_calculate_delay(&s->limit, n); } } diff --git a/block/io.c b/block/io.c index 5bbf153..061a162 100644 --- a/block/io.c +++ b/block/io.c @@ -1903,54 +1903,52 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset, /* * Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP] * - * Return true if the given sector is allocated in any image between - * BASE and TOP (inclusive). BASE can be NULL to check if the given - * sector is allocated in any image of the chain. Return false otherwise, + * Return true if the (prefix of the) given range is al
Re: [Qemu-devel] [PATCH v3 15/20] backup: Switch block_backup.h to byte-based
在 6/28/2017 3:24 AM, Eric Blake 写道: We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Continue by converting the public interface to backup jobs (no semantic change), including a change to CowRequest to track by bytes instead of cluster indices. Note that this does not change the difference between the public interface (starting point, and size of the subsequent range) and the internal interface (starting and end points). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: change a couple more parameter names --- include/block/block_backup.h | 11 +-- block/backup.c | 33 - block/replication.c | 12 Reviewed-by: Xie Changlong 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/include/block/block_backup.h b/include/block/block_backup.h index 8a75947..994a3bd 100644 --- a/include/block/block_backup.h +++ b/include/block/block_backup.h @@ -21,17 +21,16 @@ #include "block/block_int.h" typedef struct CowRequest { -int64_t start; -int64_t end; +int64_t start_byte; +int64_t end_byte; QLIST_ENTRY(CowRequest) list; CoQueue wait_queue; /* coroutines blocked on this request */ } CowRequest; -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors); +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes); void backup_cow_request_begin(CowRequest *req, BlockJob *job, - int64_t sector_num, - int nb_sectors); + int64_t offset, uint64_t bytes); void backup_cow_request_end(CowRequest *req); void backup_do_checkpoint(BlockJob *job, Error **errp); diff --git a/block/backup.c b/block/backup.c index 4e64710..cfbd921 100644 --- a/block/backup.c +++ b/block/backup.c @@ -55,7 +55,7 @@ static inline int64_t cluster_size_sectors(BackupBlockJob *job) /* See if in-flight requests overlap and wait for them to complete */ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, - int64_t start, + int64_t offset, int64_t end) { CowRequest *req; @@ -64,7 +64,7 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, do { retry = false; QLIST_FOREACH(req, &job->inflight_reqs, list) { -if (end > req->start && start < req->end) { +if (end > req->start_byte && offset < req->end_byte) { qemu_co_queue_wait(&req->wait_queue, NULL); retry = true; break; @@ -75,10 +75,10 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, /* Keep track of an in-flight request */ static void cow_request_begin(CowRequest *req, BackupBlockJob *job, - int64_t start, int64_t end) + int64_t offset, int64_t end) { -req->start = start; -req->end = end; +req->start_byte = offset; +req->end_byte = end; qemu_co_queue_init(&req->wait_queue); QLIST_INSERT_HEAD(&job->inflight_reqs, req, list); } @@ -114,8 +114,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); -wait_for_overlapping_requests(job, start, end); -cow_request_begin(&cow_request, job, start, end); +wait_for_overlapping_requests(job, start * job->cluster_size, + end * job->cluster_size); +cow_request_begin(&cow_request, job, start * job->cluster_size, + end * job->cluster_size); for (; start < end; start++) { if (test_bit(start, job->done_bitmap)) { @@ -277,32 +279,29 @@ void backup_do_checkpoint(BlockJob *job, Error **errp) bitmap_zero(backup_job->done_bitmap, len); } -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors) +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes) { BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); -int64_t sectors_per_cluster = cluster_size_sectors(backup_job); int64_t start, end; assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); -start = sector_num / sectors_per_cluster; -end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); +start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); +end = QEMU_ALIGN_UP(offset + bytes,
Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning
On Tue, Jun 27, 2017 at 2:10 PM, Edgar E. Iglesias wrote: > On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: >> When QEMU is waiting for a TCP socket connection it reports that message as >> an error. This isn't an error it is a warnign so let's change the report to >> use warning_report() instead. > > Hi Alistair, > > Isn't this more like an informational message rather than a warning? > > A warning would indicate to me that something unexpected and perhaps wrong > happened. > In this case, there's nothing wrong going on. > > We may need more classes, like 'info:' and/or maybe others... Hey Edgar, That is a good point. I can add a info_report() as well then that copies the warning_report() function but prefixes with 'info: '. Thanks, Alistair > > Cheers, > Edgar > > > >> >> Signed-off-by: Alistair Francis >> --- >> >> chardev/char-socket.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/chardev/char-socket.c b/chardev/char-socket.c >> index ccc499cfa1..5a56628e74 100644 >> --- a/chardev/char-socket.c >> +++ b/chardev/char-socket.c >> @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error >> **errp) >> * in TLS and telnet cases, only wait for an accepted socket */ >> while (!s->ioc) { >> if (s->is_listen) { >> -error_report("QEMU waiting for connection on: %s", >> +warning_report("QEMU waiting for connection on: %s", >> chr->filename); >> qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, >> NULL); >> tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); >> -- >> 2.11.0 >> >>
Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function
On Tue, Jun 27, 2017 at 3:21 PM, Thomas Huth wrote: > On 27.06.2017 22:45, Alistair Francis wrote: >> Add a functino which can be used similarly to error_report() execpt to >> inform the users about warnings instead of errors. > > s/functino/function/ > s/execpt/except/ Thanks, I'll fix these two in the next version. > > And could we maybe call the function "warn_report" or something else > instead? I often already have trouble with error_report() to fit the > string into one line of the scarce 80 columns space ... so the shorter > the name of the function, the more characters can be squeezed into the > string in one line later ;-) Good idea, I'll change it to warn_report(). Thanks, Alistair > > Thomas
Re: [Qemu-devel] Getting rid of xen_init_display() (and its dubious call into main_loop_wait())
On Tue, 27 Jun 2017, Peter Maydell wrote: > So, there is exactly one caller of main_loop_wait() in the tree that > passes it 'true' as an argument. That caller is in xen_init_display() > in hw/dispaly/xenfb.c. The function was added in 2009 with the comment > "FIXME/TODO: Kill this. Temporary needed while DisplayState > reorganization is in flight." > > I'd like to think that we've now completed whatever reorg that was, > 8 years on, so can we now get rid of this function? It definitely > seems very dubious to have a display init function with a busy loop > and a call into main_loop_wait()... LOL, you gotta love "temporary fixes". I am happy to see it wasn't me that added it ;-) I think the following should do the trick. --- xenfb: remove xen_init_display "temporary" hack Initialize xenfb properly, as all other backends, from its own "initialise" function. Signed-off-by: Stefano Stabellini diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c index e76c0d8..873e51f 100644 --- a/hw/display/xenfb.c +++ b/hw/display/xenfb.c @@ -71,7 +71,6 @@ struct XenFB { int fbpages; int feature_update; int bug_trigger; -int have_console; int do_resize; struct { @@ -80,6 +79,7 @@ struct XenFB { int up_count; int up_fullscreen; }; +static const GraphicHwOps xenfb_ops; /* */ @@ -855,6 +855,8 @@ static int fb_init(struct XenDevice *xendev) static int fb_initialise(struct XenDevice *xendev) { struct XenFB *fb = container_of(xendev, struct XenFB, c.xendev); +struct XenDevice *xin; +struct XenInput *in; struct xenfb_page *fb_page; int videoram; int rc; @@ -877,16 +879,12 @@ static int fb_initialise(struct XenDevice *xendev) if (rc != 0) return rc; -#if 0 /* handled in xen_init_display() for now */ -if (!fb->have_console) { -fb->c.ds = graphic_console_init(xenfb_update, -xenfb_invalidate, -NULL, -NULL, -fb); -fb->have_console = 1; -} -#endif +fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb); + +xin = xen_pv_find_xendev("vkbd", xen_domid, 0); +in = container_of(xin, struct XenInput, c.xendev); +in->c.con = fb->c.con; +xen_be_check_state(xin); if (xenstore_read_fe_int(xendev, "feature-update", &fb->feature_update) == -1) fb->feature_update = 0; @@ -972,42 +970,3 @@ static const GraphicHwOps xenfb_ops = { .gfx_update = xenfb_update, .update_interval = xenfb_update_interval, }; - -/* - * FIXME/TODO: Kill this. - * Temporary needed while DisplayState reorganization is in flight. - */ -void xen_init_display(int domid) -{ -struct XenDevice *xfb, *xin; -struct XenFB *fb; -struct XenInput *in; -int i = 0; - -wait_more: -i++; -main_loop_wait(true); -xfb = xen_pv_find_xendev("vfb", domid, 0); -xin = xen_pv_find_xendev("vkbd", domid, 0); -if (!xfb || !xin) { -if (i < 256) { -usleep(1); -goto wait_more; -} -xen_pv_printf(NULL, 1, "displaystate setup failed\n"); -return; -} - -/* vfb */ -fb = container_of(xfb, struct XenFB, c.xendev); -fb->c.con = graphic_console_init(NULL, 0, &xenfb_ops, fb); -fb->have_console = 1; - -/* vkbd */ -in = container_of(xin, struct XenInput, c.xendev); -in->c.con = fb->c.con; - -/* retry ->init() */ -xen_be_check_state(xin); -xen_be_check_state(xfb); -} diff --git a/hw/xenpv/xen_machine_pv.c b/hw/xenpv/xen_machine_pv.c index 79aef4e..31d2f25 100644 --- a/hw/xenpv/xen_machine_pv.c +++ b/hw/xenpv/xen_machine_pv.c @@ -94,9 +94,6 @@ static void xen_init_pv(MachineState *machine) /* config cleanup hook */ atexit(xen_config_cleanup); - -/* setup framebuffer */ -xen_init_display(xen_domid); } static void xenpv_machine_init(MachineClass *mc)
[Qemu-devel] [RFC v1 1/4] util/aio-win32: Only select on what we are actually waiting for
Signed-off-by: Alistair Francis Acked-by: Edgar E. Iglesias --- util/aio-win32.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/util/aio-win32.c b/util/aio-win32.c index bca496a47a..949979c2f5 100644 --- a/util/aio-win32.c +++ b/util/aio-win32.c @@ -71,6 +71,7 @@ void aio_set_fd_handler(AioContext *ctx, } } else { HANDLE event; +long bitmask = 0; if (node == NULL) { /* Alloc and insert if it's not already there */ @@ -95,10 +96,16 @@ void aio_set_fd_handler(AioContext *ctx, node->io_write = io_write; node->is_external = is_external; +if (io_read) { +bitmask |= FD_READ; +} + +if (io_write) { +bitmask |= FD_WRITE; +} + event = event_notifier_get_handle(&ctx->notifier); -WSAEventSelect(node->pfd.fd, event, - FD_READ | FD_ACCEPT | FD_CLOSE | - FD_CONNECT | FD_WRITE | FD_OOB); +WSAEventSelect(node->pfd.fd, event, bitmask); } qemu_lockcnt_unlock(&ctx->list_lock); -- 2.11.0
Re: [Qemu-devel] [PATCH v3 1/7] target/m68k: add fscc.
Le 27/06/2017 à 22:00, Richard Henderson a écrit : > On 06/27/2017 12:12 PM, Laurent Vivier wrote: >> case 3: /* Ordered Greater than or Equal Z || !(A || N) */ >> case 19: /* Greater than or Equal Z || !(A || N) */ >> +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); >> +c->v1 = tcg_temp_new(); >> +c->g1 = 0; >> +tcg_gen_shli_i32(c->v1, fpsr, 3); >> +tcg_gen_or_i32(c->v1, c->v1, fpsr); >> +tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N); >> +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_Z); >> +c->tcond = TCG_COND_NE; > > Still with the unmasked shift. > > tcg_gen_not_i32(c->v1, fpsr); > tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_A | FPSR_CC_N); > tcg_gen_andi_i32(fpsr, fpsr, FPSR_CC_Z); > tcg_gen_or_i32(c->v1, c->v1, fpsr); > > >> case 5: /* Ordered Less than or Equal Z || (N && !A) */ >> case 21: /* Less than or Equal Z || (N && !A) */ >> +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); >> +c->v1 = tcg_temp_new(); >> +c->g1 = 0; >> +tcg_gen_xori_i32(c->v1, fpsr, FPSR_CC_A); >> +tcg_gen_shli_i32(c->v1, c->v1, 3); >> +tcg_gen_ori_i32(c->v1, c->v1, FPSR_CC_Z); >> +tcg_gen_and_i32(c->v1, c->v1, fpsr); >> +c->tcond = TCG_COND_NE; > > Likewise. > > tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A); > tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - ctz32(FPSR_CC_A)); > tcg_gen_andc_i32(c->v1, fpsr, c->v1); > tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_Z | FPSR_CC_N); > > >> case 10: /* Unordered or Greater Than A || !(N || Z)) */ >> case 26: /* Not Less or Equal A || !(N || Z)) */ >> +g_assert(FPSR_CC_Z == (FPSR_CC_N >> 1)); >> +c->v1 = tcg_temp_new(); >> +c->g1 = 0; >> +tcg_gen_shli_i32(c->v1, fpsr, 1); >> +tcg_gen_or_i32(c->v1, c->v1, fpsr); >> +tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N); >> +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_A); >> +c->tcond = TCG_COND_NE; > > Likewise. > > tcg_gen_not_i32(c->v1, fpsr); > tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_Z | FPSR_CC_N); > tcg_gen_andi_i32(fpsr, fpsr, FPSR_CC_A); > tcg_gen_or_i32(c->v1, c->v1, fpsr); > > >> case 12: /* Unordered or Less Than A || (N && !Z) */ >> case 28: /* Not Greater than or Equal A || (N && !Z) */ >> +c->v1 = tcg_temp_new(); >> +c->g1 = 0; >> +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_Z); >> +tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - >> ctz32(FPSR_CC_Z)); >> +tcg_gen_andc_i32(c->v1, fpsr, c->v1); >> +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_A | FPSR_CC_N); >> +c->tcond = TCG_COND_NE; > > I hadn't meant that this was the only one to fix. > Thank you Richard, but all these changes break something somewhere, I'm searching why. Thanks, Laurent
[Qemu-devel] [RFC v1 0/4] Windows runtime improvements
At Xilinx we have started to run our fork of QEMU on Windows and are starting to distrubute this to customers. As part of this QEMU is launched from an Eclipse based GUI on Windows hosts. This uncovered a few performance issues in QEMU when running on CPU restricted machines. What we see is that when QEMU is run on a machine where there aren't enough spare CPUs on the host, QEMU is extreamly slow. We especially see this when running our multi-architecture QEMU setup (MicroBlaze and ARM) because we are running two QEMU instances as well as what else is running on the machine. Generally two instances of QEMU will never reach a Linux login prompt when run on four host CPUs, but will reach the login prompt when run on eight CPUs. We investigated the issue and realised that it is mostly because QEMU did not block and instead the IO thread just busy looped. This basically locked up two CPUs (two QEMU instances) with IO threads not leaving enough resources on the machine. This patch series fixed the issue for us on our fork of QEMU. Our fork is based on QEMU 2.8.1 and does not include MTTCG support. I have not tested this on the mainline QEMU or with MTTCG. It is on my todo list to see if I can repdouce the same issue on Windows with mainline QEMU. In the meantime I wanted to send this series to see if anyone else has seen Windows performance issues and if this helps with the problems. In order to see the issue we had to do a full Linux boot, smaller baremetal applications generally don't reproduce the same issue. Also, most of these patches are editing the QEMU implementation of Glib, obviously these fixes (if applicable) will need to be ported to Glib and applied there as well. I just wanted to start here. Alistair Francis (4): util/aio-win32: Only select on what we are actually waiting for util/oslib-win32: Remove invalid check util/oslib-win32: Fix up if conditional util/oslib-win32: Recursivly pass the timeout util/aio-win32.c | 13 ++--- util/oslib-win32.c | 25 +++-- 2 files changed, 29 insertions(+), 9 deletions(-) -- 2.11.0
[Qemu-devel] [RFC v1 4/4] util/oslib-win32: Recursivly pass the timeout
Signed-off-by: Alistair Francis Acked-by: Edgar E. Iglesias --- util/oslib-win32.c | 21 + 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/util/oslib-win32.c b/util/oslib-win32.c index a015e1ac96..3630e46499 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -432,10 +432,10 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, gint nhandles, } } -/* If no timeout and polling several handles, recurse to poll - * the rest of them. +/* We only found one and we are waiting on more then one. Let's try + * again. */ -if (timeout == 0 && nhandles > 1) { +if (nhandles > 1) { /* Remove the handle that fired */ int i; if ((ready - WAIT_OBJECT_0) < nhandles - 1) { @@ -444,7 +444,20 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, gint nhandles, } } nhandles--; -recursed_result = poll_rest(FALSE, handles, nhandles, fds, nfds, 0); + +/* If we just had a very small timeout let's increase it when we + * recurse to ensure we don't just busy wait. This ensures we let + * the Windows threads block at least a little. If we previously + * had some wait let's set it to zero to avoid blocking for too + * long. + */ +if (timeout < 10) { +timeout = timeout + 1; +} else { +timeout = 0; +} +recursed_result = poll_rest(FALSE, handles, nhandles, fds, +nfds, timeout); return (recursed_result == -1) ? -1 : 1 + recursed_result; } return 1; -- 2.11.0
[Qemu-devel] [RFC v1 3/4] util/oslib-win32: Fix up if conditional
Signed-off-by: Alistair Francis Acked-by: Edgar E. Iglesias --- util/oslib-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 7ec0f8e083..a015e1ac96 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -438,7 +438,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, gint nhandles, if (timeout == 0 && nhandles > 1) { /* Remove the handle that fired */ int i; -if (ready < nhandles - 1) { +if ((ready - WAIT_OBJECT_0) < nhandles - 1) { for (i = ready - WAIT_OBJECT_0 + 1; i < nhandles; i++) { handles[i-1] = handles[i]; } -- 2.11.0
[Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check
There is no way nhandles can be zero in this section so that part of the if statement will always be false. Let's just remove it to make the code easier to read. Signed-off-by: Alistair Francis Acked-by: Edgar E. Iglesias --- util/oslib-win32.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 80e4668935..7ec0f8e083 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, gint nhandles, /* If we have a timeout, or no handles to poll, be satisfied * with just noticing we have messages waiting. */ -if (timeout != 0 || nhandles == 0) { +if (timeout != 0) { return 1; } -- 2.11.0
Re: [Qemu-devel] [PATCH v4 3/4] virtio-9p: break device if buffers are misconfigured
On Tue, 27 Jun 2017, Greg Kurz wrote: > On Mon, 26 Jun 2017 16:22:23 -0700 (PDT) > Stefano Stabellini wrote: > > > On Fri, 23 Jun 2017, Greg Kurz wrote: > > > The 9P protocol is transport agnostic: if the guest misconfigured the > > > buffers, the best we can do is to set the broken flag on the device. > > > > > > Since virtio_pdu_vmarshal() may be called by several active PDUs, we > > > check if the transport isn't broken already to avoid printing extra > > > error messages. > > > > > > Signed-off-by: Greg Kurz > > > --- > > > v4: - update changelog and add comment to explain why we check > > > vdev->broken > > > in virtio_pdu_vmarshal() > > > - dropped uneeded vdev->broken check in virtio_pdu_vunmarshal() > > > --- > > > hw/9pfs/9p.c |2 +- > > > hw/9pfs/9p.h |2 +- > > > hw/9pfs/virtio-9p-device.c | 48 > > > +++- > > > hw/9pfs/xen-9p-backend.c |3 ++- > > > 4 files changed, 47 insertions(+), 8 deletions(-) > > > > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c > > > index a0ae98f7ca6f..8e5cac71eb60 100644 > > > --- a/hw/9pfs/9p.c > > > +++ b/hw/9pfs/9p.c > > > @@ -1664,7 +1664,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector > > > *qiov, V9fsPDU *pdu, > > > unsigned int niov; > > > > > > if (is_write) { > > > -pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov); > > > +pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov, size > > > + skip); > > > } else { > > > pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size + > > > skip); > > > } > > > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h > > > index aac1b0b2ce3d..d1cfeaf10e4f 100644 > > > --- a/hw/9pfs/9p.h > > > +++ b/hw/9pfs/9p.h > > > @@ -363,7 +363,7 @@ struct V9fsTransport { > > > void(*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec > > > **piov, > > > unsigned int *pniov, size_t > > > size); > > > void(*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec > > > **piov, > > > - unsigned int *pniov); > > > + unsigned int *pniov, size_t > > > size); > > > void(*push_and_notify)(V9fsPDU *pdu); > > > }; > > > > > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > > > index 1a68c1622d3a..ed9e4817a26c 100644 > > > --- a/hw/9pfs/virtio-9p-device.c > > > +++ b/hw/9pfs/virtio-9p-device.c > > > @@ -146,8 +146,22 @@ static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, > > > size_t offset, > > > V9fsState *s = pdu->s; > > > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > > > VirtQueueElement *elem = v->elems[pdu->idx]; > > > - > > > -return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, > > > ap); > > > +int ret; > > > > I think ret should be ssize_t > > > > Yes, you're right. I'll change that. > > > > > > +ret = v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, > > > ap); > > > +if (ret < 0) { > > > +VirtIODevice *vdev = VIRTIO_DEVICE(v); > > > + > > > +/* Any active PDU may try to send something back to the client > > > without > > > + * knowing if the transport is broken or not. This could result > > > in > > > + * MAX_REQ - 1 (ie, 127) extra error messages being printed. > > > + */ > > > +if (!vdev->broken) { > > > +virtio_error(vdev, "Failed to encode VirtFS reply type %d", > > > + pdu->id + 1); > > > +} > > > +} > > > +return ret; > > > } > > > > > > static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset, > > > @@ -156,28 +170,52 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, > > > size_t offset, > > > V9fsState *s = pdu->s; > > > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > > > VirtQueueElement *elem = v->elems[pdu->idx]; > > > +int ret; > > > > same here > > > > Ditto. > > > > > > -return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, > > > fmt, ap); > > > +ret = v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, > > > fmt, ap); > > > +if (ret < 0) { > > > +VirtIODevice *vdev = VIRTIO_DEVICE(v); > > > + > > > +virtio_error(vdev, "Failed to decode VirtFS request type %d", > > > pdu->id); > > > +} > > > +return ret; > > > } > > > > > > -/* The size parameter is used by other transports. Do not drop it. */ > > > static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec > > > **piov, > > > unsigned int *pniov, size_t size) > > > { > > > V9fsState *s = pdu->s; > > > V9fsVirtioState *v = container_of(s, V9fsVirtioState, state); > > > VirtQueueElement *elem = v->elems[pdu->idx]; > > > +size_t buf_size = iov_size(elem->in_sg, ele
[Qemu-devel] [PATCH 2/2] block: add default implementations for bdrv_co_get_block_status()
bdrv_co_get_block_status_from_file() and bdrv_co_get_block_status_from_backing() set *file to bs->file and bs->backing respectively, so that bdrv_co_get_block_status() can recurse to them. Future block drivers won't have to duplicate code to implement this. Signed-off-by: Manos Pitsidianakis --- block/blkdebug.c | 12 +--- block/commit.c| 12 +--- block/io.c| 24 block/mirror.c| 12 +--- include/block/block_int.h | 16 5 files changed, 43 insertions(+), 33 deletions(-) diff --git a/block/blkdebug.c b/block/blkdebug.c index b25856c4..f1539db6 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -641,16 +641,6 @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs, return bdrv_co_pdiscard(bs->file->bs, offset, bytes); } -static int64_t coroutine_fn blkdebug_co_get_block_status( -BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, -BlockDriverState **file) -{ -*pnum = nb_sectors; -*file = bs->file->bs; -return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | -(sector_num << BDRV_SECTOR_BITS); -} - static void blkdebug_close(BlockDriverState *bs) { BDRVBlkdebugState *s = bs->opaque; @@ -925,7 +915,7 @@ static BlockDriver bdrv_blkdebug = { .bdrv_co_flush_to_disk = blkdebug_co_flush, .bdrv_co_pwrite_zeroes = blkdebug_co_pwrite_zeroes, .bdrv_co_pdiscard = blkdebug_co_pdiscard, -.bdrv_co_get_block_status = blkdebug_co_get_block_status, +.bdrv_co_get_block_status = bdrv_co_get_block_status_from_file, .bdrv_debug_event = blkdebug_debug_event, .bdrv_debug_breakpoint = blkdebug_debug_breakpoint, diff --git a/block/commit.c b/block/commit.c index 524bd549..5b04f832 100644 --- a/block/commit.c +++ b/block/commit.c @@ -247,16 +247,6 @@ static int coroutine_fn bdrv_commit_top_preadv(BlockDriverState *bs, return bdrv_co_preadv(bs->backing, offset, bytes, qiov, flags); } -static int64_t coroutine_fn bdrv_commit_top_get_block_status( -BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, -BlockDriverState **file) -{ -*pnum = nb_sectors; -*file = bs->backing->bs; -return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | - (sector_num << BDRV_SECTOR_BITS); -} - static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts) { bdrv_refresh_filename(bs->backing->bs); @@ -282,7 +272,7 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c, static BlockDriver bdrv_commit_top = { .format_name= "commit_top", .bdrv_co_preadv = bdrv_commit_top_preadv, -.bdrv_co_get_block_status = bdrv_commit_top_get_block_status, +.bdrv_co_get_block_status = bdrv_co_get_block_status_from_backing, .bdrv_refresh_filename = bdrv_commit_top_refresh_filename, .bdrv_close = bdrv_commit_top_close, .bdrv_child_perm= bdrv_commit_top_child_perm, diff --git a/block/io.c b/block/io.c index c1b73226..aec520b4 100644 --- a/block/io.c +++ b/block/io.c @@ -1706,6 +1706,30 @@ typedef struct BdrvCoGetBlockStatusData { bool done; } BdrvCoGetBlockStatusData; +int64_t coroutine_fn bdrv_co_get_block_status_from_file(BlockDriverState *bs, + int64_t sector_num, + int nb_sectors, int *pnum, + BlockDriverState **file) +{ +assert(bs->file && bs->file->bs); +*pnum = nb_sectors; +*file = bs->file->bs; +return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | + (sector_num << BDRV_SECTOR_BITS); +} + +int64_t coroutine_fn bdrv_co_get_block_status_from_backing(BlockDriverState *bs, + int64_t sector_num, + int nb_sectors, int *pnum, + BlockDriverState **file) +{ +assert(bs->backing && bs->backing->bs); +*pnum = nb_sectors; +*file = bs->backing->bs; +return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | + (sector_num << BDRV_SECTOR_BITS); +} + /* * Returns the allocation status of the specified sectors. * Drivers not implementing the functionality are assumed to not support diff --git a/block/mirror.c b/block/mirror.c index 61a862dc..e8bf5f40 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -1052,16 +1052,6 @@ static int coroutine_fn bdrv_mirror_top_flush(BlockDriverState *bs) return bdrv_co_flush(bs->backing->bs); } -static int64_t coroutine_fn bdrv_mirror_top_get_block_status( -BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, -BlockDriverState **file) -{ -*pnum = nb_sectors; -*file = bs->backing->bs; -return BDRV_BLOCK_RAW | BDRV_BLOCK_OFF
[Qemu-devel] [PATCH 1/2] block: pass bdrv_* methods to bs->file by default
The following functions fail if bs->drv does not implement them: bdrv_probe_blocksizes bdrv_probe_geometry bdrv_truncate bdrv_has_zero_init bdrv_get_info bdrv_media_changed bdrv_eject bdrv_lock_medium bdrv_co_ioctl Instead, the call should be passed to bs->file if it exists, to allow filter drivers to support those methods without implementing them. Signed-off-by: Manos Pitsidianakis --- block.c| 45 ++--- block/io.c | 4 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/block.c b/block.c index 69439628..ad11230a 100644 --- a/block.c +++ b/block.c @@ -494,6 +494,8 @@ int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz) if (drv && drv->bdrv_probe_blocksizes) { return drv->bdrv_probe_blocksizes(bs, bsz); +} else if (bs->file && bs->file->bs) { +return bdrv_probe_blocksizes(bs->file->bs, bsz); } return -ENOTSUP; @@ -511,6 +513,8 @@ int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo) if (drv && drv->bdrv_probe_geometry) { return drv->bdrv_probe_geometry(bs, geo); +} else if (bs->file && bs->file->bs) { +return bdrv_probe_geometry(bs->file->bs, geo); } return -ENOTSUP; @@ -3406,13 +3410,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp) assert(child->perm & BLK_PERM_RESIZE); -if (!drv) { -error_setg(errp, "No medium inserted"); -return -ENOMEDIUM; -} -if (!drv->bdrv_truncate) { -error_setg(errp, "Image format driver does not support resize"); -return -ENOTSUP; +if (!drv || !drv->bdrv_truncate) { +if (bs->file && bs->file->bs) { +return bdrv_truncate(bs->file, offset, errp); +} +if (!drv) { +error_setg(errp, "No medium inserted"); +return -ENOMEDIUM; +} +if (!drv->bdrv_truncate) { +error_setg(errp, "Image format driver does not support resize"); +return -ENOTSUP; +} } if (bs->read_only) { error_setg(errp, "Image is read-only"); @@ -3778,6 +3787,9 @@ int bdrv_has_zero_init(BlockDriverState *bs) if (bs->drv->bdrv_has_zero_init) { return bs->drv->bdrv_has_zero_init(bs); } +if (bs->file && bs->file->bs) { +return bdrv_has_zero_init(bs->file->bs); +} /* safe default */ return 0; @@ -3832,10 +3844,15 @@ void bdrv_get_backing_filename(BlockDriverState *bs, int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) { BlockDriver *drv = bs->drv; -if (!drv) -return -ENOMEDIUM; -if (!drv->bdrv_get_info) -return -ENOTSUP; +if (!drv || !drv->bdrv_get_info) { +if (bs->file && bs->file->bs) { +return bdrv_get_info(bs->file->bs, bdi); +} +if (!drv) +return -ENOMEDIUM; +if (!drv->bdrv_get_info) +return -ENOTSUP; +} memset(bdi, 0, sizeof(*bdi)); return drv->bdrv_get_info(bs, bdi); } @@ -4205,6 +4222,8 @@ int bdrv_media_changed(BlockDriverState *bs) if (drv && drv->bdrv_media_changed) { return drv->bdrv_media_changed(bs); +} else if (bs->file && bs->file->bs) { +bdrv_media_changed(bs->file->bs); } return -ENOTSUP; } @@ -4218,6 +4237,8 @@ void bdrv_eject(BlockDriverState *bs, bool eject_flag) if (drv && drv->bdrv_eject) { drv->bdrv_eject(bs, eject_flag); +} else if (bs->file && bs->file->bs) { +bdrv_eject(bs->file->bs, eject_flag); } } @@ -4233,6 +4254,8 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked) if (drv && drv->bdrv_lock_medium) { drv->bdrv_lock_medium(bs, locked); +} else if (bs->file && bs->file->bs) { +bdrv_lock_medium(bs->file->bs, locked); } } diff --git a/block/io.c b/block/io.c index c72d7015..c1b73226 100644 --- a/block/io.c +++ b/block/io.c @@ -2403,6 +2403,10 @@ int bdrv_co_ioctl(BlockDriverState *bs, int req, void *buf) bdrv_inc_in_flight(bs); if (!drv || (!drv->bdrv_aio_ioctl && !drv->bdrv_co_ioctl)) { +if (bs->file && bs->file->bs) { +bdrv_dec_in_flight(bs); +return bdrv_co_ioctl(bs->file->bs, req, buf); +} co.ret = -ENOTSUP; goto out; } -- 2.11.0
[Qemu-devel] [PATCH 0/2] block: block driver callbacks fixes
This series makes implementing some of the bdrv_* callbacks easier for block drivers by passing requests to bs->file if bs->drv doesn't implement it instead of failing, and adding default bdrv_co_get_block_status() implementations. This based against Kevin Wolf's block branch, commit 0dd2ec6bc46bd93ea0b9df602962883417f31400 Manos Pitsidianakis (2): block: pass bdrv_* methods to bs->file by default block: add default implementations for bdrv_co_get_block_status() block.c | 45 ++--- block/blkdebug.c | 12 +--- block/commit.c| 12 +--- block/io.c| 28 block/mirror.c| 12 +--- include/block/block_int.h | 16 6 files changed, 81 insertions(+), 44 deletions(-) -- 2.11.0
Re: [Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function
On 27.06.2017 22:45, Alistair Francis wrote: > Add a functino which can be used similarly to error_report() execpt to > inform the users about warnings instead of errors. s/functino/function/ s/execpt/except/ And could we maybe call the function "warn_report" or something else instead? I often already have trouble with error_report() to fit the string into one line of the scarce 80 columns space ... so the shorter the name of the function, the more characters can be squeezed into the string in one line later ;-) Thomas
Re: [Qemu-devel] [RFC PATCH 01/14] pc-bios/s390-ccw: Add the libc from the SLOF firmware
On 27.06.2017 17:32, David Hildenbrand wrote: > On 27.06.2017 13:48, Thomas Huth wrote: >> To be able to use some more advanced libc functions in the s390-ccw >> firmware, like printf() and malloc(), we need a better libc here. >> This patch adds the C library from the SLOF firmware (taken from >> the SLOF commit ID 62674aabe20612a9786fa03e87cf6916ba97a99a). The >> files are copied without modifications here and will be adapted for >> the s390-ccw firmware by the next patch. I just removed the getopt() >> and scanf()-like functions from the libc since we likely do not need >> them in the s390-ccw firmware. >> > > We have SLOF as a git submodule in roms/SLOF. > > I wonder if there would a way to avoid duplicating files. > > E.g. build s390x-ccw.img only if roms/SLOF is checked out and link/copy > the right folder. > > Then, also the question regarding coding style is gone. > > Would something like that work? Cool idea, I like it at a first glance. It might be possible ... but that also creates a lot of dependencies, e.g. each time there is a related change in the SLOF repository, you run into the problem that it might affect the s390-ccw firmware, too. Not sure whether we really always want to deal with that situation... Thomas
[Qemu-devel] [PULL 3/3] xen-disk: add support for multi-page shared rings
From: Paul Durrant The blkif protocol has had provision for negotiation of multi-page shared rings for some time now and many guest OS have support in their frontend drivers. This patch makes the necessary modifications to xen-disk support a shared ring up to order 4 (i.e. 16 pages). Signed-off-by: Paul Durrant Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini --- hw/block/xen_disk.c | 144 +--- 1 file changed, 113 insertions(+), 31 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 8218741..d42ed70 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -36,8 +36,6 @@ static int batch_maps = 0; -static int max_requests = 32; - /* - */ #define BLOCK_SIZE 512 @@ -84,6 +82,8 @@ struct ioreq { BlockAcctCookie acct; }; +#define MAX_RING_PAGE_ORDER 4 + struct XenBlkDev { struct XenDevicexendev; /* must be first */ char*params; @@ -94,7 +94,8 @@ struct XenBlkDev { booldirectiosafe; const char *fileproto; const char *filename; -int ring_ref; +unsigned intring_ref[1 << MAX_RING_PAGE_ORDER]; +unsigned intnr_ring_ref; void*sring; int64_t file_blk; int64_t file_size; @@ -110,6 +111,7 @@ struct XenBlkDev { int requests_total; int requests_inflight; int requests_finished; +unsigned intmax_requests; /* Persistent grants extension */ gbooleanfeature_discard; @@ -199,7 +201,7 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev) struct ioreq *ioreq = NULL; if (QLIST_EMPTY(&blkdev->freelist)) { -if (blkdev->requests_total >= max_requests) { +if (blkdev->requests_total >= blkdev->max_requests) { goto out; } /* allocate new struct */ @@ -904,7 +906,7 @@ static void blk_handle_requests(struct XenBlkDev *blkdev) ioreq_runio_qemu_aio(ioreq); } -if (blkdev->more_work && blkdev->requests_inflight < max_requests) { +if (blkdev->more_work && blkdev->requests_inflight < blkdev->max_requests) { qemu_bh_schedule(blkdev->bh); } } @@ -917,15 +919,6 @@ static void blk_bh(void *opaque) blk_handle_requests(blkdev); } -/* - * We need to account for the grant allocations requiring contiguous - * chunks; the worst case number would be - * max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, - * but in order to keep things simple just use - * 2 * max_req * max_seg. - */ -#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg)) - static void blk_alloc(struct XenDevice *xendev) { struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev); @@ -937,11 +930,6 @@ static void blk_alloc(struct XenDevice *xendev) if (xen_mode != XEN_EMULATE) { batch_maps = 1; } -if (xengnttab_set_max_grants(xendev->gnttabdev, -MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) { -xen_pv_printf(xendev, 0, "xengnttab_set_max_grants failed: %s\n", - strerror(errno)); -} } static void blk_parse_discard(struct XenBlkDev *blkdev) @@ -1036,6 +1024,9 @@ static int blk_init(struct XenDevice *xendev) !blkdev->feature_grant_copy); xenstore_write_be_int(&blkdev->xendev, "info", info); +xenstore_write_be_int(&blkdev->xendev, "max-ring-page-order", + MAX_RING_PAGE_ORDER); + blk_parse_discard(blkdev); g_free(directiosafe); @@ -1057,12 +1048,25 @@ out_error: return -1; } +/* + * We need to account for the grant allocations requiring contiguous + * chunks; the worst case number would be + * max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, + * but in order to keep things simple just use + * 2 * max_req * max_seg. + */ +#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg)) + static int blk_connect(struct XenDevice *xendev) { struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev); int pers, index, qflags; bool readonly = true; bool writethrough = true; +int order, ring_ref; +unsigned int ring_size, max_grants; +unsigned int i; +uint32_t *domids; /* read-only ? */ if (blkdev->directiosafe) { @@ -1137,9 +1141,42 @@ static int blk_connect(struct XenDevice *xendev) xenstore_write_be_int64(&blkdev->xendev, "sectors", blkdev->file_size / blkdev->file_blk); -if (xenstore_read_fe_int(&blkdev->xendev, "ring-ref", &blkdev->ring_ref) == -1) { +if (xenstore_read_fe_int(&blkdev->xendev, "ring-page-order", + &order) == -1) { +blkdev->nr_ring_ref = 1; + +
Re: [Qemu-devel] [Xen-devel] [PATCH v2 0/3] xen-disk: performance improvements
On Wed, 21 Jun 2017, Paul Durrant wrote: > Paul Durrant (3): > xen-disk: only advertize feature-persistent if grant copy is not > available > xen-disk: add support for multi-page shared rings > xen-disk: use an IOThread per instance > > hw/block/trace-events | 7 ++ > hw/block/xen_disk.c | 228 > +++--- > 2 files changed, 188 insertions(+), 47 deletions(-) While waiting for an answer on patch #3, I sent a pull request for the first 2 patches
[Qemu-devel] [PULL 2/3] xen-disk: only advertize feature-persistent if grant copy is not available
From: Paul Durrant If grant copy is available then it will always be used in preference to persistent maps. In this case feature-persistent should not be advertized to the frontend, otherwise it may needlessly copy data into persistently granted buffers. Signed-off-by: Paul Durrant Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini --- hw/block/xen_disk.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 9200511..8218741 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -1022,11 +1022,18 @@ static int blk_init(struct XenDevice *xendev) blkdev->file_blk = BLOCK_SIZE; +blkdev->feature_grant_copy = +(xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0); + +xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n", + blkdev->feature_grant_copy ? "enabled" : "disabled"); + /* fill info * blk_connect supplies sector-size and sectors */ xenstore_write_be_int(&blkdev->xendev, "feature-flush-cache", 1); -xenstore_write_be_int(&blkdev->xendev, "feature-persistent", 1); +xenstore_write_be_int(&blkdev->xendev, "feature-persistent", + !blkdev->feature_grant_copy); xenstore_write_be_int(&blkdev->xendev, "info", info); blk_parse_discard(blkdev); @@ -1201,12 +1208,6 @@ static int blk_connect(struct XenDevice *xendev) xen_be_bind_evtchn(&blkdev->xendev); -blkdev->feature_grant_copy = -(xengnttab_grant_copy(blkdev->xendev.gnttabdev, 0, NULL) == 0); - -xen_pv_printf(&blkdev->xendev, 3, "grant copy operation %s\n", - blkdev->feature_grant_copy ? "enabled" : "disabled"); - xen_pv_printf(&blkdev->xendev, 1, "ok: proto %s, ring-ref %d, " "remote port %d, local port %d\n", blkdev->xendev.protocol, blkdev->ring_ref, -- 1.9.1
[Qemu-devel] [PATCH 0/3] please pull xen-20170627-tag
The following changes since commit 577caa2672ccde7352fda3ef17e44993de862f0e: Merge remote-tracking branch 'remotes/edgar/tags/edgar/mmio-exec-v2.for-upstream' into staging (2017-06-27 16:56:55 +0100) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20170627-tag for you to fetch changes up to 3284fad7283596033cb810b4788fd1bb43312dbd: xen-disk: add support for multi-page shared rings (2017-06-27 15:01:56 -0700) Xen 2017/06/27 Paul Durrant (2): xen-disk: only advertize feature-persistent if grant copy is not available xen-disk: add support for multi-page shared rings Stefano Stabellini (1): xen/disk: don't leak stack data via response ring hw/block/xen_disk.c | 184 +--- 1 file changed, 133 insertions(+), 51 deletions(-)
[Qemu-devel] [PULL 1/3] xen/disk: don't leak stack data via response ring
Rather than constructing a local structure instance on the stack, fill the fields directly on the shared ring, just like other (Linux) backends do. Build on the fact that all response structure flavors are actually identical (aside from alignment and padding at the end). This is XSA-216. Reported by: Anthony Perard Signed-off-by: Jan Beulich Signed-off-by: Stefano Stabellini Acked-by: Anthony PERARD --- hw/block/xen_disk.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 3a22805..9200511 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -769,31 +769,30 @@ static int blk_send_response_one(struct ioreq *ioreq) struct XenBlkDev *blkdev = ioreq->blkdev; int send_notify = 0; int have_requests = 0; -blkif_response_t resp; -void *dst; - -resp.id= ioreq->req.id; -resp.operation = ioreq->req.operation; -resp.status= ioreq->status; +blkif_response_t *resp; /* Place on the response ring for the relevant domain. */ switch (blkdev->protocol) { case BLKIF_PROTOCOL_NATIVE: -dst = RING_GET_RESPONSE(&blkdev->rings.native, blkdev->rings.native.rsp_prod_pvt); +resp = (blkif_response_t *) RING_GET_RESPONSE(&blkdev->rings.native, + blkdev->rings.native.rsp_prod_pvt); break; case BLKIF_PROTOCOL_X86_32: -dst = RING_GET_RESPONSE(&blkdev->rings.x86_32_part, -blkdev->rings.x86_32_part.rsp_prod_pvt); +resp = (blkif_response_t *) RING_GET_RESPONSE(&blkdev->rings.x86_32_part, + blkdev->rings.x86_32_part.rsp_prod_pvt); break; case BLKIF_PROTOCOL_X86_64: -dst = RING_GET_RESPONSE(&blkdev->rings.x86_64_part, -blkdev->rings.x86_64_part.rsp_prod_pvt); +resp = (blkif_response_t *) RING_GET_RESPONSE(&blkdev->rings.x86_64_part, + blkdev->rings.x86_64_part.rsp_prod_pvt); break; default: -dst = NULL; return 0; } -memcpy(dst, &resp, sizeof(resp)); + +resp->id= ioreq->req.id; +resp->operation = ioreq->req.operation; +resp->status= ioreq->status; + blkdev->rings.common.rsp_prod_pvt++; RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&blkdev->rings.common, send_notify); -- 1.9.1
Re: [Qemu-devel] [RFC PATCH 00/14] Implement network booting directly into the s390-ccw BIOS
On 27.06.2017 23:15, Alexander Graf wrote: > > On 27.06.17 13:48, Thomas Huth wrote: [...] >> - The code only supports TFTP (via UDP) ... I think that is OK for >>most use-cases, but if we ever want to support network booting >>via HTTP or something else that is based on TCP, we would need to >>use something else instead... Should we maybe rather head towards >>grub2, petitboot or something different instead? > > IMHO the only viable next step would be to support UEFI and build on top > of that - either by porting edk2 or U-Boot to s390x. I can't speak of U-Boot, but you certainly don't want to port edk2 to a big endian machine. Been there, seen that. > The problem with solutions like petitboot or home grown grub2 targets is > that it becomes a documentation and knowledge sharing nightmare down the > road. The less we're different from everyone else, the easier it becomes > to maintain s390x. petitboot is basically a Linux system (with a shell, too). And grub2 is also a common boot loader on x86 nowadays ... so not sure whether they would really be a knowledge sharing nightmare? I think they would be more common for "server" people than the U-Boot loader, which is likely only familiar to "embedded" folks. Thomas
Re: [Qemu-devel] [RFC PATCH 00/14] Implement network booting directly into the s390-ccw BIOS
On 27.06.2017 17:50, Viktor Mihajlovski wrote: > On 27.06.2017 13:48, Thomas Huth wrote: [...] >> The patches are still in a rough shape, but before I continue here, >> I though I'd get some feedback first. Specifically: >> >> - This adds a lot of additional code to the s390-ccw firmware (and >> the binary is afterwards three times as big as before, 75k instead >> of 25k) ... is that still acceptable? > The size may be less of an issue than the general overhead for > initialization. Which concerns with regards to overhead do you have here? Startup time? As far as I can tell, that's not an issue here - the firmware still starts very fast. > Since the current approach of the s390-ccw firmware of > lazy loading (only if a network boot is requested) takes care of that, > would it be conceivable that you build a standalone network boot > firmware image instead of incorporating that into the base firmware? That would shoot down the big advantage of this approach: Not to have to fiddle with multiple firmware images - which is my main motivation here. I'd like to avoid that we have to create yet another firmware package in the distros. (Otherwise, what's the purpose of this when putting it into yet another binary? We could then simply continue the current approach with the Linux binary instead) >> - Is it OK to require loading an .INS file first? Or does anybody >> have a better idea how to load multiple files (kernel, initrd, >> etc. ...)? > It would be nice to support PXE-style boot, because the majority of boot > servers is set up that way. A straightforward way would be to do a PXE > emulation by attempting to download a pxelinux.cfg from the well-known > locations, parsing the content (menu) and finally load the kernel, > initrd and set the kernel command line as specified there. (I know, but > you're already parsing the INS-File). Please, don't mix up PXE and pxelinux (since you've used both terms in above paragraph). Assuming that you're only talking about pxlinux config files... are they that common on s390x already? Using the pxelinux config file syntax sounds like we would be completely bound to only loading Linux guests to me, since the boot loader has to know where to load the initrd and how to patch the kernel so that it can find the initrd. Using .INS files sounds more flexible to me instead, since you can also specify the addresses here - so you can theoretically also load other guest kernels, and that's IMHO the better approach since a firmware should stay as generic as possible. > Alternatively, one could load a single boot image (consisting of kernel > and initrd concatenated, i.e. the bootable ISO format). This could serve > as a more potent "stage 2" boot loader. Agreed, that's also a common practice when doing network booting. I guess the firmware could also support both quite easily, direct single boot images, and .INS files. The latter could be detected via the file name or with the magic string "* " at the beginning. >> - The code from SLOF uses a different coding style (TABs instead >> of space) ... is it OK to keep that coding style here so we >> can share patches between SLOF and s390-ccw more easil> >> - The code only supports TFTP (via UDP) ... I think that is OK for >> most use-cases, but if we ever want to support network booting >> via HTTP or something else that is based on TCP, we would need to >> use something else instead... Should we maybe rather head towards >> grub2, petitboot or something different instead? > I don't have an opinion on whether HTTP, FTP, etc is needed, but at some > point in time it would definitely be cool to have IPv6 support. Not sure > whether SLOF has that included. Yes, IPv6 is included in this networking stack. Thomas
Re: [Qemu-devel] [RFC 15/15] [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to catch NULL errp arguments
On Tue, Jun 27, 2017 at 03:12:25PM -0500, Eric Blake wrote: > On 06/19/2017 08:26 AM, Eduardo Habkost wrote: > > >> Is gcc's __attribute__((nonnull)) any better? It seems to apply > >> to the whole function prototype rather than an individual argument > >> though so probably not :-( > > > > It's possible to specify which arguments are non-null with > > nonnull(, ...). It's harder to use, but probably more > > Coccinelle-friendly. > > Libvirt uses it, wrapped in a macro; for example: > > int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) > virCPUDefCopyModel(virCPUDefPtr dst, >const virCPUDef *src, >bool resetPolicy) > { ... > > and at least Coverity is able to use that information (libvirt had > problems in the past where older gcc _silently_ mis-optimized a program > that used the attribute, and ended up having our macro defined only > during Coverity and not during normal compilation; but I think that's > finally been resolved now that newer gcc is saner in its behavior). I > don't know how much smarts Coccinelle has for coming up with the right > parameter number in an automated conversion patch, though. In the worst case, the script could have a dozen transformations, one transformation for each possible number of function arguments. However, I hit this Coccinelle bug when trying to implement that: https://github.com/coccinelle/coccinelle/issues/107 -- Eduardo
Re: [Qemu-devel] [RFC PATCH 00/14] Implement network booting directly into the s390-ccw BIOS
On 27.06.17 13:48, Thomas Huth wrote: It's already possible to do a network boot of an s390x guest with an external netboot image (based on a Linux installation), but it would be much more convenient if the s390-ccw firmware supported network booting right out of the box, without the need to assemble such an external image first. This patch series now introduces network booting via DHCP and TFTP directly into the s390-ccw firmware by re-using the networking stack from the SLOF firmware (see https://github.com/aik/SLOF/ for details), and adds a driver for virtio-net-ccw devices. Once the patches have been applied, you can download an .INS file via TFTP which contains the information about the further files that have to be loaded - kernel, initrd, etc. For example, you can use the built-in TFTP and DHCP server of QEMU for this by starting QEMU with: qemu-system-s390x ... -device virtio-net,netdev=n1,bootindex=1 \ -netdev user,id=n1,tftp=/path/to/tftp,bootfile=generic.ins The .INS file has to use the same syntax as the .INS files that can already be found on s390x Linux distribution installation CD-ROMs. The patches are still in a rough shape, but before I continue here, I though I'd get some feedback first. Specifically: - This adds a lot of additional code to the s390-ccw firmware (and the binary is afterwards three times as big as before, 75k instead of 25k) ... is that still acceptable? I think 75k is still perfectly reasonable, yes. - Is it OK to require loading an .INS file first? Or does anybody have a better idea how to load multiple files (kernel, initrd, etc. ...)? - The code from SLOF uses a different coding style (TABs instead of space) ... is it OK to keep that coding style here so we can share patches between SLOF and s390-ccw more easily? - The code only supports TFTP (via UDP) ... I think that is OK for most use-cases, but if we ever want to support network booting via HTTP or something else that is based on TCP, we would need to use something else instead... Should we maybe rather head towards grub2, petitboot or something different instead? IMHO the only viable next step would be to support UEFI and build on top of that - either by porting edk2 or U-Boot to s390x. The problem with solutions like petitboot or home grown grub2 targets is that it becomes a documentation and knowledge sharing nightmare down the road. The less we're different from everyone else, the easier it becomes to maintain s390x. Alex
Re: [Qemu-devel] [PATCH] qga:windows os lost ip when network nic change pic order
On 06/24/2017 12:39 AM, indiff...@126.com wrote: > From: "yin.zuowei" > > Signed-off-by: yin.zuowei > > bug description: In the windows virtual machine, if there are multiple > network cards, the hypothesis is that A/B/C is equipped with a different IP > address. Once you delete a B card in the middle and restart the virtual > machine, you will find that the A/C of the network card IP has been confused, > and the IP of the C network card has become the address of B, and the service > has been interrupted. So we did a IP recovery function in qga.This is a > serious problem that can lead to business disruption. If you have a better > plan, we would like to offer it to you. > --- > qga/restoreIp.cpp | 1848 > + > 1 file changed, 1848 insertions(+) > create mode 100755 qga/restoreIp.cpp > > diff --git a/qga/restoreIp.cpp b/qga/restoreIp.cpp > new file mode 100755 > index 000..2382136 > --- /dev/null > +++ b/qga/restoreIp.cpp > @@ -0,0 +1,1848 @@ > +// restoreIp.cpp : ���̨Ӧ�ó��ڵ㡣 Your mail header said: > Content-Type: text/plain; charset=yes which looks like a bug with your git settings (typically, it should be charset=UTF-8). Much of the message didn't render for me (not sure if it lack of fonts on my end, or if it really was garbled en route rather than being valid UTF-8 round-trip). But we much prefer /* */ comments over // (even in C++ code), and prefer those comments to be in English. > +//#pragma comment(lib,"iphlpapi.lib") Dead code? > + > +/*Ҫע�⣬unicode�� memcpy��Ҫʹ��XXX_LEN * sizeof(TCHAR)*/ More encoding awkwardness. It's hard to review the patch when the content is difficult to read. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning
On Tue, Jun 27, 2017 at 01:45:48PM -0700, Alistair Francis wrote: > When QEMU is waiting for a TCP socket connection it reports that message as > an error. This isn't an error it is a warnign so let's change the report to > use warning_report() instead. Hi Alistair, Isn't this more like an informational message rather than a warning? A warning would indicate to me that something unexpected and perhaps wrong happened. In this case, there's nothing wrong going on. We may need more classes, like 'info:' and/or maybe others... Cheers, Edgar > > Signed-off-by: Alistair Francis > --- > > chardev/char-socket.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index ccc499cfa1..5a56628e74 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error > **errp) > * in TLS and telnet cases, only wait for an accepted socket */ > while (!s->ioc) { > if (s->is_listen) { > -error_report("QEMU waiting for connection on: %s", > +warning_report("QEMU waiting for connection on: %s", > chr->filename); > qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); > tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); > -- > 2.11.0 > >
Re: [Qemu-devel] [PATCH v3 2/7] target/m68k: add fmovecr
On 06/27/2017 04:12 PM, Laurent Vivier wrote: fmovecr moves a floating point constant from the FPU ROM to a floating point register. Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/fpu_helper.c | 30 ++ target/m68k/helper.h | 1 + target/m68k/translate.c | 13 - 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index a9e17f5..912c0b7 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -23,6 +23,31 @@ #include "exec/helper-proto.h" #include "exec/exec-all.h" +static const floatx80 fpu_rom[128] = { "The values contained at offsets other than those defined above are reserved for the use of Motorola and may be different on various mask sets of the floating-point coprocessor. These undefined values yield the value 0.0 [ floatx80_zero ] in the M68040FPSP." ^ with a such comment around: Reviewed-by: Philippe Mathieu-Daudé +[0x00] = floatx80_pi, /* Pi */ +[0x0b] = make_floatx80(0x3ffd, 0x9a209a84fbcff798ULL), /* Log10(2) */ +[0x0c] = make_floatx80(0x4000, 0xadf85458a2bb4a9aULL), /* e*/ +[0x0d] = make_floatx80(0x3fff, 0xb8aa3b295c17f0bcULL), /* Log2(e) */ +[0x0e] = make_floatx80(0x3ffd, 0xde5bd8a937287195ULL), /* Log10(e) */ +[0x0f] = floatx80_zero, /* Zero */ +[0x30] = floatx80_ln2, /* ln(2)*/ +[0x31] = make_floatx80(0x4000, 0x935d8dddaaa8ac17ULL), /* ln(10) */ +[0x32] = floatx80_one, /* 10^0 */ +[0x33] = make_floatx80(0x4002, 0xa000ULL), /* 10^1 */ +[0x34] = make_floatx80(0x4005, 0xc800ULL), /* 10^2 */ +[0x35] = make_floatx80(0x400c, 0x9c40ULL), /* 10^4 */ +[0x36] = make_floatx80(0x4019, 0xbebc2000ULL), /* 10^8 */ +[0x37] = make_floatx80(0x4034, 0x8e1bc9bf0400ULL), /* 10^16*/ +[0x38] = make_floatx80(0x4069, 0x9dc5ada82b70b59eULL), /* 10^32*/ +[0x39] = make_floatx80(0x40d3, 0xc2781f49ffcfa6d5ULL), /* 10^64*/ +[0x3a] = make_floatx80(0x41a8, 0x93ba47c980e98ce0ULL), /* 10^128 */ +[0x3b] = make_floatx80(0x4351, 0xaa7eebfb9df9de8eULL), /* 10^256 */ +[0x3c] = make_floatx80(0x46a3, 0xe319a0aea60e91c7ULL), /* 10^512 */ +[0x3d] = make_floatx80(0x4d48, 0xc976758681750c17ULL), /* 10^1024 */ +[0x3e] = make_floatx80(0x5a92, 0x9e8b3b5dc53d5de5ULL), /* 10^2048 */ +[0x3f] = make_floatx80(0x7525, 0xc46052028a20979bULL), /* 10^4096 */ +}; + int32_t HELPER(reds32)(CPUM68KState *env, FPReg *val) { return floatx80_to_int32(val->d, &env->fp_status); @@ -204,3 +229,8 @@ void HELPER(ftst)(CPUM68KState *env, FPReg *val) } env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | cc; } + +void HELPER(fconst)(CPUM68KState *env, FPReg *val, uint32_t offset) +{ +val->d = fpu_rom[offset]; +} diff --git a/target/m68k/helper.h b/target/m68k/helper.h index 98cbf18..d6e80e4 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -35,6 +35,7 @@ DEF_HELPER_4(fdiv, void, env, fp, fp, fp) DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp) +DEF_HELPER_3(fconst, void, env, fp, i32) DEF_HELPER_3(mac_move, void, env, i32, i32) DEF_HELPER_3(macmulf, i64, env, i32, i32) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index dff604c..0bb3300 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4518,10 +4518,21 @@ DISAS_INSN(fpu) ext = read_im16(env, s); opmode = ext & 0x7f; switch ((ext >> 13) & 7) { -case 0: case 2: +case 0: break; case 1: goto undef; +case 2: +if (insn == 0xf200 && (ext & 0xfc00) == 0x5c00) { +/* fmovecr */ +TCGv rom_offset = tcg_const_i32(opmode); you could reuse tmp32: tmp32 = tcg_const_i32(opmode); /* rom offset */ but it's good like that ;) +cpu_dest = gen_fp_ptr(REG(ext, 7)); +gen_helper_fconst(cpu_env, cpu_dest, rom_offset); +tcg_temp_free_ptr(cpu_dest); +tcg_temp_free(rom_offset); Oh this was a leak in v2? I didn't notice. +return; +} +break; case 3: /* fmove out */ cpu_src = gen_fp_ptr(REG(ext, 7)); opsize = ext_opsize(ext, 10);
Re: [Qemu-devel] [PATCH v2 2/7] target/m68k: add fmovecr
On 06/27/2017 02:58 PM, Laurent Vivier wrote: Le 27/06/2017 à 17:45, Philippe Mathieu-Daudé a écrit : +static const floatx80 fpu_rom[128] = { +[0x00] = floatx80_pi, /* Pi */ +[0x0b] = make_floatx80(0x3ffd, 0x9a209a84fbcff798ULL), /* Log10(2) */ +[0x0c] = make_floatx80(0x4000, 0xadf85458a2bb4a9aULL), /* e*/ +[0x0d] = make_floatx80(0x3fff, 0xb8aa3b295c17f0bcULL), /* Log2(e) */ +[0x0e] = make_floatx80(0x3ffd, 0xde5bd8a937287195ULL), /* Log10(e) */ +[0x0f] = floatx80_zero, /* Zero */ +[0x30] = floatx80_ln2, /* ln(2)*/ +[0x31] = make_floatx80(0x4000, 0x935d8dddaaa8ac17ULL), /* ln(10) */ +[0x32] = floatx80_one, /* 10^0 */ +[0x33] = make_floatx80(0x4002, 0xa000ULL), /* 10^1 */ +[0x34] = make_floatx80(0x4005, 0xc800ULL), /* 10^2 */ +[0x35] = make_floatx80(0x400c, 0x9c40ULL), /* 10^4 */ +[0x36] = make_floatx80(0x4019, 0xbebc2000ULL), /* 10^8 */ +[0x37] = make_floatx80(0x4034, 0x8e1bc9bf0400ULL), /* 10^16*/ +[0x38] = make_floatx80(0x4069, 0x9dc5ada82b70b59eULL), /* 10^32*/ +[0x39] = make_floatx80(0x40d3, 0xc2781f49ffcfa6d5ULL), /* 10^64*/ +[0x3a] = make_floatx80(0x41a8, 0x93ba47c980e98ce0ULL), /* 10^128 */ +[0x3b] = make_floatx80(0x4351, 0xaa7eebfb9df9de8eULL), /* 10^256 */ +[0x3c] = make_floatx80(0x46a3, 0xe319a0aea60e91c7ULL), /* 10^512 */ +[0x3d] = make_floatx80(0x4d48, 0xc976758681750c17ULL), /* 10^1024 */ +[0x3e] = make_floatx80(0x5a92, 0x9e8b3b5dc53d5de5ULL), /* 10^2048 */ +[0x3f] = make_floatx80(0x7525, 0xc46052028a20979bULL), /* 10^4096 */ +}; + int32_t HELPER(reds32)(CPUM68KState *env, FPReg *val) { return floatx80_to_int32(val->d, &env->fp_status); @@ -204,3 +229,8 @@ void HELPER(ftst)(CPUM68KState *env, FPReg *val) } env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | cc; } + +void HELPER(fconst)(CPUM68KState *env, FPReg *val, uint32_t offset) +{ +val->d = fpu_rom[offset]; For offset not declared in fpu_rom (0x1..0xa, 0x10..0x2f, 0x40..0x7f), this will return floatx80_zero, is this correct? yes, according to the doc: The values contained at offsets other than those defined above are reserved for the use of Motorola and may be different on various mask sets of the floating-point coprocessor. These undefined values yield the value 0.0 in the M68040FPSP can you add this comment before/in the fpu_rom array please?
[Qemu-devel] [RFC v1 3/3] char-socket: Report TCP socket waiting as a warning
When QEMU is waiting for a TCP socket connection it reports that message as an error. This isn't an error it is a warnign so let's change the report to use warning_report() instead. Signed-off-by: Alistair Francis --- chardev/char-socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/char-socket.c b/chardev/char-socket.c index ccc499cfa1..5a56628e74 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -765,7 +765,7 @@ static int tcp_chr_wait_connected(Chardev *chr, Error **errp) * in TLS and telnet cases, only wait for an accepted socket */ while (!s->ioc) { if (s->is_listen) { -error_report("QEMU waiting for connection on: %s", +warning_report("QEMU waiting for connection on: %s", chr->filename); qio_channel_set_blocking(QIO_CHANNEL(s->listen_ioc), true, NULL); tcp_chr_accept(QIO_CHANNEL(s->listen_ioc), G_IO_IN, chr); -- 2.11.0
[Qemu-devel] [RFC v1 1/3] util/qemu-error: Rename error_print_loc() to be more generic
Rename the error_print_loc() function in preperation for using it to print warnings as well. Signed-off-by: Alistair Francis --- util/qemu-error.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/qemu-error.c b/util/qemu-error.c index b331f8f4a4..1c5e35ecdb 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -146,7 +146,7 @@ const char *error_get_progname(void) /* * Print current location to current monitor if we have one, else to stderr. */ -static void error_print_loc(void) +static void print_loc(void) { const char *sep = ""; int i; @@ -197,7 +197,7 @@ void error_vreport(const char *fmt, va_list ap) g_free(timestr); } -error_print_loc(); +print_loc(); error_vprintf(fmt, ap); error_printf("\n"); } -- 2.11.0
[Qemu-devel] [RFC v1 2/3] util/qemu-error: Add a warning_report() function
Add a functino which can be used similarly to error_report() execpt to inform the users about warnings instead of errors. The warning print does not include the timestamp and instead will preface the messages with a 'warning: '. Signed-off-by: Alistair Francis --- include/qemu/error-report.h | 2 ++ util/qemu-error.c | 32 2 files changed, 34 insertions(+) diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h index 3001865896..c2600d2298 100644 --- a/include/qemu/error-report.h +++ b/include/qemu/error-report.h @@ -36,7 +36,9 @@ void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2); void error_set_progname(const char *argv0); void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); +void warning_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0); void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); +void warning_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2); const char *error_get_progname(void); extern bool enable_timestamp_msg; diff --git a/util/qemu-error.c b/util/qemu-error.c index 1c5e35ecdb..2edd752fec 100644 --- a/util/qemu-error.c +++ b/util/qemu-error.c @@ -203,6 +203,23 @@ void error_vreport(const char *fmt, va_list ap) } /* + * Print a warning message ot the current monitor if we have one, else to + * stderr. This follows similar formating and use cases as error_vreport() + * except for these two differentce: + * - It prefixes the message with 'warning: ' to indicate it is only a + * warning. + * - It does not print the timestamp. + */ +void warning_vreport(const char *fmt, va_list ap) +{ +error_vprintf("warning: ", ap); + +print_loc(); +error_vprintf(fmt, ap); +error_printf("\n"); +} + +/* * Print an error message to current monitor if we have one, else to stderr. * Format arguments like sprintf(). The resulting message should be a * single phrase, with no newline or trailing punctuation. @@ -217,3 +234,18 @@ void error_report(const char *fmt, ...) error_vreport(fmt, ap); va_end(ap); } + +/* + * Print an warning message to current monitor if we have one, else to stderr. + * This follows the same formating and use cases as error_report() + * except it prefixes the message with 'warning: ' to indicate it is only a + * warning. + */ +void warning_report(const char *fmt, ...) +{ +va_list ap; + +va_start(ap, fmt); +warning_vreport(fmt, ap); +va_end(ap); +} -- 2.11.0
[Qemu-devel] [RFC v1 0/3] Implement a warning_report function
QEMU currently has a standard method to report errors with error_repot(). This ensure a sane and standard format when printing errors. This series is attempting to add the same functionality for warnings. At the moment only one error is being converted, I wanted to get the implementation nailed down a little bit before I started converting others. This patch renames error_print_loc() function to be more clear, but I didn't bother renaming the others. It seems silly to change error_printf() to error_warning_printf() and printf is already taken so I just left it as is. I also didn't change the current error output as that would probably break backwards compatibilty for anyone who is parsing logs. Alistair Francis (3): util/qemu-error: Rename error_print_loc() to be more generic util/qemu-error: Add a warning_report() function char-socket: Report TCP socket waiting as a warning chardev/char-socket.c | 2 +- include/qemu/error-report.h | 2 ++ util/qemu-error.c | 36 ++-- 3 files changed, 37 insertions(+), 3 deletions(-) -- 2.11.0
[Qemu-devel] SPARC64 supported processors
Hi, I am trying to evaluate the current qemu support for sparc64 processors. First, it seems -smp is not supported for any processor, is this correct? When I set -smp greater than 1, I am getting: qemu-system-sparc64: Number of SMP CPUs requested (2) exceeds max CPUs supported by machine 'sun4u' (1) I've done some testing for all available sparc64 cpus + latest linux kernel: Fujitsu Sparc64 Working Fujitsu Sparc64 III Exception 0x30 (DAE_side_effect_page) in OpenBios Fujitsu Sparc64 IV Working Fujitsu Sparc64 V Working TI UltraSparc I Working TI UltraSparc IIWorking TI UltraSparc IIi Working TI UltraSparc IIe Exception 0x28 (division_by_zero) in init_tick_ops Can make it to work if is_hummingbird() is changed to return 0. The IO stick, and OpenBios stick properties are absent, so we have to default to %tick for now. Sun UltraSparc III Illegal instruction in cheetah_boot(): wr %g0, %g1, %dcr It appears dispatch control register is not implemented. Sun UltraSparc IIIi Sun UltraSparc IV Sun UltraSparc IV+ Sun UltraSparc IIIi+ In these four CPUs, I am getting exception 0x32 in cheetah_generic_boot: stxa %g0, [ %g3 ] #ASI_DMMU Sun UltraSparc T1 Sun UltraSparc T2 Both of the above boot pretty far but fail in this function when tmpfs is mounted: direct_pcr_write(unsigned long reg_num, u64 val) __asm__ __volatile__("wr %0, 0x0, %%pcr" : : "r" (val)); Seems like performance counter registers are not supported. needed to add these to kernel parameters: keep_bootcon -> to see where we are panicking lpj=1000 -> jiffers could not calculate for some reason. NEC UltraSparc IWorking Does this look right or may be I have missed something, and we can get some of the Sun UltraSparc to work for example? Thank you, Pasha
Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored
On 06/13/2017 11:52 AM, Eduardo Habkost wrote: > The Proposal > > > I'm proposing replacing NULL errp with a special macro: > IGNORE_ERRORS. The macro will trigger special behavior in the > error API that will make it not save any error information in the > error pointer, but still keep track of boolean error state in > *errp. > > This will allow us to simplify the documented method to propagate errors > from: > > Error *err = NULL; > foo(arg, &err); > if (err) { > handle the error... > error_propagate(errp, err); > } > > to: > > foo(arg, errp); > if (ERR_IS_SET(errp)) { > handle the error... > } Seems kind of neat to me! > > This way, we can make ERR_IS_SET work even if errp was > IGNORE_ERRORS. The ERR_IS_* macros are reimplemented as: > > #define ERR_IS_SET(e) (*(e) && *(e) != &ignored_error_unset) > #define ERR_IS_IGNORED(e) (!(e) || *(e) == &ignored_error_unset || *(e) == > &ignored_error_set) Where the initial NULL checks go away if you get your way with a final patch. > > Ensuring errp is never NULL > --- > > The last patch on this series changes the (Error **errp) > parameters in functions to (Error *errp[static 1]), just to help > validate the existing code, as clang warns about NULL arguments > on that case. I don't think we should apply that patch, though, > because the "[static 1]" syntax confuses Coccinelle. Have you filed a bug report to the Coccinelle folks? But yeah, it is rather arcane C99 syntax that you don't see much of in common code. > (Probably the easiest solution for that is to add assert(errp) > lines to the ERR_IS_*() macros.) Or even having the macros use a forced dereference through errp->xxx may at least be enough for Coverity to catch things. > Git branch > -- > > This series depend on a few extra cleanups that I didn't submit > to qemu-devel yet. A git branch including this series is > available at: > > git://github.com/ehabkost/qemu-hacks.git work/err-api-rework-ignore-ptr-v1 > > Eduardo Habkost (15): > tests: Test cases for error API > error: New IGNORE_ERRORS macro > Add qapi/error.h includes on files that will need it > [coccinelle] Use IGNORE_ERRORS instead of NULL as errp argument > qapi: Use IGNORE_ERRORS instead of NULL on generated code > test-qapi-util: Use IGNORE_ERRORS instead of NULL > Manual changes to use IGNORE_ERRORS instead of NULL > error: New ERR_IS_* macros for checking Error** values > [coccinelle] Use ERR_IS_* macros > test-qapi-util: Use ERR_IS_* macros > Manual changes to use ERR_IS_* macros > error: Make IGNORED_ERRORS not a NULL pointer > rdma: Simplify var declaration to avoid confusing Coccinelle > [coccinelle] Eliminate unnecessary local_err/error_propagate() usage > [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to > catch NULL errp arguments I have not reviewed the series yet, but the idea looks promising. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 4/7] softfloat: define floatx80_round()
On 2017-06-27 21:12, Laurent Vivier wrote: > Add a function to round a floatx80 to the defined precision > (floatx80_rounding_precision) > > Signed-off-by: Laurent Vivier > Reviewed-by: Richard Henderson > --- > fpu/softfloat.c | 15 +++ > include/fpu/softfloat.h | 1 + > 2 files changed, 16 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index 7af14e2..e9bf359 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -5086,6 +5086,21 @@ float128 floatx80_to_float128(floatx80 a, float_status > *status) > } > > > /* > +| Rounds the extended double-precision floating-point value `a' Maybe it is worth mentioning the precision to which the value is rounded (floatx80_rounding_precision) ? Otherwise looks all fine to me. Reviewed-by: Aurelien Jarno -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [RFC 15/15] [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to catch NULL errp arguments
On 06/19/2017 08:26 AM, Eduardo Habkost wrote: >> Is gcc's __attribute__((nonnull)) any better? It seems to apply >> to the whole function prototype rather than an individual argument >> though so probably not :-( > > It's possible to specify which arguments are non-null with > nonnull(, ...). It's harder to use, but probably more > Coccinelle-friendly. Libvirt uses it, wrapped in a macro; for example: int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) virCPUDefCopyModel(virCPUDefPtr dst, const virCPUDef *src, bool resetPolicy) { ... and at least Coverity is able to use that information (libvirt had problems in the past where older gcc _silently_ mis-optimized a program that used the attribute, and ended up having our macro defined only during Coverity and not during normal compilation; but I think that's finally been resolved now that newer gcc is saner in its behavior). I don't know how much smarts Coccinelle has for coming up with the right parameter number in an automated conversion patch, though. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] vcpu_dirty: share the same field in CPUState for all accelerators
On 27/06/2017 20:36, Eric Blake wrote: > On 06/19/2017 11:35 AM, Paolo Bonzini wrote: >> >> >> On 18/06/2017 21:11, Sergio Andres Gomez Del Real wrote: >>> This patch simply replaces the separate boolean field in CPUState that >>> kvm, hax (and upcoming hvf) have for keeping track of vcpu dirtiness >>> with a single shared field. >>> >>> Signed-off-by: Sergio Andres Gomez Del Real >>> --- > >>> >> >> Thanks, I queued this patch for 2.9. > > As in qemu-stable 2.9.1, or did you mean 2.10 mainline? 2.10 of course. Paolo
Re: [Qemu-devel] [PATCH v3 5/7] target/m68k: add fsglmul and fsgldiv
On 06/27/2017 12:12 PM, Laurent Vivier wrote: fsglmul and fsgldiv truncate data to single precision before computing results. Signed-off-by: Laurent Vivier --- target/m68k/fpu_helper.c | 28 target/m68k/helper.h | 2 ++ target/m68k/translate.c | 6 ++ 3 files changed, 36 insertions(+) Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 1/1] Add support for custom fmasks/dmasks in 9ps mapped mode
On 06/18/2017 03:28 AM, Tobias Schramm wrote: > Signed-off-by: Tobias Schramm > --- > fsdev/file-op-9p.h | 4 > fsdev/qemu-fsdev-opts.c | 12 > hw/9pfs/9p-local.c | 33 + > hw/9pfs/9p.c| 3 +++ > 4 files changed, 48 insertions(+), 4 deletions(-) > > diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h > index 0844a403dc..0ecb1d392b 100644 > --- a/fsdev/file-op-9p.h > +++ b/fsdev/file-op-9p.h > @@ -76,6 +76,8 @@ typedef struct FsDriverEntry { > int export_flags; > FileOperations *ops; > FsThrottle fst; > +mode_t fmask; > +mode_t dmask; > } FsDriverEntry; > > +++ b/fsdev/qemu-fsdev-opts.c > @@ -38,6 +38,12 @@ static QemuOptsList qemu_fsdev_opts = { > }, { > .name = "sock_fd", > .type = QEMU_OPT_NUMBER, > +}, { > +.name = "fmask", > +.type = QEMU_OPT_STRING, > +}, { > +.name = "dmask", > +.type = QEMU_OPT_STRING, No documentation of what these represent (other than the cover letter). I'd at least expect a comment of 'default creation mask for files' and 'default creation mask for directories'; it would also be nice to state if the mask is positive or negative logic (compare the third argument passed to open() which specifies bits to set, vs. the argument to umask which specifies bits to keep clear). Also, does this need a QAPI counterpart (or can you even create a 9pfs device through QMP)? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 1/7] target/m68k: add fscc.
On 06/27/2017 12:12 PM, Laurent Vivier wrote: case 3: /* Ordered Greater than or Equal Z || !(A || N) */ case 19: /* Greater than or Equal Z || !(A || N) */ +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_shli_i32(c->v1, fpsr, 3); +tcg_gen_or_i32(c->v1, c->v1, fpsr); +tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N); +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_Z); +c->tcond = TCG_COND_NE; Still with the unmasked shift. tcg_gen_not_i32(c->v1, fpsr); tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_A | FPSR_CC_N); tcg_gen_andi_i32(fpsr, fpsr, FPSR_CC_Z); tcg_gen_or_i32(c->v1, c->v1, fpsr); case 5: /* Ordered Less than or Equal Z || (N && !A) */ case 21: /* Less than or Equal Z || (N && !A) */ +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_xori_i32(c->v1, fpsr, FPSR_CC_A); +tcg_gen_shli_i32(c->v1, c->v1, 3); +tcg_gen_ori_i32(c->v1, c->v1, FPSR_CC_Z); +tcg_gen_and_i32(c->v1, c->v1, fpsr); +c->tcond = TCG_COND_NE; Likewise. tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A); tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - ctz32(FPSR_CC_A)); tcg_gen_andc_i32(c->v1, fpsr, c->v1); tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_Z | FPSR_CC_N); case 10: /* Unordered or Greater Than A || !(N || Z)) */ case 26: /* Not Less or Equal A || !(N || Z)) */ +g_assert(FPSR_CC_Z == (FPSR_CC_N >> 1)); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_shli_i32(c->v1, fpsr, 1); +tcg_gen_or_i32(c->v1, c->v1, fpsr); +tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N); +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_A); +c->tcond = TCG_COND_NE; Likewise. tcg_gen_not_i32(c->v1, fpsr); tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_Z | FPSR_CC_N); tcg_gen_andi_i32(fpsr, fpsr, FPSR_CC_A); tcg_gen_or_i32(c->v1, c->v1, fpsr); case 12: /* Unordered or Less Than A || (N && !Z) */ case 28: /* Not Greater than or Equal A || (N && !Z) */ +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_Z); +tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - ctz32(FPSR_CC_Z)); +tcg_gen_andc_i32(c->v1, fpsr, c->v1); +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_A | FPSR_CC_N); +c->tcond = TCG_COND_NE; I hadn't meant that this was the only one to fix. r~
Re: [Qemu-devel] [PATCH] iotests: Add test for failing qemu-img commit
On 06/16/2017 08:58 AM, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > In order to pass, this depends on "fix: avoid an infinite loop or a > dangling pointer problem in img_commit" > (http://lists.nongnu.org/archive/html/qemu-block/2017-06/msg00443.html) Looks like that is now 4172a003 > and on the "block: Don't compare strings in bdrv_reopen_prepare()" > series > (http://lists.nongnu.org/archive/html/qemu-block/2017-06/msg00424.html). Still pending. > --- > tests/qemu-iotests/020 | 27 +++ > tests/qemu-iotests/020.out | 17 + > 2 files changed, 44 insertions(+) > > diff --git a/tests/qemu-iotests/020 b/tests/qemu-iotests/020 > index 7a0..83d5ef0 100755 > --- a/tests/qemu-iotests/020 > +++ b/tests/qemu-iotests/020 > @@ -110,6 +110,33 @@ for offset in $TEST_OFFSETS; do > io_zero readv $(( offset + 64 * 1024 + 65536 * 4 )) 65536 65536 1 > done > _check_test_img > +_cleanup > +TEST_IMG=$TEST_IMG_SAVE > + > +echo > +echo 'Testing failing commit' > +echo > + > +# Create an image with a null backing file to which committing will fail > (with > +# ENOSPC so we can distinguish the result from some generic EIO which may be > +# generated anywhere in the block layer) > +_make_test_img -b "json:{'driver': 'raw', > + 'file': { > + 'driver': 'blkdebug', > + 'inject-error': [{ > + 'event': 'write_aio', > + 'errno': 28, Still nasty that we encoded an errno value rather than using a stringified enum, but that's not your problem. > + 'once': true > + }], > + 'image': { > + 'driver': 'null-co' > + }}}" The comment does wonders for explaining how it all works! > + > +# Just write anything so comitting will not be a no-op s/comitting/committing/ With the typo fix, Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] nbd: fix NBD over TLS
On 06/15/2017 02:51 PM, Paolo Bonzini wrote: > When attaching the NBD QIOChannel to an AioContext, the TLS channel should > be used, not the underlying socket channel. This is because, trivially, > the TLS channel will be the one that we read/write to and thus the one > that will get the qio_channel_yield() call. > > Fixes: ff82911cd3f69f028f2537825c9720ff78bc3f19 > Cc: qemu-sta...@nongnu.org > Cc: Daniel P. Berrange > Signed-off-by: Paolo Bonzini > --- > block/nbd-client.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] Add chardev-send-break monitor command
On 06/11/2017 02:48 AM, Stefan Fritsch wrote: > Sending a break on a serial console can be useful for debugging the > guest. But not all chardev backends support sending breaks (only telnet > and mux do). The chardev-send-break command allows to send a break even > if using other backends. > > Signed-off-by: Stefan Fritsch > Acked-by: Dr. David Alan Gilbert > --- > v2: added tests and Acked-by line > > +++ b/hmp-commands.hx > @@ -1745,6 +1745,22 @@ Removes the chardev @var{id}. > ETEXI > > { > +.name = "chardev-send-break", > +.args_type = "id:s", > +.params = "id", > +.help = "send break on chardev", Compare this wording,... > +STEXI > +@item chardev-send-break id > +@findex chardev-send-break > +Sends break on the chardev @var{id}. repeated here, > +++ b/qapi-schema.json > @@ -5114,6 +5114,26 @@ > { 'command': 'chardev-remove', 'data': {'id': 'str'} } > > ## > +# @chardev-send-break: > +# > +# Send a break to a character device ...with this wording. I like 'send a break' better than 'send break', but even better might be 'send a break sequence' or even 'emulate a break sequence' (by definition, a break is NOT a character, but on bare metal character devices it IS a defined electrical sequence distinct from characters to make the recipient aware that the sender is trying to get attention). Otherwise, the patch looks fine to me. If all that changes is some word-smithing (and the maintainer may be willing to do that), you can add: Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC 5/5] vifo: introduce new VFIO ioctl VFIO_DEVICE_PCI_GET_DIRTY_BITMAP
On Tue, 27 Jun 2017 08:56:01 + "Zhang, Yulei" wrote: > > -Original Message- > > From: Qemu-devel [mailto:qemu-devel- > > bounces+yulei.zhang=intel@nongnu.org] On Behalf Of Alex Williamson > > Sent: Tuesday, June 27, 2017 4:19 AM > > To: Zhang, Yulei > > Cc: Tian, Kevin ; joonas.lahti...@linux.intel.com; > > qemu-devel@nongnu.org; zhen...@linux.intel.com; Zheng, Xiao > > ; Wang, Zhi A > > Subject: Re: [Qemu-devel] [RFC 5/5] vifo: introduce new VFIO ioctl > > VFIO_DEVICE_PCI_GET_DIRTY_BITMAP > > > > On Tue, 4 Apr 2017 18:28:04 +0800 > > Yulei Zhang wrote: > > > > > New VFIO ioctl VFIO_DEVICE_PCI_GET_DIRTY_BITMAP is used to sync the > > > pci device dirty pages during the migration. > > > > If this needs to exist, it needs a lot more documentation. Why is this > > a PCI specific device ioctl? Couldn't any vfio device need this? > > > > > Signed-off-by: Yulei Zhang > > > --- > > > hw/vfio/pci.c | 32 > > > hw/vfio/pci.h | 2 ++ > > > linux-headers/linux/vfio.h | 14 ++ > > > 3 files changed, 48 insertions(+) > > > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > > > index 833cd90..64c851f 100644 > > > --- a/hw/vfio/pci.c > > > +++ b/hw/vfio/pci.c > > > @@ -32,6 +32,7 @@ > > > #include "pci.h" > > > #include "trace.h" > > > #include "qapi/error.h" > > > +#include "exec/ram_addr.h" > > > > > > #define MSIX_CAP_LENGTH 12 > > > > > > @@ -39,6 +40,7 @@ static void vfio_disable_interrupts(VFIOPCIDevice > > *vdev); > > > static void vfio_mmap_set_enabled(VFIOPCIDevice *vdev, bool enabled); > > > static VMStateDescription vfio_pci_vmstate; > > > static void vfio_vm_change_state_handler(void *pv, int running, RunState > > > > > state); > > > +static void vfio_log_sync(MemoryListener *listener, > > MemoryRegionSection *section); > > > > > > /* > > > * Disabling BAR mmaping can be slow, but toggling it around INTx can > > > @@ -2869,6 +2871,11 @@ static void vfio_realize(PCIDevice *pdev, Error > > **errp) > > > vfio_setup_resetfn_quirk(vdev); > > > qemu_add_vm_change_state_handler(vfio_vm_change_state_handler, > > vdev); > > > > > > +vdev->vfio_memory_listener = (MemoryListener) { > > > + .log_sync = vfio_log_sync, > > > +}; > > > +memory_listener_register(&vdev->vfio_memory_listener, > > &address_space_memory); > > > + > > > return; > > > > > > out_teardown: > > > @@ -2964,6 +2971,7 @@ static void vfio_vm_change_state_handler(void > > *pv, int running, RunState state) > > > if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_STATUS_SET, > > > vfio_status)) > > { > > > error_report("vfio: Failed to %s device\n", running ? "start" : > > > "stop"); > > > } > > > +vdev->device_stop = running ? false : true; > > > g_free(vfio_status); > > > } > > > > > > @@ -3079,6 +3087,30 @@ static int vfio_device_get(QEMUFile *f, void *pv, > > size_t size, VMStateField *fie > > > return 0; > > > } > > > > > > +static void vfio_log_sync(MemoryListener *listener, > > MemoryRegionSection *section) > > > +{ > > > +VFIOPCIDevice *vdev = container_of(listener, struct VFIOPCIDevice, > > vfio_memory_listener); > > > + > > > +if (vdev->device_stop) { > > > +struct vfio_pci_get_dirty_bitmap *d; > > > +ram_addr_t size = int128_get64(section->size); > > > +unsigned long page_nr = size >> TARGET_PAGE_BITS; > > > +unsigned long bitmap_size = (BITS_TO_LONGS(page_nr) + 1) * > > sizeof(unsigned long); > > > +d = g_malloc0(sizeof(*d) + bitmap_size); > > > +d->start_addr = section->offset_within_address_space; > > > +d->page_nr = page_nr; > > > + > > > +if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_GET_DIRTY_BITMAP, > > > d)) > > { > > > +error_report("vfio: Failed to fetch dirty pages for > > > migration\n"); > > > +goto exit; > > > +} > > > +cpu_physical_memory_set_dirty_lebitmap((unsigned long*)&d- > > >dirty_bitmap, d->start_addr, d->page_nr); > > > + > > > +exit: > > > +g_free(d); > > > +} > > > +} > > > + > > > static void vfio_instance_init(Object *obj) > > > { > > > PCIDevice *pci_dev = PCI_DEVICE(obj); > > > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h > > > index bd98618..984391d 100644 > > > --- a/hw/vfio/pci.h > > > +++ b/hw/vfio/pci.h > > > @@ -144,6 +144,8 @@ typedef struct VFIOPCIDevice { > > > bool no_kvm_intx; > > > bool no_kvm_msi; > > > bool no_kvm_msix; > > > +bool device_stop; > > > +MemoryListener vfio_memory_listener; > > > } VFIOPCIDevice; > > > > > > uint32_t vfio_pci_read_config(PCIDevice *pdev, uint32_t addr, int len); > > > diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h > > > index fa17848..aa73ee1 100644 > > > --- a/linux-headers/linux/vfio.h > > > +++ b/linux-headers/linux/vfio.h > > > @@ -502,6 +502,20 @@ struct vfio_pci_st
[Qemu-devel] [PATCH v3 19/20] block: Minimize raw use of bds->total_sectors
bdrv_is_allocated_above() was relying on intermediate->total_sectors, which is a field that can have stale contents depending on the value of intermediate->has_variable_length. An audit shows that we are safe (we were first calling through bdrv_co_get_block_status() which in turn calls bdrv_nb_sectors() and therefore just refreshed the current length), but it's nicer to favor our accessor functions to avoid having to repeat such an audit, even if it means refresh_total_sectors() is called more frequently. Suggested-by: John Snow Signed-off-by: Eric Blake --- v2: new patch --- block/io.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/block/io.c b/block/io.c index 0545180..5bbf153 100644 --- a/block/io.c +++ b/block/io.c @@ -1924,6 +1924,7 @@ int bdrv_is_allocated_above(BlockDriverState *top, intermediate = top; while (intermediate && intermediate != base) { int64_t pnum_inter; +int64_t size_inter; int psectors_inter; ret = bdrv_is_allocated(intermediate, sector_num * BDRV_SECTOR_SIZE, @@ -1941,13 +1942,14 @@ int bdrv_is_allocated_above(BlockDriverState *top, /* * [sector_num, nb_sectors] is unallocated on top but intermediate - * might have - * - * [sector_num+x, nr_sectors] allocated. + * might have [sector_num+x, nb_sectors-x] allocated. */ +size_inter = bdrv_nb_sectors(intermediate); +if (size_inter < 0) { +return size_inter; +} if (n > psectors_inter && -(intermediate == top || - sector_num + psectors_inter < intermediate->total_sectors)) { +(intermediate == top || sector_num + psectors_inter < size_inter)) { n = psectors_inter; } -- 2.9.4
[Qemu-devel] [PATCH v3 17/20] backup: Switch backup_run() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Change the internal loop iteration of backups to track by bytes instead of sectors (although we are still guaranteed that we iterate by steps that are cluster-aligned). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/backup.c | 32 +++- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/block/backup.c b/block/backup.c index c029d44..04def91 100644 --- a/block/backup.c +++ b/block/backup.c @@ -370,11 +370,10 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) int ret = 0; int clusters_per_iter; uint32_t granularity; -int64_t sector; +int64_t offset; int64_t cluster; int64_t end; int64_t last_cluster = -1; -int64_t sectors_per_cluster = cluster_size_sectors(job); BdrvDirtyBitmapIter *dbi; granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap); @@ -382,8 +381,8 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) dbi = bdrv_dirty_iter_new(job->sync_bitmap, 0); /* Find the next dirty sector(s) */ -while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { -cluster = sector / sectors_per_cluster; +while ((offset = bdrv_dirty_iter_next(dbi) * BDRV_SECTOR_SIZE) >= 0) { +cluster = offset / job->cluster_size; /* Fake progress updates for any clusters we skipped */ if (cluster != last_cluster + 1) { @@ -410,7 +409,8 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) /* If the bitmap granularity is smaller than the backup granularity, * we need to advance the iterator pointer to the next cluster. */ if (granularity < job->cluster_size) { -bdrv_set_dirty_iter(dbi, cluster * sectors_per_cluster); +bdrv_set_dirty_iter(dbi, +cluster * job->cluster_size / BDRV_SECTOR_SIZE); } last_cluster = cluster - 1; @@ -432,17 +432,15 @@ static void coroutine_fn backup_run(void *opaque) BackupBlockJob *job = opaque; BackupCompleteData *data; BlockDriverState *bs = blk_bs(job->common.blk); -int64_t start, end; +int64_t offset; int64_t sectors_per_cluster = cluster_size_sectors(job); int ret = 0; QLIST_INIT(&job->inflight_reqs); qemu_co_rwlock_init(&job->flush_rwlock); -start = 0; -end = DIV_ROUND_UP(job->common.len, job->cluster_size); - -job->done_bitmap = bitmap_new(end); +job->done_bitmap = bitmap_new(DIV_ROUND_UP(job->common.len, + job->cluster_size)); job->before_write.notify = backup_before_write_notify; bdrv_add_before_write_notifier(bs, &job->before_write); @@ -457,7 +455,8 @@ static void coroutine_fn backup_run(void *opaque) ret = backup_run_incremental(job); } else { /* Both FULL and TOP SYNC_MODE's require copying.. */ -for (; start < end; start++) { +for (offset = 0; offset < job->common.len; + offset += job->cluster_size) { bool error_is_read; int alloced = 0; @@ -480,8 +479,8 @@ static void coroutine_fn backup_run(void *opaque) * needed but at some point that is always the case. */ alloced = bdrv_is_allocated(bs, -start * sectors_per_cluster + i, -sectors_per_cluster - i, &n); + (offset >> BDRV_SECTOR_BITS) + i, + sectors_per_cluster - i, &n); i += n; if (alloced || n == 0) { @@ -499,9 +498,8 @@ static void coroutine_fn backup_run(void *opaque) if (alloced < 0) { ret = alloced; } else { -ret = backup_do_cow(job, start * job->cluster_size, -job->cluster_size, &error_is_read, -false); +ret = backup_do_cow(job, offset, job->cluster_size, +&error_is_read, false); } if (ret < 0) { /* Depending on error action, fail now or retry cluster */ @@ -510,7 +508,7 @@ static void coroutine_fn backup_run(void *opaque) if (action == BLOCK_ERROR_ACTION_REPORT) { break; } else { -start--; +offset -= job->cluster_size; continue; } } -- 2.9.4
[Qemu-devel] [PATCH v3 15/20] backup: Switch block_backup.h to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Continue by converting the public interface to backup jobs (no semantic change), including a change to CowRequest to track by bytes instead of cluster indices. Note that this does not change the difference between the public interface (starting point, and size of the subsequent range) and the internal interface (starting and end points). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: change a couple more parameter names --- include/block/block_backup.h | 11 +-- block/backup.c | 33 - block/replication.c | 12 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/include/block/block_backup.h b/include/block/block_backup.h index 8a75947..994a3bd 100644 --- a/include/block/block_backup.h +++ b/include/block/block_backup.h @@ -21,17 +21,16 @@ #include "block/block_int.h" typedef struct CowRequest { -int64_t start; -int64_t end; +int64_t start_byte; +int64_t end_byte; QLIST_ENTRY(CowRequest) list; CoQueue wait_queue; /* coroutines blocked on this request */ } CowRequest; -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors); +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes); void backup_cow_request_begin(CowRequest *req, BlockJob *job, - int64_t sector_num, - int nb_sectors); + int64_t offset, uint64_t bytes); void backup_cow_request_end(CowRequest *req); void backup_do_checkpoint(BlockJob *job, Error **errp); diff --git a/block/backup.c b/block/backup.c index 4e64710..cfbd921 100644 --- a/block/backup.c +++ b/block/backup.c @@ -55,7 +55,7 @@ static inline int64_t cluster_size_sectors(BackupBlockJob *job) /* See if in-flight requests overlap and wait for them to complete */ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, - int64_t start, + int64_t offset, int64_t end) { CowRequest *req; @@ -64,7 +64,7 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, do { retry = false; QLIST_FOREACH(req, &job->inflight_reqs, list) { -if (end > req->start && start < req->end) { +if (end > req->start_byte && offset < req->end_byte) { qemu_co_queue_wait(&req->wait_queue, NULL); retry = true; break; @@ -75,10 +75,10 @@ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, /* Keep track of an in-flight request */ static void cow_request_begin(CowRequest *req, BackupBlockJob *job, - int64_t start, int64_t end) + int64_t offset, int64_t end) { -req->start = start; -req->end = end; +req->start_byte = offset; +req->end_byte = end; qemu_co_queue_init(&req->wait_queue); QLIST_INSERT_HEAD(&job->inflight_reqs, req, list); } @@ -114,8 +114,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); -wait_for_overlapping_requests(job, start, end); -cow_request_begin(&cow_request, job, start, end); +wait_for_overlapping_requests(job, start * job->cluster_size, + end * job->cluster_size); +cow_request_begin(&cow_request, job, start * job->cluster_size, + end * job->cluster_size); for (; start < end; start++) { if (test_bit(start, job->done_bitmap)) { @@ -277,32 +279,29 @@ void backup_do_checkpoint(BlockJob *job, Error **errp) bitmap_zero(backup_job->done_bitmap, len); } -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t sector_num, - int nb_sectors) +void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, + uint64_t bytes) { BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); -int64_t sectors_per_cluster = cluster_size_sectors(backup_job); int64_t start, end; assert(job->driver->job_type == BLOCK_JOB_TYPE_BACKUP); -start = sector_num / sectors_per_cluster; -end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); +start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); +end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); wait_for_overlapping_requests(backup_job, start, end); } void backup_cow_r
Re: [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns
On Tue, Jun 27, 2017 at 22:16:39 +0300, Michael S. Tsirkin wrote: > On Tue, Jun 27, 2017 at 02:51:52PM -0400, Emilio G. Cota wrote: > > On Fri, Dec 02, 2016 at 22:01:52 +0100, Laszlo Ersek wrote: > > > When passed to git-diff (and to every other git command producing diffs > > > and/or diffstats) with "-O" or "diff.orderFile", this list of patterns > > > will place the more declarative / abstract hunks first, while changes to > > > imperative code / details will be near the end of the patches. This saves > > > on scrolling / searching and makes for easier reviewing. > > > > > > We intend to advise contributors in the Wiki to run > > > > > > git config diff.orderFile scripts/git.orderfile > > > > > > once, as part of their initial setup, before formatting their first (or, > > > for repeat contributors, next) patches. > > > > > > See the "-O" option and the "diff.orderFile" configuration variable in > > > git-diff(1) and git-config(1). > > > > > > Cc: "Michael S. Tsirkin" > > > Cc: Eric Blake > > > Cc: Fam Zheng > > > Cc: Gerd Hoffmann > > > Cc: John Snow > > > Cc: Max Reitz > > > Cc: Stefan Hajnoczi > > > Signed-off-by: Laszlo Ersek > > > --- > > > > Refloating 6+ months later, but.. > > > > Someone please merge this! :-) > > > > E. > > Users should be aware that if they do this, the need to > supply -O /dev/null to git pull-request. Otherwise the > diffstat is all scrambled and you can't figure out > what's included. I'd do the opposite: recommend passing '-O scripts/git.orderfile' to git-format patch, and leave the default for all other commands. E.
[Qemu-devel] [PATCH v3 13/20] block: Drop unused bdrv_round_sectors_to_clusters()
Now that the last user [mirror_iteration()] has converted to using bytes, we no longer need a function to round sectors to clusters. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: hoist to earlier series, no change --- include/block/block.h | 4 block/io.c| 21 - 2 files changed, 25 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 3e91cac..5cdd690 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -473,10 +473,6 @@ const char *bdrv_get_device_or_node_name(const BlockDriverState *bs); int bdrv_get_flags(BlockDriverState *bs); int bdrv_get_info(BlockDriverState *bs, BlockDriverInfo *bdi); ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs); -void bdrv_round_sectors_to_clusters(BlockDriverState *bs, -int64_t sector_num, int nb_sectors, -int64_t *cluster_sector_num, -int *cluster_nb_sectors); void bdrv_round_to_clusters(BlockDriverState *bs, int64_t offset, unsigned int bytes, int64_t *cluster_offset, diff --git a/block/io.c b/block/io.c index c72d701..d9fec1f 100644 --- a/block/io.c +++ b/block/io.c @@ -419,27 +419,6 @@ static void mark_request_serialising(BdrvTrackedRequest *req, uint64_t align) } /** - * Round a region to cluster boundaries (sector-based) - */ -void bdrv_round_sectors_to_clusters(BlockDriverState *bs, -int64_t sector_num, int nb_sectors, -int64_t *cluster_sector_num, -int *cluster_nb_sectors) -{ -BlockDriverInfo bdi; - -if (bdrv_get_info(bs, &bdi) < 0 || bdi.cluster_size == 0) { -*cluster_sector_num = sector_num; -*cluster_nb_sectors = nb_sectors; -} else { -int64_t c = bdi.cluster_size / BDRV_SECTOR_SIZE; -*cluster_sector_num = QEMU_ALIGN_DOWN(sector_num, c); -*cluster_nb_sectors = QEMU_ALIGN_UP(sector_num - *cluster_sector_num + -nb_sectors, c); -} -} - -/** * Round a region to cluster boundaries */ void bdrv_round_to_clusters(BlockDriverState *bs, -- 2.9.4
[Qemu-devel] [PATCH v3 20/20] block: Make bdrv_is_allocated_above() byte-based
We are gradually moving away from sector-based interfaces, towards byte-based. In the common case, allocation is unlikely to ever use values that are not naturally sector-aligned, but it is possible that byte-based values will let us be more precise about allocation at the end of an unaligned file that can do byte-based access. Changing the signature of the function to use int64_t *pnum ensures that the compiler enforces that all callers are updated. For now, the io.c layer still assert()s that all callers are sector-aligned, but that can be relaxed when a later patch implements byte-based block status. Therefore, for the most part this patch is just the addition of scaling at the callers followed by inverse scaling at bdrv_is_allocated(). But some code, particularly stream_run(), gets a lot simpler because it no longer has to mess with sectors. For ease of review, bdrv_is_allocated() was tackled separately. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: tweak function comments, favor bdrv_getlength() over ->total_sectors --- include/block/block.h | 2 +- block/commit.c| 20 block/io.c| 42 -- block/mirror.c| 5 - block/replication.c | 17 - block/stream.c| 21 + qemu-img.c| 10 +++--- 7 files changed, 61 insertions(+), 56 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 9b9d87b..13022d5 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -428,7 +428,7 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs, int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, int64_t *pnum); int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, -int64_t sector_num, int nb_sectors, int *pnum); +int64_t offset, int64_t bytes, int64_t *pnum); bool bdrv_is_read_only(BlockDriverState *bs); bool bdrv_is_writable(BlockDriverState *bs); diff --git a/block/commit.c b/block/commit.c index 241aa95..774a8a5 100644 --- a/block/commit.c +++ b/block/commit.c @@ -146,7 +146,7 @@ static void coroutine_fn commit_run(void *opaque) int64_t offset; uint64_t delay_ns = 0; int ret = 0; -int n = 0; /* sectors */ +int64_t n = 0; /* bytes */ void *buf = NULL; int bytes_written = 0; int64_t base_len; @@ -171,7 +171,7 @@ static void coroutine_fn commit_run(void *opaque) buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE); -for (offset = 0; offset < s->common.len; offset += n * BDRV_SECTOR_SIZE) { +for (offset = 0; offset < s->common.len; offset += n) { bool copy; /* Note that even when no rate limit is applied we need to yield @@ -183,15 +183,12 @@ static void coroutine_fn commit_run(void *opaque) } /* Copy if allocated above the base */ ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base), - offset / BDRV_SECTOR_SIZE, - COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE, - &n); + offset, COMMIT_BUFFER_SIZE, &n); copy = (ret == 1); -trace_commit_one_iteration(s, offset, n * BDRV_SECTOR_SIZE, ret); +trace_commit_one_iteration(s, offset, n, ret); if (copy) { -ret = commit_populate(s->top, s->base, offset, - n * BDRV_SECTOR_SIZE, buf); -bytes_written += n * BDRV_SECTOR_SIZE; +ret = commit_populate(s->top, s->base, offset, n, buf); +bytes_written += n; } if (ret < 0) { BlockErrorAction action = @@ -204,11 +201,10 @@ static void coroutine_fn commit_run(void *opaque) } } /* Publish progress */ -s->common.offset += n * BDRV_SECTOR_SIZE; +s->common.offset += n; if (copy && s->common.speed) { -delay_ns = ratelimit_calculate_delay(&s->limit, - n * BDRV_SECTOR_SIZE); +delay_ns = ratelimit_calculate_delay(&s->limit, n); } } diff --git a/block/io.c b/block/io.c index 5bbf153..061a162 100644 --- a/block/io.c +++ b/block/io.c @@ -1903,54 +1903,52 @@ int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t offset, /* * Given an image chain: ... -> [BASE] -> [INTER1] -> [INTER2] -> [TOP] * - * Return true if the given sector is allocated in any image between - * BASE and TOP (inclusive). BASE can be NULL to check if the given - * sector is allocated in any image of the chain. Return false otherwise, + * Return true if the (prefix of the) given range is allocated in any image + * between BASE and TOP (inclusive). BASE can be NULL to check if the given + * offset is allocated in any im
[Qemu-devel] [PATCH v3 09/20] mirror: Update signature of mirror_clip_sectors()
Rather than having a void function that modifies its input in-place as the output, change the signature to reduce a layer of indirection and return the result. Suggested-by: John Snow Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: new patch --- block/mirror.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index af27bcc..1a43304 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -176,12 +176,12 @@ static void mirror_read_complete(void *opaque, int ret) aio_context_release(blk_get_aio_context(s->common.blk)); } -static inline void mirror_clip_sectors(MirrorBlockJob *s, - int64_t sector_num, - int *nb_sectors) +static inline int mirror_clip_sectors(MirrorBlockJob *s, + int64_t sector_num, + int nb_sectors) { -*nb_sectors = MIN(*nb_sectors, - s->bdev_length / BDRV_SECTOR_SIZE - sector_num); +return MIN(nb_sectors, + s->bdev_length / BDRV_SECTOR_SIZE - sector_num); } /* Round sector_num and/or nb_sectors to target cluster if COW is needed, and @@ -216,7 +216,8 @@ static int mirror_cow_align(MirrorBlockJob *s, } /* Clipping may result in align_nb_sectors unaligned to chunk boundary, but * that doesn't matter because it's already the end of source image. */ -mirror_clip_sectors(s, align_sector_num, &align_nb_sectors); +align_nb_sectors = mirror_clip_sectors(s, align_sector_num, + align_nb_sectors); ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors); *sector_num = align_sector_num; @@ -445,7 +446,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) return 0; } -mirror_clip_sectors(s, sector_num, &io_sectors); +io_sectors = mirror_clip_sectors(s, sector_num, io_sectors); switch (mirror_method) { case MIRROR_METHOD_COPY: io_sectors = mirror_do_read(s, sector_num, io_sectors); -- 2.9.4
[Qemu-devel] [PATCH v3 18/20] block: Make bdrv_is_allocated() byte-based
We are gradually moving away from sector-based interfaces, towards byte-based. In the common case, allocation is unlikely to ever use values that are not naturally sector-aligned, but it is possible that byte-based values will let us be more precise about allocation at the end of an unaligned file that can do byte-based access. Changing the signature of the function to use int64_t *pnum ensures that the compiler enforces that all callers are updated. For now, the io.c layer still assert()s that all callers are sector-aligned on input and that *pnum is sector-aligned on return to the caller, but that can be relaxed when a later patch implements byte-based block status. Therefore, this code adds usages like DIV_ROUND_UP(,BDRV_SECTOR_SIZE) to callers that still want aligned values, where the call might reasonbly give non-aligned results in the future; on the other hand, no rounding is needed for callers that should just continue to work with byte alignment. For the most part this patch is just the addition of scaling at the callers followed by inverse scaling at bdrv_is_allocated(). But some code, particularly bdrv_commit(), gets a lot simpler because it no longer has to mess with sectors; also, it is now possible to pass NULL if the caller does not care how much of the image is allocated beyond the initial offset. For ease of review, bdrv_is_allocated_above() will be tackled separately. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: rebase to earlier changes, tweak commit message --- include/block/block.h | 4 +-- block/backup.c| 17 - block/commit.c| 21 +++- block/io.c| 49 +--- block/stream.c| 5 ++-- block/vvfat.c | 34 ++--- migration/block.c | 9 --- qemu-img.c| 5 +++- qemu-io-cmds.c| 70 +++ 9 files changed, 114 insertions(+), 100 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 5cdd690..9b9d87b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -425,8 +425,8 @@ int64_t bdrv_get_block_status_above(BlockDriverState *bs, int64_t sector_num, int nb_sectors, int *pnum, BlockDriverState **file); -int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors, - int *pnum); +int bdrv_is_allocated(BlockDriverState *bs, int64_t offset, int64_t bytes, + int64_t *pnum); int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, int64_t sector_num, int nb_sectors, int *pnum); diff --git a/block/backup.c b/block/backup.c index 04def91..b2048bf 100644 --- a/block/backup.c +++ b/block/backup.c @@ -47,12 +47,6 @@ typedef struct BackupBlockJob { QLIST_HEAD(, CowRequest) inflight_reqs; } BackupBlockJob; -/* Size of a cluster in sectors, instead of bytes. */ -static inline int64_t cluster_size_sectors(BackupBlockJob *job) -{ - return job->cluster_size / BDRV_SECTOR_SIZE; -} - /* See if in-flight requests overlap and wait for them to complete */ static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job, int64_t offset, @@ -433,7 +427,6 @@ static void coroutine_fn backup_run(void *opaque) BackupCompleteData *data; BlockDriverState *bs = blk_bs(job->common.blk); int64_t offset; -int64_t sectors_per_cluster = cluster_size_sectors(job); int ret = 0; QLIST_INIT(&job->inflight_reqs); @@ -465,12 +458,13 @@ static void coroutine_fn backup_run(void *opaque) } if (job->sync_mode == MIRROR_SYNC_MODE_TOP) { -int i, n; +int i; +int64_t n; /* Check to see if these blocks are already in the * backing file. */ -for (i = 0; i < sectors_per_cluster;) { +for (i = 0; i < job->cluster_size;) { /* bdrv_is_allocated() only returns true/false based * on the first set of sectors it comes across that * are are all in the same state. @@ -478,9 +472,8 @@ static void coroutine_fn backup_run(void *opaque) * backup cluster length. We end up copying more than * needed but at some point that is always the case. */ alloced = -bdrv_is_allocated(bs, - (offset >> BDRV_SECTOR_BITS) + i, - sectors_per_cluster - i, &n); +bdrv_is_allocated(bs, offset + i, + job->cluster_size - i, &n); i += n; if
[Qemu-devel] [PATCH v3 14/20] backup: Switch BackupBlockJob to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Continue by converting an internal structure (no semantic change), and all references to tracking progress. Drop a redundant local variable bytes_per_cluster. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/backup.c | 33 +++-- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/block/backup.c b/block/backup.c index 06431ac..4e64710 100644 --- a/block/backup.c +++ b/block/backup.c @@ -39,7 +39,7 @@ typedef struct BackupBlockJob { BlockdevOnError on_source_error; BlockdevOnError on_target_error; CoRwlock flush_rwlock; -uint64_t sectors_read; +uint64_t bytes_read; unsigned long *done_bitmap; int64_t cluster_size; bool compress; @@ -102,16 +102,15 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, void *bounce_buffer = NULL; int ret = 0; int64_t sectors_per_cluster = cluster_size_sectors(job); -int64_t bytes_per_cluster = sectors_per_cluster * BDRV_SECTOR_SIZE; -int64_t start, end; -int n; +int64_t start, end; /* clusters */ +int n; /* bytes */ qemu_co_rwlock_rdlock(&job->flush_rwlock); start = sector_num / sectors_per_cluster; end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); -trace_backup_do_cow_enter(job, start * bytes_per_cluster, +trace_backup_do_cow_enter(job, start * job->cluster_size, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE); @@ -120,28 +119,27 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, for (; start < end; start++) { if (test_bit(start, job->done_bitmap)) { -trace_backup_do_cow_skip(job, start * bytes_per_cluster); +trace_backup_do_cow_skip(job, start * job->cluster_size); continue; /* already copied */ } -trace_backup_do_cow_process(job, start * bytes_per_cluster); +trace_backup_do_cow_process(job, start * job->cluster_size); -n = MIN(sectors_per_cluster, -job->common.len / BDRV_SECTOR_SIZE - -start * sectors_per_cluster); +n = MIN(job->cluster_size, +job->common.len - start * job->cluster_size); if (!bounce_buffer) { bounce_buffer = blk_blockalign(blk, job->cluster_size); } iov.iov_base = bounce_buffer; -iov.iov_len = n * BDRV_SECTOR_SIZE; +iov.iov_len = n; qemu_iovec_init_external(&bounce_qiov, &iov, 1); ret = blk_co_preadv(blk, start * job->cluster_size, bounce_qiov.size, &bounce_qiov, is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0); if (ret < 0) { -trace_backup_do_cow_read_fail(job, start * bytes_per_cluster, ret); +trace_backup_do_cow_read_fail(job, start * job->cluster_size, ret); if (error_is_read) { *error_is_read = true; } @@ -157,7 +155,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0); } if (ret < 0) { -trace_backup_do_cow_write_fail(job, start * bytes_per_cluster, ret); +trace_backup_do_cow_write_fail(job, start * job->cluster_size, ret); if (error_is_read) { *error_is_read = false; } @@ -169,8 +167,8 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, /* Publish progress, guest I/O counts as progress too. Note that the * offset field is an opaque progress value, it is not a disk offset. */ -job->sectors_read += n; -job->common.offset += n * BDRV_SECTOR_SIZE; +job->bytes_read += n; +job->common.offset += n; } out: @@ -363,9 +361,8 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) */ if (job->common.speed) { uint64_t delay_ns = ratelimit_calculate_delay(&job->limit, - job->sectors_read * - BDRV_SECTOR_SIZE); -job->sectors_read = 0; + job->bytes_read); +job->bytes_read = 0; block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns); } else { block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, 0); -- 2.9.4
[Qemu-devel] [PATCH v3 07/20] mirror: Switch MirrorBlockJob to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Continue by converting an internal structure (no semantic change), and all references to the buffer size. [checkpatch has a false positive on use of MIN() in this patch] Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/mirror.c | 79 -- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index b4dfe95..9e28d59 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -24,9 +24,8 @@ #define SLICE_TIME1ULL /* ns */ #define MAX_IN_FLIGHT 16 -#define MAX_IO_SECTORS ((1 << 20) >> BDRV_SECTOR_BITS) /* 1 Mb */ -#define DEFAULT_MIRROR_BUF_SIZE \ -(MAX_IN_FLIGHT * MAX_IO_SECTORS * BDRV_SECTOR_SIZE) +#define MAX_IO_BYTES (1 << 20) /* 1 Mb */ +#define DEFAULT_MIRROR_BUF_SIZE (MAX_IN_FLIGHT * MAX_IO_BYTES) /* The mirroring buffer is a list of granularity-sized chunks. * Free chunks are organized in a list. @@ -67,11 +66,11 @@ typedef struct MirrorBlockJob { uint64_t last_pause_ns; unsigned long *in_flight_bitmap; int in_flight; -int64_t sectors_in_flight; +int64_t bytes_in_flight; int ret; bool unmap; bool waiting_for_io; -int target_cluster_sectors; +int target_cluster_size; int max_iov; bool initial_zeroing_ongoing; } MirrorBlockJob; @@ -79,8 +78,8 @@ typedef struct MirrorBlockJob { typedef struct MirrorOp { MirrorBlockJob *s; QEMUIOVector qiov; -int64_t sector_num; -int nb_sectors; +int64_t offset; +uint64_t bytes; } MirrorOp; static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, @@ -101,13 +100,12 @@ static void mirror_iteration_done(MirrorOp *op, int ret) MirrorBlockJob *s = op->s; struct iovec *iov; int64_t chunk_num; -int i, nb_chunks, sectors_per_chunk; +int i, nb_chunks; -trace_mirror_iteration_done(s, op->sector_num * BDRV_SECTOR_SIZE, -op->nb_sectors * BDRV_SECTOR_SIZE, ret); +trace_mirror_iteration_done(s, op->offset, op->bytes, ret); s->in_flight--; -s->sectors_in_flight -= op->nb_sectors; +s->bytes_in_flight -= op->bytes; iov = op->qiov.iov; for (i = 0; i < op->qiov.niov; i++) { MirrorBuffer *buf = (MirrorBuffer *) iov[i].iov_base; @@ -115,16 +113,15 @@ static void mirror_iteration_done(MirrorOp *op, int ret) s->buf_free_count++; } -sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS; -chunk_num = op->sector_num / sectors_per_chunk; -nb_chunks = DIV_ROUND_UP(op->nb_sectors, sectors_per_chunk); +chunk_num = op->offset / s->granularity; +nb_chunks = DIV_ROUND_UP(op->bytes, s->granularity); bitmap_clear(s->in_flight_bitmap, chunk_num, nb_chunks); if (ret >= 0) { if (s->cow_bitmap) { bitmap_set(s->cow_bitmap, chunk_num, nb_chunks); } if (!s->initial_zeroing_ongoing) { -s->common.offset += (uint64_t)op->nb_sectors * BDRV_SECTOR_SIZE; +s->common.offset += op->bytes; } } qemu_iovec_destroy(&op->qiov); @@ -144,7 +141,8 @@ static void mirror_write_complete(void *opaque, int ret) if (ret < 0) { BlockErrorAction action; -bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors); +bdrv_set_dirty_bitmap(s->dirty_bitmap, op->offset >> BDRV_SECTOR_BITS, + op->bytes >> BDRV_SECTOR_BITS); action = mirror_error_action(s, false, -ret); if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { s->ret = ret; @@ -163,7 +161,8 @@ static void mirror_read_complete(void *opaque, int ret) if (ret < 0) { BlockErrorAction action; -bdrv_set_dirty_bitmap(s->dirty_bitmap, op->sector_num, op->nb_sectors); +bdrv_set_dirty_bitmap(s->dirty_bitmap, op->offset >> BDRV_SECTOR_BITS, + op->bytes >> BDRV_SECTOR_BITS); action = mirror_error_action(s, true, -ret); if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { s->ret = ret; @@ -171,7 +170,7 @@ static void mirror_read_complete(void *opaque, int ret) mirror_iteration_done(op, ret); } else { -blk_aio_pwritev(s->target, op->sector_num * BDRV_SECTOR_SIZE, &op->qiov, +blk_aio_pwritev(s->target, op->offset, &op->qiov, 0, mirror_write_complete, op); } aio_context_release(blk_get_aio_context(s->common.blk)); @@ -211,7 +210,8 @@ static int mirror_cow_align(MirrorBlockJob *s, align_nb_sectors = max_sectors; if (need_cow) { align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors, - s->target_cluster_sectors); + s->target_cluster_size >> +
[Qemu-devel] [PATCH v3 16/20] backup: Switch backup_do_cow() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Convert another internal function (no semantic change). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/backup.c | 62 -- 1 file changed, 26 insertions(+), 36 deletions(-) diff --git a/block/backup.c b/block/backup.c index cfbd921..c029d44 100644 --- a/block/backup.c +++ b/block/backup.c @@ -91,7 +91,7 @@ static void cow_request_end(CowRequest *req) } static int coroutine_fn backup_do_cow(BackupBlockJob *job, - int64_t sector_num, int nb_sectors, + int64_t offset, uint64_t bytes, bool *error_is_read, bool is_write_notifier) { @@ -101,34 +101,28 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, QEMUIOVector bounce_qiov; void *bounce_buffer = NULL; int ret = 0; -int64_t sectors_per_cluster = cluster_size_sectors(job); -int64_t start, end; /* clusters */ +int64_t start, end; /* bytes */ int n; /* bytes */ qemu_co_rwlock_rdlock(&job->flush_rwlock); -start = sector_num / sectors_per_cluster; -end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); +start = QEMU_ALIGN_DOWN(offset, job->cluster_size); +end = QEMU_ALIGN_UP(bytes + offset, job->cluster_size); -trace_backup_do_cow_enter(job, start * job->cluster_size, - sector_num * BDRV_SECTOR_SIZE, - nb_sectors * BDRV_SECTOR_SIZE); +trace_backup_do_cow_enter(job, start, offset, bytes); -wait_for_overlapping_requests(job, start * job->cluster_size, - end * job->cluster_size); -cow_request_begin(&cow_request, job, start * job->cluster_size, - end * job->cluster_size); +wait_for_overlapping_requests(job, start, end); +cow_request_begin(&cow_request, job, start, end); -for (; start < end; start++) { -if (test_bit(start, job->done_bitmap)) { -trace_backup_do_cow_skip(job, start * job->cluster_size); +for (; start < end; start += job->cluster_size) { +if (test_bit(start / job->cluster_size, job->done_bitmap)) { +trace_backup_do_cow_skip(job, start); continue; /* already copied */ } -trace_backup_do_cow_process(job, start * job->cluster_size); +trace_backup_do_cow_process(job, start); -n = MIN(job->cluster_size, -job->common.len - start * job->cluster_size); +n = MIN(job->cluster_size, job->common.len - start); if (!bounce_buffer) { bounce_buffer = blk_blockalign(blk, job->cluster_size); @@ -137,11 +131,10 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, iov.iov_len = n; qemu_iovec_init_external(&bounce_qiov, &iov, 1); -ret = blk_co_preadv(blk, start * job->cluster_size, -bounce_qiov.size, &bounce_qiov, +ret = blk_co_preadv(blk, start, bounce_qiov.size, &bounce_qiov, is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0); if (ret < 0) { -trace_backup_do_cow_read_fail(job, start * job->cluster_size, ret); +trace_backup_do_cow_read_fail(job, start, ret); if (error_is_read) { *error_is_read = true; } @@ -149,22 +142,22 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, } if (buffer_is_zero(iov.iov_base, iov.iov_len)) { -ret = blk_co_pwrite_zeroes(job->target, start * job->cluster_size, +ret = blk_co_pwrite_zeroes(job->target, start, bounce_qiov.size, BDRV_REQ_MAY_UNMAP); } else { -ret = blk_co_pwritev(job->target, start * job->cluster_size, +ret = blk_co_pwritev(job->target, start, bounce_qiov.size, &bounce_qiov, job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0); } if (ret < 0) { -trace_backup_do_cow_write_fail(job, start * job->cluster_size, ret); +trace_backup_do_cow_write_fail(job, start, ret); if (error_is_read) { *error_is_read = false; } goto out; } -set_bit(start, job->done_bitmap); +set_bit(start / job->cluster_size, job->done_bitmap); /* Publish progress, guest I/O counts as progress too. Note that the * offset field is an opaque progress value, it is not a disk offset. @@ -180,8 +173,7 @@ out: cow_request_end(&cow_request); -trace_backup_do_cow_return(job, sector_num * BDRV_SECTOR_SIZE, - nb_sectors * BDRV_SECTOR_SIZE,
[Qemu-devel] [PATCH v3 08/20] mirror: Switch mirror_do_zero_or_discard() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Convert another internal function (no semantic change). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/mirror.c | 20 +++- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 9e28d59..af27bcc 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -305,8 +305,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, } static void mirror_do_zero_or_discard(MirrorBlockJob *s, - int64_t sector_num, - int nb_sectors, + int64_t offset, + uint64_t bytes, bool is_discard) { MirrorOp *op; @@ -315,16 +315,16 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s, * so the freeing in mirror_iteration_done is nop. */ op = g_new0(MirrorOp, 1); op->s = s; -op->offset = sector_num * BDRV_SECTOR_SIZE; -op->bytes = nb_sectors * BDRV_SECTOR_SIZE; +op->offset = offset; +op->bytes = bytes; s->in_flight++; -s->bytes_in_flight += nb_sectors * BDRV_SECTOR_SIZE; +s->bytes_in_flight += bytes; if (is_discard) { -blk_aio_pdiscard(s->target, sector_num << BDRV_SECTOR_BITS, +blk_aio_pdiscard(s->target, offset, op->bytes, mirror_write_complete, op); } else { -blk_aio_pwrite_zeroes(s->target, sector_num * BDRV_SECTOR_SIZE, +blk_aio_pwrite_zeroes(s->target, offset, op->bytes, s->unmap ? BDRV_REQ_MAY_UNMAP : 0, mirror_write_complete, op); } @@ -453,7 +453,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) break; case MIRROR_METHOD_ZERO: case MIRROR_METHOD_DISCARD: -mirror_do_zero_or_discard(s, sector_num, io_sectors, +mirror_do_zero_or_discard(s, sector_num * BDRV_SECTOR_SIZE, + io_sectors * BDRV_SECTOR_SIZE, mirror_method == MIRROR_METHOD_DISCARD); if (write_zeroes_ok) { io_bytes_acct = 0; @@ -657,7 +658,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) continue; } -mirror_do_zero_or_discard(s, sector_num, nb_sectors, false); +mirror_do_zero_or_discard(s, sector_num * BDRV_SECTOR_SIZE, + nb_sectors * BDRV_SECTOR_SIZE, false); sector_num += nb_sectors; } -- 2.9.4
[Qemu-devel] [PATCH v3 12/20] mirror: Switch mirror_iteration() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Change the internal loop iteration of mirroring to track by bytes instead of sectors (although we are still guaranteed that we iterate by steps that are both sector-aligned and multiples of the granularity). Drop the now-unused mirror_clip_sectors(). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v3: rebase to Paolo's thread-safety changes, R-b kept v2: straightforward rebase to earlier mirror_clip_bytes() change, R-b kept --- block/mirror.c | 105 + 1 file changed, 46 insertions(+), 59 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 81ff784..0eb2af4 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -184,15 +184,6 @@ static inline int64_t mirror_clip_bytes(MirrorBlockJob *s, return MIN(bytes, s->bdev_length - offset); } -/* Clip nb_sectors relative to sector_num to not exceed end-of-file */ -static inline int mirror_clip_sectors(MirrorBlockJob *s, - int64_t sector_num, - int nb_sectors) -{ -return MIN(nb_sectors, - s->bdev_length / BDRV_SECTOR_SIZE - sector_num); -} - /* Round offset and/or bytes to target cluster if COW is needed, and * return the offset of the adjusted tail against original. */ static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, @@ -336,30 +327,28 @@ static void mirror_do_zero_or_discard(MirrorBlockJob *s, static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) { BlockDriverState *source = s->source; -int64_t sector_num, first_chunk; +int64_t offset, first_chunk; uint64_t delay_ns = 0; /* At least the first dirty chunk is mirrored in one iteration. */ int nb_chunks = 1; -int64_t end = s->bdev_length / BDRV_SECTOR_SIZE; int sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS; bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target)); int max_io_bytes = MAX(s->buf_size / MAX_IN_FLIGHT, MAX_IO_BYTES); bdrv_dirty_bitmap_lock(s->dirty_bitmap); -sector_num = bdrv_dirty_iter_next(s->dbi); -if (sector_num < 0) { +offset = bdrv_dirty_iter_next(s->dbi) * BDRV_SECTOR_SIZE; +if (offset < 0) { bdrv_set_dirty_iter(s->dbi, 0); -sector_num = bdrv_dirty_iter_next(s->dbi); +offset = bdrv_dirty_iter_next(s->dbi) * BDRV_SECTOR_SIZE; trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) * BDRV_SECTOR_SIZE); -assert(sector_num >= 0); +assert(offset >= 0); } bdrv_dirty_bitmap_unlock(s->dirty_bitmap); -first_chunk = sector_num / sectors_per_chunk; +first_chunk = offset / s->granularity; while (test_bit(first_chunk, s->in_flight_bitmap)) { -trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE, - s->in_flight); +trace_mirror_yield_in_flight(s, offset, s->in_flight); mirror_wait_for_io(s); } @@ -368,25 +357,26 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) /* Find the number of consective dirty chunks following the first dirty * one, and wait for in flight requests in them. */ bdrv_dirty_bitmap_lock(s->dirty_bitmap); -while (nb_chunks * sectors_per_chunk < (s->buf_size >> BDRV_SECTOR_BITS)) { +while (nb_chunks * s->granularity < s->buf_size) { int64_t next_dirty; -int64_t next_sector = sector_num + nb_chunks * sectors_per_chunk; -int64_t next_chunk = next_sector / sectors_per_chunk; -if (next_sector >= end || -!bdrv_get_dirty_locked(source, s->dirty_bitmap, next_sector)) { +int64_t next_offset = offset + nb_chunks * s->granularity; +int64_t next_chunk = next_offset / s->granularity; +if (next_offset >= s->bdev_length || +!bdrv_get_dirty_locked(source, s->dirty_bitmap, + next_offset >> BDRV_SECTOR_BITS)) { break; } if (test_bit(next_chunk, s->in_flight_bitmap)) { break; } -next_dirty = bdrv_dirty_iter_next(s->dbi); -if (next_dirty > next_sector || next_dirty < 0) { +next_dirty = bdrv_dirty_iter_next(s->dbi) * BDRV_SECTOR_SIZE; +if (next_dirty > next_offset || next_dirty < 0) { /* The bitmap iterator's cache is stale, refresh it */ -bdrv_set_dirty_iter(s->dbi, next_sector); -next_dirty = bdrv_dirty_iter_next(s->dbi); +bdrv_set_dirty_iter(s->dbi, next_offset >> BDRV_SECTOR_BITS); +next_dirty = bdrv_dirty_iter_next(s->dbi) * BDRV_SECTOR_SIZE; } -assert(next_dirty == next_sector); +assert(next_dirty == next_offset); nb_chunks++; } @@ -394,14 +384,15 @@ static uint64_t coroutine_fn
[Qemu-devel] [PATCH v3 10/20] mirror: Switch mirror_cow_align() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Convert another internal function (no semantic change), and add mirror_clip_bytes() as a counterpart to mirror_clip_sectors(). Some of the conversion is a bit tricky, requiring temporaries to convert between units; it will be cleared up in a following patch. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: tweak mirror_clip_bytes() signature to match previous patch --- block/mirror.c | 64 ++ 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 1a43304..1a4602a 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -176,6 +176,15 @@ static void mirror_read_complete(void *opaque, int ret) aio_context_release(blk_get_aio_context(s->common.blk)); } +/* Clip bytes relative to offset to not exceed end-of-file */ +static inline int64_t mirror_clip_bytes(MirrorBlockJob *s, +int64_t offset, +int64_t bytes) +{ +return MIN(bytes, s->bdev_length - offset); +} + +/* Clip nb_sectors relative to sector_num to not exceed end-of-file */ static inline int mirror_clip_sectors(MirrorBlockJob *s, int64_t sector_num, int nb_sectors) @@ -184,44 +193,39 @@ static inline int mirror_clip_sectors(MirrorBlockJob *s, s->bdev_length / BDRV_SECTOR_SIZE - sector_num); } -/* Round sector_num and/or nb_sectors to target cluster if COW is needed, and - * return the offset of the adjusted tail sector against original. */ -static int mirror_cow_align(MirrorBlockJob *s, -int64_t *sector_num, -int *nb_sectors) +/* Round offset and/or bytes to target cluster if COW is needed, and + * return the offset of the adjusted tail against original. */ +static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, +unsigned int *bytes) { bool need_cow; int ret = 0; -int chunk_sectors = s->granularity >> BDRV_SECTOR_BITS; -int64_t align_sector_num = *sector_num; -int align_nb_sectors = *nb_sectors; -int max_sectors = chunk_sectors * s->max_iov; +int64_t align_offset = *offset; +unsigned int align_bytes = *bytes; +int max_bytes = s->granularity * s->max_iov; -need_cow = !test_bit(*sector_num / chunk_sectors, s->cow_bitmap); -need_cow |= !test_bit((*sector_num + *nb_sectors - 1) / chunk_sectors, +need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap); +need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity, s->cow_bitmap); if (need_cow) { -bdrv_round_sectors_to_clusters(blk_bs(s->target), *sector_num, - *nb_sectors, &align_sector_num, - &align_nb_sectors); +bdrv_round_to_clusters(blk_bs(s->target), *offset, *bytes, + &align_offset, &align_bytes); } -if (align_nb_sectors > max_sectors) { -align_nb_sectors = max_sectors; +if (align_bytes > max_bytes) { +align_bytes = max_bytes; if (need_cow) { -align_nb_sectors = QEMU_ALIGN_DOWN(align_nb_sectors, - s->target_cluster_size >> - BDRV_SECTOR_BITS); +align_bytes = QEMU_ALIGN_DOWN(align_bytes, + s->target_cluster_size); } } -/* Clipping may result in align_nb_sectors unaligned to chunk boundary, but +/* Clipping may result in align_bytes unaligned to chunk boundary, but * that doesn't matter because it's already the end of source image. */ -align_nb_sectors = mirror_clip_sectors(s, align_sector_num, - align_nb_sectors); +align_bytes = mirror_clip_bytes(s, align_offset, align_bytes); -ret = align_sector_num + align_nb_sectors - (*sector_num + *nb_sectors); -*sector_num = align_sector_num; -*nb_sectors = align_nb_sectors; +ret = align_offset + align_bytes - (*offset + *bytes); +*offset = align_offset; +*bytes = align_bytes; assert(ret >= 0); return ret; } @@ -257,10 +261,18 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, nb_sectors = MIN(s->buf_size >> BDRV_SECTOR_BITS, nb_sectors); nb_sectors = MIN(max_sectors, nb_sectors); assert(nb_sectors); +assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); ret = nb_sectors; if (s->cow_bitmap) { -ret += mirror_cow_align(s, §or_num, &nb_sectors); +int64_t offset = sector_num * BDRV_SECTOR_SIZE; +unsigned int bytes = nb_sectors * BDRV_SECTOR_SIZE; +int gap; + +gap = mirror_cow_align
[Qemu-devel] [PATCH v3 03/20] stream: Switch stream_populate() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Start by converting an internal function (no semantic change). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/stream.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/block/stream.c b/block/stream.c index 6cb3939..746d525 100644 --- a/block/stream.c +++ b/block/stream.c @@ -41,20 +41,20 @@ typedef struct StreamBlockJob { } StreamBlockJob; static int coroutine_fn stream_populate(BlockBackend *blk, -int64_t sector_num, int nb_sectors, +int64_t offset, uint64_t bytes, void *buf) { struct iovec iov = { .iov_base = buf, -.iov_len = nb_sectors * BDRV_SECTOR_SIZE, +.iov_len = bytes, }; QEMUIOVector qiov; +assert(bytes < SIZE_MAX); qemu_iovec_init_external(&qiov, &iov, 1); /* Copy-on-read the unallocated clusters */ -return blk_co_preadv(blk, sector_num * BDRV_SECTOR_SIZE, qiov.size, &qiov, - BDRV_REQ_COPY_ON_READ); +return blk_co_preadv(blk, offset, qiov.size, &qiov, BDRV_REQ_COPY_ON_READ); } typedef struct { @@ -171,7 +171,8 @@ static void coroutine_fn stream_run(void *opaque) trace_stream_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, n * BDRV_SECTOR_SIZE, ret); if (copy) { -ret = stream_populate(blk, sector_num, n, buf); +ret = stream_populate(blk, sector_num * BDRV_SECTOR_SIZE, + n * BDRV_SECTOR_SIZE, buf); } if (ret < 0) { BlockErrorAction action = -- 2.9.4
[Qemu-devel] [PATCH v3 11/20] mirror: Switch mirror_do_read() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Convert another internal function (no semantic change). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: rebase to earlier changes --- block/mirror.c | 75 ++ 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 1a4602a..81ff784 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -196,7 +196,7 @@ static inline int mirror_clip_sectors(MirrorBlockJob *s, /* Round offset and/or bytes to target cluster if COW is needed, and * return the offset of the adjusted tail against original. */ static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, -unsigned int *bytes) +uint64_t *bytes) { bool need_cow; int ret = 0; @@ -204,6 +204,7 @@ static int mirror_cow_align(MirrorBlockJob *s, int64_t *offset, unsigned int align_bytes = *bytes; int max_bytes = s->granularity * s->max_iov; +assert(*bytes < INT_MAX); need_cow = !test_bit(*offset / s->granularity, s->cow_bitmap); need_cow |= !test_bit((*offset + *bytes - 1) / s->granularity, s->cow_bitmap); @@ -239,59 +240,50 @@ static inline void mirror_wait_for_io(MirrorBlockJob *s) } /* Submit async read while handling COW. - * Returns: The number of sectors copied after and including sector_num, - * excluding any sectors copied prior to sector_num due to alignment. - * This will be nb_sectors if no alignment is necessary, or - * (new_end - sector_num) if tail is rounded up or down due to + * Returns: The number of bytes copied after and including offset, + * excluding any bytes copied prior to offset due to alignment. + * This will be @bytes if no alignment is necessary, or + * (new_end - offset) if tail is rounded up or down due to * alignment or buffer limit. */ -static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, - int nb_sectors) +static uint64_t mirror_do_read(MirrorBlockJob *s, int64_t offset, + uint64_t bytes) { BlockBackend *source = s->common.blk; -int sectors_per_chunk, nb_chunks; -int ret; +int nb_chunks; +uint64_t ret; MirrorOp *op; -int max_sectors; +uint64_t max_bytes; -sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS; -max_sectors = sectors_per_chunk * s->max_iov; +max_bytes = s->granularity * s->max_iov; /* We can only handle as much as buf_size at a time. */ -nb_sectors = MIN(s->buf_size >> BDRV_SECTOR_BITS, nb_sectors); -nb_sectors = MIN(max_sectors, nb_sectors); -assert(nb_sectors); -assert(nb_sectors < BDRV_REQUEST_MAX_SECTORS); -ret = nb_sectors; +bytes = MIN(s->buf_size, MIN(max_bytes, bytes)); +assert(bytes); +assert(bytes < BDRV_REQUEST_MAX_BYTES); +ret = bytes; if (s->cow_bitmap) { -int64_t offset = sector_num * BDRV_SECTOR_SIZE; -unsigned int bytes = nb_sectors * BDRV_SECTOR_SIZE; -int gap; - -gap = mirror_cow_align(s, &offset, &bytes); -sector_num = offset / BDRV_SECTOR_SIZE; -nb_sectors = bytes / BDRV_SECTOR_SIZE; -ret += gap / BDRV_SECTOR_SIZE; +ret += mirror_cow_align(s, &offset, &bytes); } -assert(nb_sectors << BDRV_SECTOR_BITS <= s->buf_size); -/* The sector range must meet granularity because: +assert(bytes <= s->buf_size); +/* The range will be sector-aligned because: * 1) Caller passes in aligned values; - * 2) mirror_cow_align is used only when target cluster is larger. */ -assert(!(sector_num % sectors_per_chunk)); -nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk); + * 2) mirror_cow_align is used only when target cluster is larger. + * But it might not be cluster-aligned at end-of-file. */ +assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); +nb_chunks = DIV_ROUND_UP(bytes, s->granularity); while (s->buf_free_count < nb_chunks) { -trace_mirror_yield_in_flight(s, sector_num * BDRV_SECTOR_SIZE, - s->in_flight); +trace_mirror_yield_in_flight(s, offset, s->in_flight); mirror_wait_for_io(s); } /* Allocate a MirrorOp that is used as an AIO callback. */ op = g_new(MirrorOp, 1); op->s = s; -op->offset = sector_num * BDRV_SECTOR_SIZE; -op->bytes = nb_sectors * BDRV_SECTOR_SIZE; +op->offset = offset; +op->bytes = bytes; /* Now make a QEMUIOVector taking enough granularity-sized chunks * from s->buf_free. @@ -299,7 +291,7 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, qemu_iovec_init(&op->qiov, nb_chunks); while (nb_chunks-- > 0) { MirrorBuffer *buf = QSIMPLEQ_FIRST(&s->buf_free); -
[Qemu-devel] [PATCH v3 04/20] stream: Switch stream_run() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Change the internal loop iteration of streaming to track by bytes instead of sectors (although we are still guaranteed that we iterate by steps that are sector-aligned). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/stream.c | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/block/stream.c b/block/stream.c index 746d525..2f9618b 100644 --- a/block/stream.c +++ b/block/stream.c @@ -108,12 +108,11 @@ static void coroutine_fn stream_run(void *opaque) BlockBackend *blk = s->common.blk; BlockDriverState *bs = blk_bs(blk); BlockDriverState *base = s->base; -int64_t sector_num = 0; -int64_t end = -1; +int64_t offset = 0; uint64_t delay_ns = 0; int error = 0; int ret = 0; -int n = 0; +int n = 0; /* sectors */ void *buf; if (!bs->backing) { @@ -126,7 +125,6 @@ static void coroutine_fn stream_run(void *opaque) goto out; } -end = s->common.len >> BDRV_SECTOR_BITS; buf = qemu_blockalign(bs, STREAM_BUFFER_SIZE); /* Turn on copy-on-read for the whole block device so that guest read @@ -138,7 +136,7 @@ static void coroutine_fn stream_run(void *opaque) bdrv_enable_copy_on_read(bs); } -for (sector_num = 0; sector_num < end; sector_num += n) { +for ( ; offset < s->common.len; offset += n * BDRV_SECTOR_SIZE) { bool copy; /* Note that even when no rate limit is applied we need to yield @@ -151,28 +149,26 @@ static void coroutine_fn stream_run(void *opaque) copy = false; -ret = bdrv_is_allocated(bs, sector_num, +ret = bdrv_is_allocated(bs, offset / BDRV_SECTOR_SIZE, STREAM_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n); if (ret == 1) { /* Allocated in the top, no need to copy. */ } else if (ret >= 0) { /* Copy if allocated in the intermediate images. Limit to the - * known-unallocated area [sector_num, sector_num+n). */ + * known-unallocated area [offset, offset+n*BDRV_SECTOR_SIZE). */ ret = bdrv_is_allocated_above(backing_bs(bs), base, - sector_num, n, &n); + offset / BDRV_SECTOR_SIZE, n, &n); /* Finish early if end of backing file has been reached */ if (ret == 0 && n == 0) { -n = end - sector_num; +n = (s->common.len - offset) / BDRV_SECTOR_SIZE; } copy = (ret == 1); } -trace_stream_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, - n * BDRV_SECTOR_SIZE, ret); +trace_stream_one_iteration(s, offset, n * BDRV_SECTOR_SIZE, ret); if (copy) { -ret = stream_populate(blk, sector_num * BDRV_SECTOR_SIZE, - n * BDRV_SECTOR_SIZE, buf); +ret = stream_populate(blk, offset, n * BDRV_SECTOR_SIZE, buf); } if (ret < 0) { BlockErrorAction action = @@ -211,7 +207,7 @@ out: /* Modify backing chain and close BDSes in main loop */ data = g_malloc(sizeof(*data)); data->ret = ret; -data->reached_end = sector_num == end; +data->reached_end = offset == s->common.len; block_job_defer_to_main_loop(&s->common, stream_complete, data); } -- 2.9.4
[Qemu-devel] [PATCH v3 05/20] commit: Switch commit_populate() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Start by converting an internal function (no semantic change). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/commit.c | 15 --- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/block/commit.c b/block/commit.c index 4cda7f2..6f67d78 100644 --- a/block/commit.c +++ b/block/commit.c @@ -47,26 +47,25 @@ typedef struct CommitBlockJob { } CommitBlockJob; static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base, -int64_t sector_num, int nb_sectors, +int64_t offset, uint64_t bytes, void *buf) { int ret = 0; QEMUIOVector qiov; struct iovec iov = { .iov_base = buf, -.iov_len = nb_sectors * BDRV_SECTOR_SIZE, +.iov_len = bytes, }; +assert(bytes < SIZE_MAX); qemu_iovec_init_external(&qiov, &iov, 1); -ret = blk_co_preadv(bs, sector_num * BDRV_SECTOR_SIZE, -qiov.size, &qiov, 0); +ret = blk_co_preadv(bs, offset, qiov.size, &qiov, 0); if (ret < 0) { return ret; } -ret = blk_co_pwritev(base, sector_num * BDRV_SECTOR_SIZE, - qiov.size, &qiov, 0); +ret = blk_co_pwritev(base, offset, qiov.size, &qiov, 0); if (ret < 0) { return ret; } @@ -193,7 +192,9 @@ static void coroutine_fn commit_run(void *opaque) trace_commit_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, n * BDRV_SECTOR_SIZE, ret); if (copy) { -ret = commit_populate(s->top, s->base, sector_num, n, buf); +ret = commit_populate(s->top, s->base, + sector_num * BDRV_SECTOR_SIZE, + n * BDRV_SECTOR_SIZE, buf); bytes_written += n * BDRV_SECTOR_SIZE; } if (ret < 0) { -- 2.9.4
[Qemu-devel] [PATCH v3 00/20] make bdrv_is_allocated[_above] byte-based
There are patches floating around to add NBD_CMD_BLOCK_STATUS, but NBD wants to report status on byte granularity (even if the reporting will probably be naturally aligned to sectors or even much higher levels). I've therefore started the task of converting our block status code to report at a byte granularity rather than sectors. This is part one of that conversion: bdrv_is_allocated(). Other parts still need a v3, but here's the link to their most recent posting: tracking dirty bitmaps by bytes: https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03859.html replacing bdrv_get_block_status() with a byte based callback in all the drivers: https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02642.html Available as a tag at: git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-allocated-v3 Depends on Kevin's block branch Changes since v2 are limited to rebase artifacts (Paolo's conversion to thread-safety being the biggest cause of context conflicts, and also affecting patch 12 - but where I think the resolution is sane enough to keep R-b). Added R-b on patches where it has been given, leaving 19/20 as the only unreviewed patch. 001/20:[] [--] 'blockjob: Track job ratelimits via bytes, not sectors' 002/20:[] [-C] 'trace: Show blockjob actions via bytes, not sectors' 003/20:[] [--] 'stream: Switch stream_populate() to byte-based' 004/20:[] [--] 'stream: Switch stream_run() to byte-based' 005/20:[] [--] 'commit: Switch commit_populate() to byte-based' 006/20:[] [--] 'commit: Switch commit_run() to byte-based' 007/20:[] [-C] 'mirror: Switch MirrorBlockJob to byte-based' 008/20:[] [--] 'mirror: Switch mirror_do_zero_or_discard() to byte-based' 009/20:[] [--] 'mirror: Update signature of mirror_clip_sectors()' 010/20:[] [--] 'mirror: Switch mirror_cow_align() to byte-based' 011/20:[] [--] 'mirror: Switch mirror_do_read() to byte-based' 012/20:[0012] [FC] 'mirror: Switch mirror_iteration() to byte-based' 013/20:[] [--] 'block: Drop unused bdrv_round_sectors_to_clusters()' 014/20:[] [--] 'backup: Switch BackupBlockJob to byte-based' 015/20:[] [--] 'backup: Switch block_backup.h to byte-based' 016/20:[] [--] 'backup: Switch backup_do_cow() to byte-based' 017/20:[] [--] 'backup: Switch backup_run() to byte-based' 018/20:[] [--] 'block: Make bdrv_is_allocated() byte-based' 019/20:[] [--] 'block: Minimize raw use of bds->total_sectors' 020/20:[] [-C] 'block: Make bdrv_is_allocated_above() byte-based' Eric Blake (20): blockjob: Track job ratelimits via bytes, not sectors trace: Show blockjob actions via bytes, not sectors stream: Switch stream_populate() to byte-based stream: Switch stream_run() to byte-based commit: Switch commit_populate() to byte-based commit: Switch commit_run() to byte-based mirror: Switch MirrorBlockJob to byte-based mirror: Switch mirror_do_zero_or_discard() to byte-based mirror: Update signature of mirror_clip_sectors() mirror: Switch mirror_cow_align() to byte-based mirror: Switch mirror_do_read() to byte-based mirror: Switch mirror_iteration() to byte-based block: Drop unused bdrv_round_sectors_to_clusters() backup: Switch BackupBlockJob to byte-based backup: Switch block_backup.h to byte-based backup: Switch backup_do_cow() to byte-based backup: Switch backup_run() to byte-based block: Make bdrv_is_allocated() byte-based block: Minimize raw use of bds->total_sectors block: Make bdrv_is_allocated_above() byte-based include/block/block.h| 10 +- include/block/block_backup.h | 11 +- include/qemu/ratelimit.h | 3 +- block/backup.c | 130 --- block/commit.c | 54 block/io.c | 92 +++-- block/mirror.c | 302 ++- block/replication.c | 29 +++-- block/stream.c | 35 +++-- block/vvfat.c| 34 +++-- migration/block.c| 9 +- qemu-img.c | 15 ++- qemu-io-cmds.c | 70 +- block/trace-events | 14 +- 14 files changed, 397 insertions(+), 411 deletions(-) -- 2.9.4
[Qemu-devel] [PATCH v3 06/20] commit: Switch commit_run() to byte-based
We are gradually converting to byte-based interfaces, as they are easier to reason about than sector-based. Change the internal loop iteration of committing to track by bytes instead of sectors (although we are still guaranteed that we iterate by steps that are sector-aligned). Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: no change --- block/commit.c | 16 ++-- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/block/commit.c b/block/commit.c index 6f67d78..c3a7bca 100644 --- a/block/commit.c +++ b/block/commit.c @@ -143,17 +143,16 @@ static void coroutine_fn commit_run(void *opaque) { CommitBlockJob *s = opaque; CommitCompleteData *data; -int64_t sector_num, end; +int64_t offset; uint64_t delay_ns = 0; int ret = 0; -int n = 0; +int n = 0; /* sectors */ void *buf = NULL; int bytes_written = 0; int64_t base_len; ret = s->common.len = blk_getlength(s->top); - if (s->common.len < 0) { goto out; } @@ -170,10 +169,9 @@ static void coroutine_fn commit_run(void *opaque) } } -end = s->common.len >> BDRV_SECTOR_BITS; buf = blk_blockalign(s->top, COMMIT_BUFFER_SIZE); -for (sector_num = 0; sector_num < end; sector_num += n) { +for (offset = 0; offset < s->common.len; offset += n * BDRV_SECTOR_SIZE) { bool copy; /* Note that even when no rate limit is applied we need to yield @@ -185,15 +183,13 @@ static void coroutine_fn commit_run(void *opaque) } /* Copy if allocated above the base */ ret = bdrv_is_allocated_above(blk_bs(s->top), blk_bs(s->base), - sector_num, + offset / BDRV_SECTOR_SIZE, COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n); copy = (ret == 1); -trace_commit_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, - n * BDRV_SECTOR_SIZE, ret); +trace_commit_one_iteration(s, offset, n * BDRV_SECTOR_SIZE, ret); if (copy) { -ret = commit_populate(s->top, s->base, - sector_num * BDRV_SECTOR_SIZE, +ret = commit_populate(s->top, s->base, offset, n * BDRV_SECTOR_SIZE, buf); bytes_written += n * BDRV_SECTOR_SIZE; } -- 2.9.4
[Qemu-devel] [PATCH v3 02/20] trace: Show blockjob actions via bytes, not sectors
Upcoming patches are going to switch to byte-based interfaces instead of sector-based. Even worse, trace_backup_do_cow_enter() had a weird mix of cluster and sector indices. The trace interface is low enough that there are no stability guarantees, and therefore nothing wrong with changing our units, even in cases like trace_backup_do_cow_skip() where we are not changing the trace output. So make the tracing uniformly use bytes. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: improve commit message, no code change --- block/backup.c | 16 ++-- block/commit.c | 3 ++- block/mirror.c | 26 +- block/stream.c | 3 ++- block/trace-events | 14 +++--- 5 files changed, 38 insertions(+), 24 deletions(-) diff --git a/block/backup.c b/block/backup.c index 9ca1d8e..06431ac 100644 --- a/block/backup.c +++ b/block/backup.c @@ -102,6 +102,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, void *bounce_buffer = NULL; int ret = 0; int64_t sectors_per_cluster = cluster_size_sectors(job); +int64_t bytes_per_cluster = sectors_per_cluster * BDRV_SECTOR_SIZE; int64_t start, end; int n; @@ -110,18 +111,20 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, start = sector_num / sectors_per_cluster; end = DIV_ROUND_UP(sector_num + nb_sectors, sectors_per_cluster); -trace_backup_do_cow_enter(job, start, sector_num, nb_sectors); +trace_backup_do_cow_enter(job, start * bytes_per_cluster, + sector_num * BDRV_SECTOR_SIZE, + nb_sectors * BDRV_SECTOR_SIZE); wait_for_overlapping_requests(job, start, end); cow_request_begin(&cow_request, job, start, end); for (; start < end; start++) { if (test_bit(start, job->done_bitmap)) { -trace_backup_do_cow_skip(job, start); +trace_backup_do_cow_skip(job, start * bytes_per_cluster); continue; /* already copied */ } -trace_backup_do_cow_process(job, start); +trace_backup_do_cow_process(job, start * bytes_per_cluster); n = MIN(sectors_per_cluster, job->common.len / BDRV_SECTOR_SIZE - @@ -138,7 +141,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, bounce_qiov.size, &bounce_qiov, is_write_notifier ? BDRV_REQ_NO_SERIALISING : 0); if (ret < 0) { -trace_backup_do_cow_read_fail(job, start, ret); +trace_backup_do_cow_read_fail(job, start * bytes_per_cluster, ret); if (error_is_read) { *error_is_read = true; } @@ -154,7 +157,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job, job->compress ? BDRV_REQ_WRITE_COMPRESSED : 0); } if (ret < 0) { -trace_backup_do_cow_write_fail(job, start, ret); +trace_backup_do_cow_write_fail(job, start * bytes_per_cluster, ret); if (error_is_read) { *error_is_read = false; } @@ -177,7 +180,8 @@ out: cow_request_end(&cow_request); -trace_backup_do_cow_return(job, sector_num, nb_sectors, ret); +trace_backup_do_cow_return(job, sector_num * BDRV_SECTOR_SIZE, + nb_sectors * BDRV_SECTOR_SIZE, ret); qemu_co_rwlock_unlock(&job->flush_rwlock); diff --git a/block/commit.c b/block/commit.c index 6993994..4cda7f2 100644 --- a/block/commit.c +++ b/block/commit.c @@ -190,7 +190,8 @@ static void coroutine_fn commit_run(void *opaque) COMMIT_BUFFER_SIZE / BDRV_SECTOR_SIZE, &n); copy = (ret == 1); -trace_commit_one_iteration(s, sector_num, n, ret); +trace_commit_one_iteration(s, sector_num * BDRV_SECTOR_SIZE, + n * BDRV_SECTOR_SIZE, ret); if (copy) { ret = commit_populate(s->top, s->base, sector_num, n, buf); bytes_written += n * BDRV_SECTOR_SIZE; diff --git a/block/mirror.c b/block/mirror.c index eb27efc..b4dfe95 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -103,7 +103,8 @@ static void mirror_iteration_done(MirrorOp *op, int ret) int64_t chunk_num; int i, nb_chunks, sectors_per_chunk; -trace_mirror_iteration_done(s, op->sector_num, op->nb_sectors, ret); +trace_mirror_iteration_done(s, op->sector_num * BDRV_SECTOR_SIZE, +op->nb_sectors * BDRV_SECTOR_SIZE, ret); s->in_flight--; s->sectors_in_flight -= op->nb_sectors; @@ -268,7 +269,8 @@ static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num, nb_chunks = DIV_ROUND_UP(nb_sectors, sectors_per_chunk); while (s->buf_free_count < nb_chunks) { -trace_mirror_yield_in_flight(s, sector_num, s->in_flight); +trace_mirror_yield_in_flight(s, se
[Qemu-devel] [PATCH v3 01/20] blockjob: Track job ratelimits via bytes, not sectors
The user interface specifies job rate limits in bytes/second. It's pointless to have our internal representation track things in sectors/second, particularly since we want to move away from sector-based interfaces. Fix up a doc typo found while verifying that the ratelimit code handles the scaling difference. Repetition of expressions like 'n * BDRV_SECTOR_SIZE' will be cleaned up later when functions are converted to iterate over images by bytes rather than by sectors. Signed-off-by: Eric Blake Reviewed-by: John Snow --- v2: adjust commit message based on review; no code change --- include/qemu/ratelimit.h | 3 ++- block/backup.c | 5 +++-- block/commit.c | 5 +++-- block/mirror.c | 13 +++-- block/stream.c | 5 +++-- 5 files changed, 18 insertions(+), 13 deletions(-) diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h index 8da1232..8dece48 100644 --- a/include/qemu/ratelimit.h +++ b/include/qemu/ratelimit.h @@ -24,7 +24,8 @@ typedef struct { /** Calculate and return delay for next request in ns * - * Record that we sent @p n data units. If we may send more data units + * Record that we sent @n data units (where @n matches the scale chosen + * during ratelimit_set_speed). If we may send more data units * in the current time slice, return 0 (i.e. no delay). Otherwise * return the amount of time (in ns) until the start of the next time * slice that will permit sending the next chunk of data. diff --git a/block/backup.c b/block/backup.c index 5387fbd..9ca1d8e 100644 --- a/block/backup.c +++ b/block/backup.c @@ -208,7 +208,7 @@ static void backup_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } -ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME); +ratelimit_set_speed(&s->limit, speed, SLICE_TIME); } static void backup_cleanup_sync_bitmap(BackupBlockJob *job, int ret) @@ -359,7 +359,8 @@ static bool coroutine_fn yield_and_check(BackupBlockJob *job) */ if (job->common.speed) { uint64_t delay_ns = ratelimit_calculate_delay(&job->limit, - job->sectors_read); + job->sectors_read * + BDRV_SECTOR_SIZE); job->sectors_read = 0; block_job_sleep_ns(&job->common, QEMU_CLOCK_REALTIME, delay_ns); } else { diff --git a/block/commit.c b/block/commit.c index 524bd54..6993994 100644 --- a/block/commit.c +++ b/block/commit.c @@ -209,7 +209,8 @@ static void coroutine_fn commit_run(void *opaque) s->common.offset += n * BDRV_SECTOR_SIZE; if (copy && s->common.speed) { -delay_ns = ratelimit_calculate_delay(&s->limit, n); +delay_ns = ratelimit_calculate_delay(&s->limit, + n * BDRV_SECTOR_SIZE); } } @@ -231,7 +232,7 @@ static void commit_set_speed(BlockJob *job, int64_t speed, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER, "speed"); return; } -ratelimit_set_speed(&s->limit, speed / BDRV_SECTOR_SIZE, SLICE_TIME); +ratelimit_set_speed(&s->limit, speed, SLICE_TIME); } static const BlockJobDriver commit_job_driver = { diff --git a/block/mirror.c b/block/mirror.c index 61a862d..eb27efc 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -396,7 +396,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) bitmap_set(s->in_flight_bitmap, sector_num / sectors_per_chunk, nb_chunks); while (nb_chunks > 0 && sector_num < end) { int64_t ret; -int io_sectors, io_sectors_acct; +int io_sectors; +int64_t io_bytes_acct; BlockDriverState *file; enum MirrorMethod { MIRROR_METHOD_COPY, @@ -444,16 +445,16 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) switch (mirror_method) { case MIRROR_METHOD_COPY: io_sectors = mirror_do_read(s, sector_num, io_sectors); -io_sectors_acct = io_sectors; +io_bytes_acct = io_sectors * BDRV_SECTOR_SIZE; break; case MIRROR_METHOD_ZERO: case MIRROR_METHOD_DISCARD: mirror_do_zero_or_discard(s, sector_num, io_sectors, mirror_method == MIRROR_METHOD_DISCARD); if (write_zeroes_ok) { -io_sectors_acct = 0; +io_bytes_acct = 0; } else { -io_sectors_acct = io_sectors; +io_bytes_acct = io_sectors * BDRV_SECTOR_SIZE; } break; default: @@ -463,7 +464,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) sector_num += io_sectors; nb_chunks -= DIV_ROUND_UP(io_sectors, sectors_per_chunk);
[Qemu-devel] [PATCH v3 7/7] target/m68k: add fmovem
Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/fpu_helper.c | 120 +++ target/m68k/helper.h | 6 +++ target/m68k/translate.c | 93 3 files changed, 189 insertions(+), 30 deletions(-) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index 41ab5ce..60b11ce 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -22,6 +22,7 @@ #include "cpu.h" #include "exec/helper-proto.h" #include "exec/exec-all.h" +#include "exec/cpu_ldst.h" static const floatx80 fpu_rom[128] = { [0x00] = floatx80_pi, /* Pi */ @@ -384,3 +385,122 @@ void HELPER(fconst)(CPUM68KState *env, FPReg *val, uint32_t offset) { val->d = fpu_rom[offset]; } + +typedef int (*float_access)(CPUM68KState *env, uint32_t addr, FPReg *fp, +uintptr_t ra); + +static uint32_t fmovem_predec(CPUM68KState *env, uint32_t addr, uint32_t mask, + float_access access) +{ +uintptr_t ra = GETPC(); +int i, size; + +for (i = 7; i >= 0; i--, mask <<= 1) { +if (mask & 0x80) { +size = access(env, addr, &env->fregs[i], ra); +if ((mask & 0xff) != 0x80) { +addr -= size; +} +} +} + +return addr; +} + +static uint32_t fmovem_postinc(CPUM68KState *env, uint32_t addr, uint32_t mask, + float_access access) +{ +uintptr_t ra = GETPC(); +int i, size; + +for (i = 0; i < 8; i++, mask <<= 1) { +if (mask & 0x80) { +size = access(env, addr, &env->fregs[i], ra); +addr += size; +} +} + +return addr; +} + +static int cpu_ld_floatx80_ra(CPUM68KState *env, uint32_t addr, FPReg *fp, + uintptr_t ra) +{ +uint32_t high; +uint64_t low; + +high = cpu_ldl_data_ra(env, addr, ra); +low = cpu_ldq_data_ra(env, addr + 4, ra); + +fp->l.upper = high >> 16; +fp->l.lower = low; + +return 12; +} + +static int cpu_st_floatx80_ra(CPUM68KState *env, uint32_t addr, FPReg *fp, + uintptr_t ra) +{ +cpu_stl_data_ra(env, addr, fp->l.upper << 16, ra); +cpu_stq_data_ra(env, addr + 4, fp->l.lower, ra); + +return 12; +} + +static int cpu_ld_float64_ra(CPUM68KState *env, uint32_t addr, FPReg *fp, + uintptr_t ra) +{ +uint64_t val; + +val = cpu_ldq_data_ra(env, addr, ra); +fp->d = float64_to_floatx80(*(float64 *)&val, &env->fp_status); + +return 8; +} + +static int cpu_st_float64_ra(CPUM68KState *env, uint32_t addr, FPReg *fp, + uintptr_t ra) +{ +float64 val; + +val = floatx80_to_float64(fp->d, &env->fp_status); +cpu_stq_data_ra(env, addr, *(uint64_t *)&val, ra); + +return 8; +} + +uint32_t HELPER(fmovemx_st_predec)(CPUM68KState *env, uint32_t addr, + uint32_t mask) +{ +return fmovem_predec(env, addr, mask, cpu_st_floatx80_ra); +} + +uint32_t HELPER(fmovemx_st_postinc)(CPUM68KState *env, uint32_t addr, +uint32_t mask) +{ +return fmovem_postinc(env, addr, mask, cpu_st_floatx80_ra); +} + +uint32_t HELPER(fmovemx_ld_postinc)(CPUM68KState *env, uint32_t addr, +uint32_t mask) +{ +return fmovem_postinc(env, addr, mask, cpu_ld_floatx80_ra); +} + +uint32_t HELPER(fmovemd_st_predec)(CPUM68KState *env, uint32_t addr, + uint32_t mask) +{ +return fmovem_predec(env, addr, mask, cpu_st_float64_ra); +} + +uint32_t HELPER(fmovemd_st_postinc)(CPUM68KState *env, uint32_t addr, +uint32_t mask) +{ +return fmovem_postinc(env, addr, mask, cpu_st_float64_ra); +} + +uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr, +uint32_t mask) +{ +return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra); +} diff --git a/target/m68k/helper.h b/target/m68k/helper.h index b396899..475a1f2 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -54,6 +54,12 @@ DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp) DEF_HELPER_3(fconst, void, env, fp, i32) +DEF_HELPER_3(fmovemx_st_predec, i32, env, i32, i32) +DEF_HELPER_3(fmovemx_st_postinc, i32, env, i32, i32) +DEF_HELPER_3(fmovemx_ld_postinc, i32, env, i32, i32) +DEF_HELPER_3(fmovemd_st_predec, i32, env, i32, i32) +DEF_HELPER_3(fmovemd_st_postinc, i32, env, i32, i32) +DEF_HELPER_3(fmovemd_ld_postinc, i32, env, i32, i32) DEF_HELPER_3(mac_move, void, env, i32, i32) DEF_HELPER_3(macmulf, i64, env, i32, i32) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 836b80d..1c60664 100644 --- a/target/m68
Re: [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns
On Tue, Jun 27, 2017 at 02:51:52PM -0400, Emilio G. Cota wrote: > On Fri, Dec 02, 2016 at 22:01:52 +0100, Laszlo Ersek wrote: > > When passed to git-diff (and to every other git command producing diffs > > and/or diffstats) with "-O" or "diff.orderFile", this list of patterns > > will place the more declarative / abstract hunks first, while changes to > > imperative code / details will be near the end of the patches. This saves > > on scrolling / searching and makes for easier reviewing. > > > > We intend to advise contributors in the Wiki to run > > > > git config diff.orderFile scripts/git.orderfile > > > > once, as part of their initial setup, before formatting their first (or, > > for repeat contributors, next) patches. > > > > See the "-O" option and the "diff.orderFile" configuration variable in > > git-diff(1) and git-config(1). > > > > Cc: "Michael S. Tsirkin" > > Cc: Eric Blake > > Cc: Fam Zheng > > Cc: Gerd Hoffmann > > Cc: John Snow > > Cc: Max Reitz > > Cc: Stefan Hajnoczi > > Signed-off-by: Laszlo Ersek > > --- > > Refloating 6+ months later, but.. > > Someone please merge this! :-) > > E. Users should be aware that if they do this, the need to supply -O /dev/null to git pull-request. Otherwise the diffstat is all scrambled and you can't figure out what's included. -- MST
[Qemu-devel] [PATCH v3 3/7] target/m68k: add explicit single and double precision operations
Add fssqrt, fdsqrt, fsadd, fdadd, fssub, fdsub, fsmul, fdmul, fsdiv, fddiv. The precision is managed using set_floatx80_rounding_precision(). Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/fpu_helper.c | 80 target/m68k/helper.h | 10 ++ target/m68k/translate.c | 40 +--- 3 files changed, 125 insertions(+), 5 deletions(-) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index 912c0b7..f6b6788 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -153,11 +153,35 @@ void HELPER(set_fpcr)(CPUM68KState *env, uint32_t val) cpu_m68k_set_fpcr(env, val); } +#define PREC_BEGIN(prec)\ +do {\ +int old;\ +old = get_floatx80_rounding_precision(&env->fp_status); \ +set_floatx80_rounding_precision(prec, &env->fp_status) \ + +#define PREC_END() \ +set_floatx80_rounding_precision(old, &env->fp_status); \ +} while (0) + void HELPER(fsqrt)(CPUM68KState *env, FPReg *res, FPReg *val) { res->d = floatx80_sqrt(val->d, &env->fp_status); } +void HELPER(fssqrt)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(32); +res->d = floatx80_sqrt(val->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fdsqrt)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(64); +res->d = floatx80_sqrt(val->d, &env->fp_status); +PREC_END(); +} + void HELPER(fabs)(CPUM68KState *env, FPReg *res, FPReg *val) { res->d = floatx80_abs(val->d); @@ -173,21 +197,77 @@ void HELPER(fadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) res->d = floatx80_add(val0->d, val1->d, &env->fp_status); } +void HELPER(fsadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(32); +res->d = floatx80_add(val0->d, val1->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fdadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(64); +res->d = floatx80_add(val0->d, val1->d, &env->fp_status); +PREC_END(); +} + void HELPER(fsub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) { res->d = floatx80_sub(val1->d, val0->d, &env->fp_status); } +void HELPER(fssub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(32); +res->d = floatx80_sub(val1->d, val0->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fdsub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(64); +res->d = floatx80_sub(val1->d, val0->d, &env->fp_status); +PREC_END(); +} + void HELPER(fmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) { res->d = floatx80_mul(val0->d, val1->d, &env->fp_status); } +void HELPER(fsmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(32); +res->d = floatx80_mul(val0->d, val1->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fdmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(64); +res->d = floatx80_mul(val0->d, val1->d, &env->fp_status); +PREC_END(); +} + void HELPER(fdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) { res->d = floatx80_div(val1->d, val0->d, &env->fp_status); } +void HELPER(fsdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(32); +res->d = floatx80_div(val1->d, val0->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fddiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +PREC_BEGIN(64); +res->d = floatx80_div(val1->d, val0->d, &env->fp_status); +PREC_END(); +} + static int float_comp_to_cc(int float_compare) { switch (float_compare) { diff --git a/target/m68k/helper.h b/target/m68k/helper.h index d6e80e4..0c7f06f 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -26,12 +26,22 @@ DEF_HELPER_2(reds32, s32, env, fp) DEF_HELPER_3(firound, void, env, fp, fp) DEF_HELPER_3(fitrunc, void, env, fp, fp) DEF_HELPER_3(fsqrt, void, env, fp, fp) +DEF_HELPER_3(fssqrt, void, env, fp, fp) +DEF_HELPER_3(fdsqrt, void, env, fp, fp) DEF_HELPER_3(fabs, void, env, fp, fp) DEF_HELPER_3(fchs, void, env, fp, fp) DEF_HELPER_4(fadd, void, env, fp, fp, fp) +DEF_HELPER_4(fsadd, void, env, fp, fp, fp) +DEF_HELPER_4(fdadd, void, env, fp, fp, fp) DEF_HELPER_4(fsub, void, env, fp, fp, fp) +DEF_HELPER_4(fssub, void, env, fp, fp, fp) +DEF_HELPER_4(fdsub, void, env, fp, fp, fp) DEF_HELPER_4(fmul, void, env, fp, fp, fp) +DEF_HELPER_4(fsmul, void, env, fp, fp, fp) +DEF_HELPER_4(fdmul, void, env, fp, fp, fp) DEF_HELPER_4(fdiv, void, env, fp, fp, fp) +DEF_HELPER_4(fsdiv, void, env, fp, fp, fp) +DEF_HELPER_4(fddiv, void, env, fp, fp, fp) DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) D
[Qemu-devel] [PATCH v3 0/7] target/m68k: implement 680x0 FPU (part 2)
Second part of patches submitted in the v3. This series adds a subset of single precision and double precision instructions using set_floatx80_rounding_precision() to round the result. For some other instructions, we introduce a new function, floatx80_round(), to round them manually. It also adds instructions fsglmul, fsgldiv, fmovecr and fscc (since v3, use tcg_gen_setcond()) fmovem manages static and dynamic register list, all the code has been moved to an helper. v3: Remove extra space in fmovem patch truncate operands of fsglmul and fslgdiv update gen_fcc_cond() as suggested by Richard fmoverc: use opmode instead of rom_offset, define and free the tcg constant v2: Fix gen_fcc_cond(): g[12] mark global variables, not variables to free. add 2 patches to compute trigonometric functions and fmod use floatx80_round() to compute fsglmul and fsgldiv split fmovem helper into 6 helpers Laurent Vivier (7): target/m68k: add fscc. target/m68k: add fmovecr target/m68k: add explicit single and double precision operations softfloat: define floatx80_round() target/m68k: add fsglmul and fsgldiv target/m68k: add explicit single and double precision operations (part 2) target/m68k: add fmovem fpu/softfloat.c | 15 ++ include/fpu/softfloat.h | 1 + target/m68k/fpu_helper.c | 306 - target/m68k/helper.h | 27 +++- target/m68k/translate.c | 389 --- 5 files changed, 615 insertions(+), 123 deletions(-) -- 2.9.4
[Qemu-devel] [PATCH v3 5/7] target/m68k: add fsglmul and fsgldiv
fsglmul and fsgldiv truncate data to single precision before computing results. Signed-off-by: Laurent Vivier --- target/m68k/fpu_helper.c | 28 target/m68k/helper.h | 2 ++ target/m68k/translate.c | 6 ++ 3 files changed, 36 insertions(+) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index f6b6788..0daad4a 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -249,6 +249,20 @@ void HELPER(fdmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) PREC_END(); } +void HELPER(fsglmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +int rounding_mode = get_float_rounding_mode(&env->fp_status); +floatx80 a, b; + +PREC_BEGIN(32); +set_float_rounding_mode(float_round_to_zero, &env->fp_status); +a = floatx80_round(val0->d, &env->fp_status); +b = floatx80_round(val1->d, &env->fp_status); +set_float_rounding_mode(rounding_mode, &env->fp_status); +res->d = floatx80_mul(a, b, &env->fp_status); +PREC_END(); +} + void HELPER(fdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) { res->d = floatx80_div(val1->d, val0->d, &env->fp_status); @@ -268,6 +282,20 @@ void HELPER(fddiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) PREC_END(); } +void HELPER(fsgldiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) +{ +int rounding_mode = get_float_rounding_mode(&env->fp_status); +floatx80 a, b; + +PREC_BEGIN(32); +set_float_rounding_mode(float_round_to_zero, &env->fp_status); +a = floatx80_round(val1->d, &env->fp_status); +b = floatx80_round(val0->d, &env->fp_status); +set_float_rounding_mode(rounding_mode, &env->fp_status); +res->d = floatx80_div(a, b, &env->fp_status); +PREC_END(); +} + static int float_comp_to_cc(int float_compare) { switch (float_compare) { diff --git a/target/m68k/helper.h b/target/m68k/helper.h index 0c7f06f..f05191b 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -39,9 +39,11 @@ DEF_HELPER_4(fdsub, void, env, fp, fp, fp) DEF_HELPER_4(fmul, void, env, fp, fp, fp) DEF_HELPER_4(fsmul, void, env, fp, fp, fp) DEF_HELPER_4(fdmul, void, env, fp, fp, fp) +DEF_HELPER_4(fsglmul, void, env, fp, fp, fp) DEF_HELPER_4(fdiv, void, env, fp, fp, fp) DEF_HELPER_4(fsdiv, void, env, fp, fp, fp) DEF_HELPER_4(fddiv, void, env, fp, fp, fp) +DEF_HELPER_4(fsgldiv, void, env, fp, fp, fp) DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 84f78f9..4775770 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4646,6 +4646,12 @@ DISAS_INSN(fpu) case 0x67: /* fdmul */ gen_helper_fdmul(cpu_env, cpu_dest, cpu_src, cpu_dest); break; +case 0x24: /* fsgldiv */ +gen_helper_fsgldiv(cpu_env, cpu_dest, cpu_src, cpu_dest); +break; +case 0x27: /* fsglmul */ +gen_helper_fsglmul(cpu_env, cpu_dest, cpu_src, cpu_dest); +break; case 0x28: /* fsub */ gen_helper_fsub(cpu_env, cpu_dest, cpu_src, cpu_dest); break; -- 2.9.4
[Qemu-devel] [PATCH v3 1/7] target/m68k: add fscc.
use DisasCompare with FPU conditions in fscc and fbcc. Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 211 ++-- 1 file changed, 132 insertions(+), 79 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 7aa0fdc..dff604c 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4633,142 +4633,194 @@ undef: disas_undef_fpu(env, s, insn); } -DISAS_INSN(fbcc) +static void gen_fcc_cond(DisasCompare *c, DisasContext *s, int cond) { -uint32_t offset; -uint32_t addr; -TCGLabel *l1; -TCGv tmp, fpsr; - -addr = s->pc; -offset = cpu_ldsw_code(env, s->pc); -s->pc += 2; -if (insn & (1 << 6)) { -offset = (offset << 16) | read_im16(env, s); -} +TCGv fpsr; +c->g1 = 1; +c->v2 = tcg_const_i32(0); +c->g2 = 0; +/* TODO: Raise BSUN exception. */ fpsr = tcg_temp_new(); gen_load_fcr(s, fpsr, M68K_FPSR); -l1 = gen_new_label(); -/* TODO: Raise BSUN exception. */ -/* Jump to l1 if condition is true. */ -switch (insn & 0x3f) { +switch (cond) { case 0: /* False */ case 16: /* Signaling False */ +c->v1 = c->v2; +c->tcond = TCG_COND_NEVER; break; case 1: /* EQual Z */ case 17: /* Signaling EQual Z */ -tmp = tcg_temp_new(); -tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_Z); -tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_Z); +c->tcond = TCG_COND_NE; break; case 2: /* Ordered Greater Than !(A || Z || N) */ case 18: /* Greater Than !(A || Z || N) */ -tmp = tcg_temp_new(); -tcg_gen_andi_i32(tmp, fpsr, +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A | FPSR_CC_Z | FPSR_CC_N); -tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1); +c->tcond = TCG_COND_EQ; break; case 3: /* Ordered Greater than or Equal Z || !(A || N) */ case 19: /* Greater than or Equal Z || !(A || N) */ -assert(FPSR_CC_A == (FPSR_CC_N >> 3)); -tmp = tcg_temp_new(); -tcg_gen_shli_i32(tmp, fpsr, 3); -tcg_gen_or_i32(tmp, tmp, fpsr); -tcg_gen_xori_i32(tmp, tmp, FPSR_CC_N); -tcg_gen_andi_i32(tmp, tmp, FPSR_CC_N | FPSR_CC_Z); -tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1); +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_shli_i32(c->v1, fpsr, 3); +tcg_gen_or_i32(c->v1, c->v1, fpsr); +tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N); +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_Z); +c->tcond = TCG_COND_NE; break; case 4: /* Ordered Less Than !(!N || A || Z); */ case 20: /* Less Than !(!N || A || Z); */ -tmp = tcg_temp_new(); -tcg_gen_xori_i32(tmp, fpsr, FPSR_CC_N); -tcg_gen_andi_i32(tmp, tmp, FPSR_CC_N | FPSR_CC_A | FPSR_CC_Z); -tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_xori_i32(c->v1, fpsr, FPSR_CC_N); +tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_A | FPSR_CC_Z); +c->tcond = TCG_COND_EQ; break; case 5: /* Ordered Less than or Equal Z || (N && !A) */ case 21: /* Less than or Equal Z || (N && !A) */ -assert(FPSR_CC_A == (FPSR_CC_N >> 3)); -tmp = tcg_temp_new(); -tcg_gen_xori_i32(tmp, fpsr, FPSR_CC_A); -tcg_gen_shli_i32(tmp, tmp, 3); -tcg_gen_ori_i32(tmp, tmp, FPSR_CC_Z); -tcg_gen_and_i32(tmp, tmp, fpsr); -tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1); +g_assert(FPSR_CC_A == (FPSR_CC_N >> 3)); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_xori_i32(c->v1, fpsr, FPSR_CC_A); +tcg_gen_shli_i32(c->v1, c->v1, 3); +tcg_gen_ori_i32(c->v1, c->v1, FPSR_CC_Z); +tcg_gen_and_i32(c->v1, c->v1, fpsr); +c->tcond = TCG_COND_NE; break; case 6: /* Ordered Greater or Less than !(A || Z) */ case 22: /* Greater or Less than !(A || Z) */ -tmp = tcg_temp_new(); -tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_A | FPSR_CC_Z); -tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A | FPSR_CC_Z); +c->tcond = TCG_COND_EQ; break; case 7: /* Ordered !A */ case 23: /* Greater, Less or Equal !A */ -tmp = tcg_temp_new(); -tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_A); -tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1); +c->v1 = tcg_temp_new(); +c->g1 = 0; +tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A); +c->tcond = TCG_COND_EQ; break; case 8: /* Unordered A */
[Qemu-devel] [PATCH v3 2/7] target/m68k: add fmovecr
fmovecr moves a floating point constant from the FPU ROM to a floating point register. Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/fpu_helper.c | 30 ++ target/m68k/helper.h | 1 + target/m68k/translate.c | 13 - 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index a9e17f5..912c0b7 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -23,6 +23,31 @@ #include "exec/helper-proto.h" #include "exec/exec-all.h" +static const floatx80 fpu_rom[128] = { +[0x00] = floatx80_pi, /* Pi */ +[0x0b] = make_floatx80(0x3ffd, 0x9a209a84fbcff798ULL), /* Log10(2) */ +[0x0c] = make_floatx80(0x4000, 0xadf85458a2bb4a9aULL), /* e*/ +[0x0d] = make_floatx80(0x3fff, 0xb8aa3b295c17f0bcULL), /* Log2(e) */ +[0x0e] = make_floatx80(0x3ffd, 0xde5bd8a937287195ULL), /* Log10(e) */ +[0x0f] = floatx80_zero, /* Zero */ +[0x30] = floatx80_ln2, /* ln(2)*/ +[0x31] = make_floatx80(0x4000, 0x935d8dddaaa8ac17ULL), /* ln(10) */ +[0x32] = floatx80_one, /* 10^0 */ +[0x33] = make_floatx80(0x4002, 0xa000ULL), /* 10^1 */ +[0x34] = make_floatx80(0x4005, 0xc800ULL), /* 10^2 */ +[0x35] = make_floatx80(0x400c, 0x9c40ULL), /* 10^4 */ +[0x36] = make_floatx80(0x4019, 0xbebc2000ULL), /* 10^8 */ +[0x37] = make_floatx80(0x4034, 0x8e1bc9bf0400ULL), /* 10^16*/ +[0x38] = make_floatx80(0x4069, 0x9dc5ada82b70b59eULL), /* 10^32*/ +[0x39] = make_floatx80(0x40d3, 0xc2781f49ffcfa6d5ULL), /* 10^64*/ +[0x3a] = make_floatx80(0x41a8, 0x93ba47c980e98ce0ULL), /* 10^128 */ +[0x3b] = make_floatx80(0x4351, 0xaa7eebfb9df9de8eULL), /* 10^256 */ +[0x3c] = make_floatx80(0x46a3, 0xe319a0aea60e91c7ULL), /* 10^512 */ +[0x3d] = make_floatx80(0x4d48, 0xc976758681750c17ULL), /* 10^1024 */ +[0x3e] = make_floatx80(0x5a92, 0x9e8b3b5dc53d5de5ULL), /* 10^2048 */ +[0x3f] = make_floatx80(0x7525, 0xc46052028a20979bULL), /* 10^4096 */ +}; + int32_t HELPER(reds32)(CPUM68KState *env, FPReg *val) { return floatx80_to_int32(val->d, &env->fp_status); @@ -204,3 +229,8 @@ void HELPER(ftst)(CPUM68KState *env, FPReg *val) } env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | cc; } + +void HELPER(fconst)(CPUM68KState *env, FPReg *val, uint32_t offset) +{ +val->d = fpu_rom[offset]; +} diff --git a/target/m68k/helper.h b/target/m68k/helper.h index 98cbf18..d6e80e4 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -35,6 +35,7 @@ DEF_HELPER_4(fdiv, void, env, fp, fp, fp) DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp) +DEF_HELPER_3(fconst, void, env, fp, i32) DEF_HELPER_3(mac_move, void, env, i32, i32) DEF_HELPER_3(macmulf, i64, env, i32, i32) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index dff604c..0bb3300 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4518,10 +4518,21 @@ DISAS_INSN(fpu) ext = read_im16(env, s); opmode = ext & 0x7f; switch ((ext >> 13) & 7) { -case 0: case 2: +case 0: break; case 1: goto undef; +case 2: +if (insn == 0xf200 && (ext & 0xfc00) == 0x5c00) { +/* fmovecr */ +TCGv rom_offset = tcg_const_i32(opmode); +cpu_dest = gen_fp_ptr(REG(ext, 7)); +gen_helper_fconst(cpu_env, cpu_dest, rom_offset); +tcg_temp_free_ptr(cpu_dest); +tcg_temp_free(rom_offset); +return; +} +break; case 3: /* fmove out */ cpu_src = gen_fp_ptr(REG(ext, 7)); opsize = ext_opsize(ext, 10); -- 2.9.4
[Qemu-devel] [PATCH v3 6/7] target/m68k: add explicit single and double precision operations (part 2)
Add fsabs, fdabs, fsneg, fdneg, fsmove and fdmove. The value is converted using the new floatx80_round() function. Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- target/m68k/fpu_helper.c | 48 +--- target/m68k/helper.h | 8 +++- target/m68k/translate.c | 26 ++ 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index 0daad4a..41ab5ce 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -163,6 +163,20 @@ void HELPER(set_fpcr)(CPUM68KState *env, uint32_t val) set_floatx80_rounding_precision(old, &env->fp_status); \ } while (0) +void HELPER(fsround)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(32); +res->d = floatx80_round(val->d, &env->fp_status); +PREC_END(); +} + +void HELPER(fdround)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(64); +res->d = floatx80_round(val->d, &env->fp_status); +PREC_END(); +} + void HELPER(fsqrt)(CPUM68KState *env, FPReg *res, FPReg *val) { res->d = floatx80_sqrt(val->d, &env->fp_status); @@ -184,12 +198,40 @@ void HELPER(fdsqrt)(CPUM68KState *env, FPReg *res, FPReg *val) void HELPER(fabs)(CPUM68KState *env, FPReg *res, FPReg *val) { -res->d = floatx80_abs(val->d); +res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status); +} + +void HELPER(fsabs)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(32); +res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status); +PREC_END(); +} + +void HELPER(fdabs)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(64); +res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status); +PREC_END(); +} + +void HELPER(fneg)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status); } -void HELPER(fchs)(CPUM68KState *env, FPReg *res, FPReg *val) +void HELPER(fsneg)(CPUM68KState *env, FPReg *res, FPReg *val) { -res->d = floatx80_chs(val->d); +PREC_BEGIN(32); +res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status); +PREC_END(); +} + +void HELPER(fdneg)(CPUM68KState *env, FPReg *res, FPReg *val) +{ +PREC_BEGIN(64); +res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status); +PREC_END(); } void HELPER(fadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1) diff --git a/target/m68k/helper.h b/target/m68k/helper.h index f05191b..b396899 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -23,13 +23,19 @@ DEF_HELPER_2(redf32, f32, env, fp) DEF_HELPER_2(redf64, f64, env, fp) DEF_HELPER_2(reds32, s32, env, fp) +DEF_HELPER_3(fsround, void, env, fp, fp) +DEF_HELPER_3(fdround, void, env, fp, fp) DEF_HELPER_3(firound, void, env, fp, fp) DEF_HELPER_3(fitrunc, void, env, fp, fp) DEF_HELPER_3(fsqrt, void, env, fp, fp) DEF_HELPER_3(fssqrt, void, env, fp, fp) DEF_HELPER_3(fdsqrt, void, env, fp, fp) DEF_HELPER_3(fabs, void, env, fp, fp) -DEF_HELPER_3(fchs, void, env, fp, fp) +DEF_HELPER_3(fsabs, void, env, fp, fp) +DEF_HELPER_3(fdabs, void, env, fp, fp) +DEF_HELPER_3(fneg, void, env, fp, fp) +DEF_HELPER_3(fsneg, void, env, fp, fp) +DEF_HELPER_3(fdneg, void, env, fp, fp) DEF_HELPER_4(fadd, void, env, fp, fp, fp) DEF_HELPER_4(fsadd, void, env, fp, fp, fp) DEF_HELPER_4(fdadd, void, env, fp, fp, fp) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 4775770..836b80d 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4595,9 +4595,15 @@ DISAS_INSN(fpu) } cpu_dest = gen_fp_ptr(REG(ext, 7)); switch (opmode) { -case 0: case 0x40: case 0x44: /* fmove */ +case 0: /* fmove */ gen_fp_move(cpu_dest, cpu_src); break; +case 0x40: /* fsmove */ +gen_helper_fsround(cpu_env, cpu_dest, cpu_src); +break; +case 0x44: /* fdmove */ +gen_helper_fdround(cpu_env, cpu_dest, cpu_src); +break; case 1: /* fint */ gen_helper_firound(cpu_env, cpu_dest, cpu_src); break; @@ -4613,11 +4619,23 @@ DISAS_INSN(fpu) case 0x45: /* fdsqrt */ gen_helper_fdsqrt(cpu_env, cpu_dest, cpu_src); break; -case 0x18: case 0x58: case 0x5c: /* fabs */ +case 0x18: /* fabs */ gen_helper_fabs(cpu_env, cpu_dest, cpu_src); break; -case 0x1a: case 0x5a: case 0x5e: /* fneg */ -gen_helper_fchs(cpu_env, cpu_dest, cpu_src); +case 0x58: /* fsabs */ +gen_helper_fsabs(cpu_env, cpu_dest, cpu_src); +break; +case 0x5c: /* fdabs */ +gen_helper_fdabs(cpu_env, cpu_dest, cpu_src); +break; +case 0x1a: /* fneg */ +gen_helper_fneg(cpu_env, cpu_dest, cpu_src); +break; +case 0x5a: /* fsneg */ +gen_helper_fsneg(cpu_env, cpu_dest, cpu_src); +break; +case 0x5e: /* fdneg */ +gen_helper_fdneg(cpu_env, cpu_dest, cpu_src)
[Qemu-devel] [PATCH v3 4/7] softfloat: define floatx80_round()
Add a function to round a floatx80 to the defined precision (floatx80_rounding_precision) Signed-off-by: Laurent Vivier Reviewed-by: Richard Henderson --- fpu/softfloat.c | 15 +++ include/fpu/softfloat.h | 1 + 2 files changed, 16 insertions(+) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index 7af14e2..e9bf359 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -5086,6 +5086,21 @@ float128 floatx80_to_float128(floatx80 a, float_status *status) } /* +| Rounds the extended double-precision floating-point value `a' +| and returns the result as an extended double-precision floating-point +| value. The operation is performed according to the IEC/IEEE Standard for +| Binary Floating-Point Arithmetic. +**/ + +floatx80 floatx80_round(floatx80 a, float_status *status) +{ +return roundAndPackFloatx80(status->floatx80_rounding_precision, +extractFloatx80Sign(a), +extractFloatx80Exp(a), +extractFloatx80Frac(a), 0, status); +} + +/* | Rounds the extended double-precision floating-point value `a' to an integer, | and returns the result as an extended quadruple-precision floating-point | value. The operation is performed according to the IEC/IEEE Standard for diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index f1288ef..d9689ec 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -621,6 +621,7 @@ float128 floatx80_to_float128(floatx80, float_status *status); /* | Software IEC/IEEE extended double-precision operations. **/ +floatx80 floatx80_round(floatx80 a, float_status *status); floatx80 floatx80_round_to_int(floatx80, float_status *status); floatx80 floatx80_add(floatx80, floatx80, float_status *status); floatx80 floatx80_sub(floatx80, floatx80, float_status *status); -- 2.9.4
Re: [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns
On Fri, Dec 02, 2016 at 22:01:52 +0100, Laszlo Ersek wrote: > When passed to git-diff (and to every other git command producing diffs > and/or diffstats) with "-O" or "diff.orderFile", this list of patterns > will place the more declarative / abstract hunks first, while changes to > imperative code / details will be near the end of the patches. This saves > on scrolling / searching and makes for easier reviewing. > > We intend to advise contributors in the Wiki to run > > git config diff.orderFile scripts/git.orderfile > > once, as part of their initial setup, before formatting their first (or, > for repeat contributors, next) patches. > > See the "-O" option and the "diff.orderFile" configuration variable in > git-diff(1) and git-config(1). > > Cc: "Michael S. Tsirkin" > Cc: Eric Blake > Cc: Fam Zheng > Cc: Gerd Hoffmann > Cc: John Snow > Cc: Max Reitz > Cc: Stefan Hajnoczi > Signed-off-by: Laszlo Ersek > --- Refloating 6+ months later, but.. Someone please merge this! :-) E.
Re: [Qemu-devel] [PATCH v1 1/1] char-socket: Don't report TCP socket waiting as an error
On Thu, Jun 8, 2017 at 10:16 PM, Markus Armbruster wrote: > Alistair Francis writes: > >> On Thu, Jun 8, 2017 at 10:56 AM, Markus Armbruster wrote: >>> Alistair Francis writes: >>> On Wed, Jun 7, 2017 at 11:03 PM, Markus Armbruster wrote: > Alistair Francis writes: > >> On Wed, Jun 7, 2017 at 12:19 AM, Markus Armbruster >> wrote: >>> Paolo Bonzini writes: >>> On 06/06/2017 18:30, Alistair Francis wrote: >> >> This is somehow confusing. I don't think it is worth having another >> qemu_log_stderr() function rather than using error_report() but this >> very >> call might deserve a comment explaining this unusual use. What do >> you think? > > The problem with stderr is that this isn't an error. Some uses of QEMU > (inside Eclipse for example) flag everything printed on stderr as red > which confuses users that they are seeing an error when they really > aren't. But they are wrong. >>> >>> Concur. We also print warnings and informational messages to stderr. >>> >>> We should make errors easy to recognize. Fortunately, error_report() >>> prints errors to stderr in a rigid format. Unfortunately, error >>> messages bypassing error_report() still exist in places. We suck. >>> >>> The format is >>> >>> timestamp-if-enabled progname ':' location message >>> >>> timestamp-if-enabled is normally empty. With -msg timestamp=on, it's >>> the current time in ISO 8601 format, followed by a space. >>> >>> progname is the program name (main()'s argv[0]). >>> >>> location is either empty, or a reference to the command line or a >>> configuration file. >>> >>> See error_vreport() for details. >> >> Ok, but this isn't an error, it's more information. So it sounds like >> we should still print to stderr but not print in the format described >> above? > > Yes. > > I explained the error message format to show how to distinguish actual > errors from other stuff. Sorry, I should have been more clear. I meant we should not use the error_report() function here. I don't think we have any warning_report() function though, is that something worth having? >>> >>> So far we simply use error_printf() for such things. >>> >>> A function to report a warning would let us report them more uniformly, >>> but only if we actually use it uniformly. In other words, adding one >>> without also converting the existing warnings to use it would create yet >>> another open-ended incremental conversion job. Are we up to it? >> >> Yeah! Why not. I am happy to give it a shot changing some errors to warnings. >> >> First thing though, what is the format for printing warnings? > > We make one up. > > For what it's worth, gcc uses the same format as for errors with the > message prefixed either by "warning: " or by "error: ". Also common is > prefixing warnings, but not errors. Ok, I think that is what I'm going to do. I will send a patch series out soon that basically copies the error_report() format but prefixes it with warning. I also removed the timestamp for the warning, although I'm not sure if that is what we want to do. I'll CC you when I send it out. Thanks, Alistair > > We already have several error_report() calls with messages that start > with (a variation of) "warning: ". >
Re: [Qemu-devel] [Qemu-block] [PATCH v4 0/4] more blkdebug tweaks
Am 27.06.2017 um 18:01 hat Eric Blake geschrieben: > On 06/06/2017 07:26 AM, Kevin Wolf wrote: > > Am 05.06.2017 um 22:38 hat Eric Blake geschrieben: > >> I found a crasher and some odd behavior while rebasing my > >> bdrv_get_block_status series, so I figured I'd get these things > >> fixed first. This is based on top of Max's block branch. > >> > >> Available as a tag at: > >> git fetch git://repo.or.cz/qemu/ericb.git nbd-blkdebug-status-v4 > >> > >> Since v3: > >> - check all qemu-iotests (patch 1) > > > > Whoops, somehow I ended up applying and reviewing v4 while looking at > > the v3 thread... Looks like there really aren't any missing test case > > updates any more. > > > > Thanks, applied to the block branch. > > Did this get lost somehow? No idea what happened there, but it looks like it. It's queued now. Kevin pgpsaDVH3a4pP.pgp Description: PGP signature