Re: [Qemu-devel] [Qemu-ppc] [PULL 0/9] ppc-for-2.12 queue 20180315
On 17.03.2018 12:30, BALATON Zoltan wrote: > On Sat, 17 Mar 2018, BALATON Zoltan wrote: >> On Sat, 17 Mar 2018, Peter Maydell wrote: [...] > /ppc64/boot-serial/sam460ex: > /home/petmay01/linaro/qemu-for-merges/target/ppc/translate.c:2979:15: > runtime error: load of value 85, which is not a valid value for type > 'bool' > OK > > Looks like you're not initializing ctx->lazy_tlb_flush for all > configs: > if (env->mmu_model == POWERPC_MMU_32B || > env->mmu_model == POWERPC_MMU_601 || > (env->mmu_model & POWERPC_MMU_64B)) > ctx->lazy_tlb_flush = true; > > should perhaps be > ctx->lazy_tlb_flush = > env->mmu_model == POWERPC_MMU_32B || > env->mmu_model == POWERPC_MMU_601 || > (env->mmu_model & POWERPC_MMU_64B); > > ? Uh.. maybe.. except I don't see anything in the series that would be likely to change that behaviour. >>> >>> I imagine it's "tests/boot-serial: Test the sam460ex board" -- >>> this code was previously not being exercised in 'make check', >>> and now it is. >> >> I'm not sure what could cause this in case of sam460ex. It has PPC440 >> which has POWERPC_MMU_BOOKE but the ppce500 should also have that and >> a similar u-boot and that does not produce this error. Is there maybe >> some initialisation of some structure I've missed somewhere? But these >> DisasContext structs seem to be internal to TCG so I'm not sure what >> could be missing outside of TCG to avoid this. Could be that the >> different u-boot version does something that triggers this while the >> one for ppce500 does not execute code that causes this warning during >> the test? > > Oops, replied too soon. I've checked e500 and it seems to have > POWERPC_MMU_BOOKE206 (I thought e500 was BookE but I don't know these > very well). Only bamboo, virtex-ml507 and sam460ex seem to be > POWERPC_MMU_BOOKE so if only the sam460ex test is added now and the > others were never tested then it could be this is the first time this is > catched. Right, bamboo and virtex-ml507 do not ship with a pre-built firmware image, so they are *not* tested in the boot-serial tester. ppce500 is the only embedded PPC board that is tested so far. So it's good that we finally have one more test case for an additional CPU type here :-) Thomas
[Qemu-devel] [Bug 1435359] Re: Booting kernel 3.19.2 fails most of the time
Ok, thanks, so I'm closing this now. ** Changed in: qemu Status: Incomplete => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1435359 Title: Booting kernel 3.19.2 fails most of the time Status in QEMU: Fix Released Bug description: Host system: openSuSE 13.2 + kernel 4.0.0-rc4 + qemu 2.2.1. When I try to boot a virtual machine with Ubuntu 14.10 and kernel 3.13.0 every boot succeeds. However, with kernel 3.19.2 booting fails most of the time. The following appears in /var/log/libvirt/qemu /ubuntu-vm.log when I try to boot that VM with kernel 3.19.2: 2015-03-23 02:44:18.801+: starting up LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name ubuntu-vm -S -machine pc-i440fx-2.1,accel=kvm,usb=off -cpu Haswell -m 2048 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid 395110dc-9fbe-4542-8fce-4ef958f24b2c -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/ubuntu-vm.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 -drive file=/var/lib/libvirt/images/ubuntusaucy.qcow2,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1 -drive file=/var/lib/libvirt/images/ubuntu-14.04-mini.iso,if=none,id=drive-ide0-0-0,readonly=on,format=raw -device ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=2 -netdev tap,fd=22,id=hostnet0,vhost=on,vhostfd=23 -device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:5e:71:5e,bus=pci.0,addr=0x3 -chardev spicevmc,id=charchannel0,name=vdagent -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0 -spice port=5900,addr=127.0.0.1,disable-ticketing,seamless-migration=on -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,bus=pci.0,addr=0x2 -chardev spicevmc,id=charredir0,name=usbredir -device usb-redir,chardev=charredir0,id=redir0 -chardev spicevmc,id=charredir1,name=usbredir -device usb-redir,chardev=charredir1,id=redir1 -chardev spicevmc,id=charredir2,name=usbredir -device usb-redir,chardev=charredir2,id=redir2 -chardev spicevmc,id=charredir3,name=usbredir -device usb-redir,chardev=charredir3,id=redir3 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-pci,rng=rng0,bus=pci.0,addr=0x9 -msg timestamp=on main_channel_link: add main channel client main_channel_handle_parsed: net test: latency 0.229000 ms, bitrate 284 bps (27126.736111 Mbps) red_dispatcher_set_cursor_peer: inputs_connect: inputs channel client create ((null):30728): SpiceWorker-ERROR **: red_worker.c:8337:red_marshall_qxl_drawable: invalid type KVM: injection failed, MSI lost (Input/output error) qemu-system-x86_64: /home/bart/software/qemu-2.2.1/hw/net/vhost_net.c:264: vhost_net_stop_one: Assertion `r >= 0' failed. 2015-03-23 02:44:44.952+: shutting down That message is similar to the message reported by the older qemu version provided by openSuse (qemu 2.1.0 + qemu-kvm 2.1.0): 2015-03-21 13:51:00.724+: starting up LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin QEMU_AUDIO_DRV=spice /usr/bin/qemu-system-x86_64 -name ubuntu-vm -S -machine pc-i440fx-2.1,accel=kvm,usb=off -cpu Haswell -m 1024 -realtime mlock=off -smp 4,sockets=4,cores=1,threads=1 -uuid 395110dc-9fbe-4542-8fce-4ef958f24b2c -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/ubuntu-vm.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew -global kvm-pit.lost_tick_policy=discard -no-hpet -no-shutdown -global PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr =0x5 -device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1 -device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2 -device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive file=/var/lib/libvirt/images/ubuntusaucy.qcow2,if=none,id=drive-virtio-disk0,format=qcow2 -device virtio-blk
[Qemu-devel] [PATCH v2 3/6] virtio: support adding sub-regions for notify region
Provide APIs to support querying whether the page-per-vq is enabled and adding sub-regions for notify region. Signed-off-by: Tiwei Bie --- Makefile.target| 4 hw/virtio/virtio-pci.c | 48 ++ hw/virtio/virtio-pci.h | 5 + hw/virtio/virtio.c | 39 + include/hw/virtio/virtio.h | 5 + include/qemu/osdep.h | 1 + scripts/create_config | 3 +++ 7 files changed, 105 insertions(+) diff --git a/Makefile.target b/Makefile.target index 6549481096..b2cf618dc9 100644 --- a/Makefile.target +++ b/Makefile.target @@ -39,6 +39,9 @@ STPFILES= config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak +config-devices.h: config-devices.h-timestamp +config-devices.h-timestamp: config-devices.mak + ifdef CONFIG_TRACE_SYSTEMTAP stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp @@ -224,4 +227,5 @@ ifdef CONFIG_TRACE_SYSTEMTAP endif GENERATED_FILES += config-target.h +GENERATED_FILES += config-devices.h Makefile: $(GENERATED_FILES) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 1e8ab7bbc5..b17471092a 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1534,6 +1534,54 @@ static void virtio_pci_modern_io_region_unmap(VirtIOPCIProxy *proxy, ®ion->mr); } +static VirtIOPCIProxy *virtio_device_to_virtio_pci_proxy(VirtIODevice *vdev) +{ +VirtIOPCIProxy *proxy = NULL; + +if (vdev->device_id == VIRTIO_ID_NET) { +VirtIONetPCI *d = container_of(vdev, VirtIONetPCI, vdev.parent_obj); +proxy = &d->parent_obj; +} + +return proxy; +} + +bool virtio_pci_page_per_vq_enabled(VirtIODevice *vdev) +{ +VirtIOPCIProxy *proxy = virtio_device_to_virtio_pci_proxy(vdev); + +if (proxy == NULL) { +return false; +} + +return !!(proxy->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ); +} + +int virtio_pci_notify_region_map(VirtIODevice *vdev, int queue_idx, + MemoryRegion *mr) +{ +VirtIOPCIProxy *proxy = virtio_device_to_virtio_pci_proxy(vdev); +int offset; + +if (proxy == NULL || !virtio_pci_modern(proxy)) { +return -1; +} + +offset = virtio_pci_queue_mem_mult(proxy) * queue_idx; +memory_region_add_subregion(&proxy->notify.mr, offset, mr); + +return 0; +} + +void virtio_pci_notify_region_unmap(VirtIODevice *vdev, MemoryRegion *mr) +{ +VirtIOPCIProxy *proxy = virtio_device_to_virtio_pci_proxy(vdev); + +if (proxy != NULL) { +memory_region_del_subregion(&proxy->notify.mr, mr); +} +} + static void virtio_pci_pre_plugged(DeviceState *d, Error **errp) { VirtIOPCIProxy *proxy = VIRTIO_PCI(d); diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 813082b0d7..8061133741 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -213,6 +213,11 @@ static inline void virtio_pci_disable_modern(VirtIOPCIProxy *proxy) proxy->disable_modern = true; } +bool virtio_pci_page_per_vq_enabled(VirtIODevice *vdev); +int virtio_pci_notify_region_map(VirtIODevice *vdev, int queue_idx, + MemoryRegion *mr); +void virtio_pci_notify_region_unmap(VirtIODevice *vdev, MemoryRegion *mr); + /* * virtio-scsi-pci: This extends VirtioPCIProxy. */ diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 006d3d1148..90ee72984c 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -22,6 +22,7 @@ #include "qemu/atomic.h" #include "hw/virtio/virtio-bus.h" #include "hw/virtio/virtio-access.h" +#include "hw/virtio/virtio-pci.h" #include "sysemu/dma.h" /* @@ -2681,6 +2682,44 @@ void virtio_device_release_ioeventfd(VirtIODevice *vdev) virtio_bus_release_ioeventfd(vbus); } +bool virtio_device_parent_is_pci_device(VirtIODevice *vdev) +{ +BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); +const char *typename = object_get_typename(OBJECT(qbus->parent)); + +return strstr(typename, "pci") != NULL; +} + +bool virtio_device_page_per_vq_enabled(VirtIODevice *vdev) +{ +#ifdef CONFIG_VIRTIO_PCI +if (virtio_device_parent_is_pci_device(vdev)) { +return virtio_pci_page_per_vq_enabled(vdev); +} +#endif +return false; +} + +int virtio_device_notify_region_map(VirtIODevice *vdev, int queue_idx, +MemoryRegion *mr) +{ +#ifdef CONFIG_VIRTIO_PCI +if (virtio_device_parent_is_pci_device(vdev)) { +return virtio_pci_notify_region_map(vdev, queue_idx, mr); +} +#endif +return -1; +} + +void virtio_device_notify_region_unmap(VirtIODevice *vdev, MemoryRegion *mr) +{ +#ifdef CONFIG_VIRTIO_PCI +if (virtio_device_parent_is_pci_device(vdev)) { +virtio_pci_notify_region_unmap(vdev, mr); +} +#endif +} + static void virtio_device_class_init(ObjectClass *klass, void *data) { /* Set the default value here. */ diff --git a/incl
[Qemu-devel] [PATCH v2 2/6] vhost-user: introduce shared vhost-user state
When multi-queue is enabled for virtio-net, each virtio queue pair will have a vhost_dev, and the only thing they share currently is the chardev. This patch introduces a vhost-user state structure which will be shared by all virtio queue pairs of the same virtio device. Signed-off-by: Tiwei Bie --- hw/scsi/vhost-user-scsi.c | 6 +++--- hw/virtio/vhost-user.c | 9 + include/hw/virtio/vhost-user.h | 17 + include/hw/virtio/virtio-scsi.h | 6 +- net/vhost-user.c| 30 -- 5 files changed, 46 insertions(+), 22 deletions(-) create mode 100644 include/hw/virtio/vhost-user.h diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 9389ed48e0..64972bdd7d 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -72,7 +72,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) Error *err = NULL; int ret; -if (!vs->conf.chardev.chr) { +if (!vs->conf.vhost_user.chr.chr) { error_setg(errp, "vhost-user-scsi: missing chardev"); return; } @@ -90,7 +90,7 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp) vsc->dev.vq_index = 0; vsc->dev.backend_features = 0; -ret = vhost_dev_init(&vsc->dev, (void *)&vs->conf.chardev, +ret = vhost_dev_init(&vsc->dev, (void *)&vs->conf.vhost_user, VHOST_BACKEND_TYPE_USER, 0); if (ret < 0) { error_setg(errp, "vhost-user-scsi: vhost initialization failed: %s", @@ -131,7 +131,7 @@ static uint64_t vhost_user_scsi_get_features(VirtIODevice *vdev, } static Property vhost_user_scsi_properties[] = { -DEFINE_PROP_CHR("chardev", VirtIOSCSICommon, conf.chardev), +DEFINE_PROP_CHR("chardev", VirtIOSCSICommon, conf.vhost_user.chr), DEFINE_PROP_UINT32("boot_tpgt", VirtIOSCSICommon, conf.boot_tpgt, 0), DEFINE_PROP_UINT32("num_queues", VirtIOSCSICommon, conf.num_queues, 1), DEFINE_PROP_UINT32("virtqueue_size", VirtIOSCSICommon, conf.virtqueue_size, diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 1ad6caa6a3..b228994ffd 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -12,6 +12,7 @@ #include "qapi/error.h" #include "hw/virtio/vhost.h" #include "hw/virtio/vhost-backend.h" +#include "hw/virtio/vhost-user.h" #include "hw/virtio/virtio-net.h" #include "chardev/char-fe.h" #include "sysemu/kvm.h" @@ -164,7 +165,7 @@ static VhostUserMsg m __attribute__ ((unused)); #define VHOST_USER_VERSION(0x1) struct vhost_user { -CharBackend *chr; +VhostUser *shared; int slave_fd; }; @@ -176,7 +177,7 @@ static bool ioeventfd_enabled(void) static int vhost_user_read(struct vhost_dev *dev, VhostUserMsg *msg) { struct vhost_user *u = dev->opaque; -CharBackend *chr = u->chr; +CharBackend *chr = &u->shared->chr; uint8_t *p = (uint8_t *) msg; int r, size = VHOST_USER_HDR_SIZE; @@ -262,7 +263,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg, int *fds, int fd_num) { struct vhost_user *u = dev->opaque; -CharBackend *chr = u->chr; +CharBackend *chr = &u->shared->chr; int ret, size = VHOST_USER_HDR_SIZE + msg->hdr.size; /* @@ -839,7 +840,7 @@ static int vhost_user_init(struct vhost_dev *dev, void *opaque) assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER); u = g_new0(struct vhost_user, 1); -u->chr = opaque; +u->shared = opaque; u->slave_fd = -1; dev->opaque = u; diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h new file mode 100644 index 00..4f5a1477d1 --- /dev/null +++ b/include/hw/virtio/vhost-user.h @@ -0,0 +1,17 @@ +/* + * Copyright (c) 2017-2018 Intel Corporation + * + * This work is licensed under the terms of the GNU GPL, version 2. + * See the COPYING file in the top-level directory. + */ + +#ifndef HW_VIRTIO_VHOST_USER_H +#define HW_VIRTIO_VHOST_USER_H + +#include "chardev/char-fe.h" + +typedef struct VhostUser { +CharBackend chr; +} VhostUser; + +#endif diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 4c0bcdb788..885c3e84b5 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -19,6 +19,7 @@ #define VIRTIO_SCSI_SENSE_SIZE 0 #include "standard-headers/linux/virtio_scsi.h" #include "hw/virtio/virtio.h" +#include "hw/virtio/vhost-user.h" #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" #include "chardev/char-fe.h" @@ -54,7 +55,10 @@ struct VirtIOSCSIConf { char *vhostfd; char *wwpn; #endif -CharBackend chardev; +union { +VhostUser vhost_user; +CharBackend chardev; +}; uint32_t boot_tpgt; IOThread *iothread; }; diff --git a/net/vhost-user.c b/net/vhost-user.c index e0f16c895b..49ee72bd42 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -12,6 +12,7 @@ #include
[Qemu-devel] [PATCH v2 0/6] Extend vhost-user to support VFIO based accelerators
This patch set does some small extensions to vhost-user protocol to support VFIO based accelerators, and makes it possible to get the similar performance of VFIO based PCI passthru while keeping the virtio device emulation in QEMU. How does accelerator accelerate vhost (data path) = Any virtio ring compatible devices potentially can be used as the vhost data path accelerators. We can setup the accelerator based on the informations (e.g. memory table, features, ring info, etc) available on the vhost backend. And accelerator will be able to use the virtio ring provided by the virtio driver in the VM directly. So the virtio driver in the VM can exchange e.g. network packets with the accelerator directly via the virtio ring. That is to say, we will be able to use the accelerator to accelerate the vhost data path. We call it vDPA: vhost Data Path Acceleration. Notice: Although the accelerator can talk with the virtio driver in the VM via the virtio ring directly. The control path events (e.g. device start/stop) in the VM will still be trapped and handled by QEMU, and QEMU will deliver such events to the vhost backend via standard vhost protocol. Below link is an example showing how to setup a such environment via nested VM. In this case, the virtio device in the outer VM is the accelerator. It will be used to accelerate the virtio device in the inner VM. In reality, we could use virtio ring compatible hardware device as the accelerators. http://dpdk.org/ml/archives/dev/2017-December/085044.html In above example, it doesn't require any changes to QEMU, but it has lower performance compared with the traditional VFIO based PCI passthru. And that's the problem this patch set wants to solve. The performance issue of vDPA/vhost-user and solutions == For vhost-user backend, the critical issue in vDPA is that the data path performance is relatively low and some host threads are needed for the data path, because some necessary mechanisms are missing to support: 1) guest driver notifies the device directly; 2) device interrupts the guest directly; So this patch set does some small extensions to the vhost-user protocol to make both of them possible. It leverages the same mechanisms (e.g. EPT and Posted-Interrupt on Intel platform) as the PCI passthru. A new protocol feature bit is added to negotiate the accelerator feature support. Two new slave message types are added to control the notify region and queue interrupt passthru for each queue. >From the view of vhost-user protocol design, it's very flexible. The passthru can be enabled/disabled for each queue individually, and it's possible to accelerate each queue by different devices. More design and implementation details can be found from the last patch. Difference between vDPA and PCI passthru The key difference between PCI passthru and vDPA is that, in vDPA only the data path of the device (e.g. DMA ring, notify region and queue interrupt) is pass-throughed to the VM, the device control path (e.g. PCI configuration space and MMIO regions) is still defined and emulated by QEMU. The benefits of keeping virtio device emulation in QEMU compared with virtio device PCI passthru include (but not limit to): - consistent device interface for guest OS in the VM; - max flexibility on the hardware (i.e. the accelerators) design; - leveraging the existing virtio live-migration framework; Why extend vhost-user for vDPA == We have already implemented various virtual switches (e.g. OVS-DPDK) based on vhost-user for VMs in the Cloud. They are purely software running on CPU cores. When we have accelerators for such NFVi applications, it's ideal if the applications could keep using the original interface (i.e. vhost-user netdev) with QEMU, and infrastructure is able to decide when and how to switch between CPU and accelerators within the interface. And the switching (i.e. switch between CPU and accelerators) can be done flexibly and quickly inside the applications. More details about this can be found from the Cunming's discussions on the RFC patch set. Update notes IOMMU feature bit check is removed in this version, because: The IOMMU feature is negotiable, when an accelerator is used and it doesn't support virtual IOMMU, its driver just won't provide this feature bit when vhost library querying its features. And if it supports the virtual IOMMU, its driver can provide this feature bit. It's not reasonable to add this limitation in this patch set. The previous links: RFC: http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg04844.html v1: http://lists.nongnu.org/archive/html/qemu-devel/2018-01/msg06028.html v1 -> v2: - Add some explanations about why extend vhost-user in commit log (Paolo); - Bug fix in slave_read() according to Stefan's fix in DPDK; - Remove IOMMU feature chec
[Qemu-devel] [PATCH v2 1/6] vhost-user: support receiving file descriptors in slave_read
Signed-off-by: Tiwei Bie --- hw/virtio/vhost-user.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 41ff5cff41..1ad6caa6a3 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -674,14 +674,44 @@ static void slave_read(void *opaque) VhostUserHeader hdr = { 0, }; VhostUserPayload payload = { 0, }; int size, ret = 0; +struct iovec iov; +struct msghdr msgh; +int fd = -1; +size_t fdsize = sizeof(fd); +char control[CMSG_SPACE(fdsize)]; +struct cmsghdr *cmsg; + +memset(&msgh, 0, sizeof(msgh)); +msgh.msg_iov = &iov; +msgh.msg_iovlen = 1; +msgh.msg_control = control; +msgh.msg_controllen = sizeof(control); /* Read header */ -size = read(u->slave_fd, &hdr, VHOST_USER_HDR_SIZE); +iov.iov_base = &hdr; +iov.iov_len = VHOST_USER_HDR_SIZE; + +size = recvmsg(u->slave_fd, &msgh, 0); if (size != VHOST_USER_HDR_SIZE) { error_report("Failed to read from slave."); goto err; } +if (msgh.msg_flags & MSG_CTRUNC) { +error_report("Truncated message."); +goto err; +} + +for (cmsg = CMSG_FIRSTHDR(&msgh); cmsg != NULL; + cmsg = CMSG_NXTHDR(&msgh, cmsg)) { +if (cmsg->cmsg_level == SOL_SOCKET && +cmsg->cmsg_type == SCM_RIGHTS) { +fdsize = cmsg->cmsg_len - CMSG_LEN(0); +memcpy(&fd, CMSG_DATA(cmsg), fdsize); +break; +} +} + if (hdr.size > VHOST_USER_PAYLOAD_SIZE) { error_report("Failed to read msg header." " Size %d exceeds the maximum %zu.", hdr.size, @@ -705,9 +735,15 @@ static void slave_read(void *opaque) break; default: error_report("Received unexpected msg type."); +if (fd != -1) { +close(fd); +} ret = -EINVAL; } +/* Message handlers need to make sure that fd will be consumed. */ +fd = -1; + /* * REPLY_ACK feature handling. Other reply types has to be managed * directly in their request handlers. @@ -740,6 +776,9 @@ err: qemu_set_fd_handler(u->slave_fd, NULL, NULL, NULL); close(u->slave_fd); u->slave_fd = -1; +if (fd != -1) { +close(fd); +} return; } -- 2.11.0
[Qemu-devel] [PATCH v2 4/6] vfio: support getting VFIOGroup from groupfd
Add an API to support getting VFIOGroup from groupfd. When groupfd is shared by another process, the VFIOGroup may not have its container and address space in QEMU. Besides, add a reference counter to better support getting VFIOGroup multiple times. Signed-off-by: Tiwei Bie --- hw/vfio/common.c | 97 ++- include/hw/vfio/vfio-common.h | 2 + 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 5e84716218..24ec0f2c8d 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -1038,6 +1038,11 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as, int ret, fd; VFIOAddressSpace *space; +if (as == NULL) { +vfio_kvm_device_add_group(group); +return 0; +} + space = vfio_get_address_space(as); QLIST_FOREACH(container, &space->containers, next) { @@ -1237,6 +1242,10 @@ static void vfio_disconnect_container(VFIOGroup *group) { VFIOContainer *container = group->container; +if (container == NULL) { +return; +} + QLIST_REMOVE(group, container_next); group->container = NULL; @@ -1275,6 +1284,86 @@ static void vfio_disconnect_container(VFIOGroup *group) } } +static int vfio_groupfd_to_groupid(int groupfd) +{ +char linkname[PATH_MAX]; +char pathname[PATH_MAX]; +char *filename; +int groupid, len; + +snprintf(linkname, sizeof(linkname), "/proc/self/fd/%d", groupfd); + +len = readlink(linkname, pathname, sizeof(pathname)); +if (len <= 0 || len >= sizeof(pathname)) { +return -1; +} +pathname[len] = '\0'; + +filename = g_path_get_basename(pathname); +groupid = atoi(filename); +g_free(filename); + +return groupid; +} + +/* + * The @as param could be NULL. In this case, groupfd is shared by + * another process which will setup the DMA mapping for this group, + * and this group won't have container and address space in QEMU. + */ +VFIOGroup *vfio_get_group_from_fd(int groupfd, AddressSpace *as, Error **errp) +{ +VFIOGroup *group; +int groupid; + +groupid = vfio_groupfd_to_groupid(groupfd); +if (groupid < 0) { +return NULL; +} + +QLIST_FOREACH(group, &vfio_group_list, next) { +if (group->groupid == groupid) { +/* Found it. Now is it already in the right context? */ +if ((group->container == NULL && as == NULL) || +(group->container && group->container->space->as == as)) { +group->refcnt++; +return group; +} +error_setg(errp, "group %d used in multiple address spaces", + group->groupid); +return NULL; +} +} + +group = g_malloc0(sizeof(*group)); + +group->fd = groupfd; +group->groupid = groupid; +group->refcnt = 1; + +QLIST_INIT(&group->device_list); + +if (vfio_connect_container(group, as, errp)) { +error_prepend(errp, "failed to setup container for group %d: ", + groupid); +goto free_group_exit; +} + +if (QLIST_EMPTY(&vfio_group_list)) { +qemu_register_reset(vfio_reset_handler, NULL); +} + +QLIST_INSERT_HEAD(&vfio_group_list, group, next); + +return group; + +free_group_exit: +g_free(group); + +return NULL; +} + +/* The @as param cannot be NULL. */ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) { VFIOGroup *group; @@ -1284,7 +1373,8 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) QLIST_FOREACH(group, &vfio_group_list, next) { if (group->groupid == groupid) { /* Found it. Now is it already in the right context? */ -if (group->container->space->as == as) { +if (group->container && group->container->space->as == as) { +group->refcnt++; return group; } else { error_setg(errp, "group %d used in multiple address spaces", @@ -1317,6 +1407,7 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp) } group->groupid = groupid; +group->refcnt = 1; QLIST_INIT(&group->device_list); if (vfio_connect_container(group, as, errp)) { @@ -1348,6 +1439,10 @@ void vfio_put_group(VFIOGroup *group) return; } +if (--group->refcnt > 0) { +return; +} + vfio_kvm_device_del_group(group); vfio_disconnect_container(group); QLIST_REMOVE(group, next); diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index d9360148e6..b820f7984c 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -137,6 +137,7 @@ struct VFIODeviceOps { typedef struct VFIOGroup { int fd; int groupid; +int refcnt; VFIOContainer *container; QLIST_HEAD(, VFIODevice) device_list; QLIST_ENTRY(VFIOGroup)
[Qemu-devel] [PATCH v2 5/6] vfio: remove DPRINTF() definition from vfio-common.h
This macro isn't used by any VFIO code. And its name is too generic. The vfio-common.h (in include/hw/vfio) can be included by other modules in QEMU. It can introduce conflicts. Signed-off-by: Tiwei Bie --- include/hw/vfio/vfio-common.h | 9 - 1 file changed, 9 deletions(-) diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index b820f7984c..f6aa4ae959 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -34,15 +34,6 @@ #define ERR_PREFIX "vfio error: %s: " #define WARN_PREFIX "vfio warning: %s: " -/*#define DEBUG_VFIO*/ -#ifdef DEBUG_VFIO -#define DPRINTF(fmt, ...) \ -do { fprintf(stderr, "vfio: " fmt, ## __VA_ARGS__); } while (0) -#else -#define DPRINTF(fmt, ...) \ -do { } while (0) -#endif - enum { VFIO_DEVICE_TYPE_PCI = 0, VFIO_DEVICE_TYPE_PLATFORM = 1, -- 2.11.0
[Qemu-devel] [PATCH v2 6/6] vhost-user: add VFIO based accelerators support
This patch does some small extensions to vhost-user protocol to support VFIO based accelerators, and makes it possible to get the similar performance of VFIO based PCI passthru while keeping the virtio device emulation in QEMU. Any virtio ring compatible devices potentially can be used as the vhost data path accelerators. We can setup the accelerator based on the informations (e.g. memory table, features, ring info, etc) available on the vhost backend. And accelerator will be able to use the virtio ring provided by the virtio driver in the VM directly. So the virtio driver in the VM can exchange e.g. network packets with the accelerator directly via the virtio ring. But for vhost-user, the critical issue in this case is that the data path performance is relatively low and some host threads are needed for the data path, because some necessary mechanisms are missing to support: 1) guest driver notifies the device directly; 2) device interrupts the guest directly; So this patch does some small extensions to vhost-user protocol to make both of them possible. It leverages the same mechanisms as the VFIO based PCI passthru. A new protocol feature bit is added to negotiate the accelerator feature support. Two new slave message types are added to control the notify region and queue interrupt passthru for each queue. >From the view of vhost-user protocol design, it's very flexible. The passthru can be enabled/disabled for each queue individually, and it's possible to accelerate each queue by different devices. The key difference with PCI passthru is that, in this case only the data path of the device (e.g. DMA ring, notify region and queue interrupt) is pass-throughed to the VM, the device control path (e.g. PCI configuration space and MMIO regions) is still defined and emulated by QEMU. The benefits of keeping virtio device emulation in QEMU compared with virtio device PCI passthru include (but not limit to): - consistent device interface for guest OS in the VM; - max flexibility on the hardware (i.e. the accelerators) design; - leveraging the existing virtio live-migration framework; Normally, vhost-user is meant for connecting to e.g. user-space switch which is shared between multiple VMs. Typically, a vhost accelerator isn't a simple NIC which is just for packet I/O, but e.g. an switch accelerator which is also shared between multiple VMs. This commit extends vhost-user to better support connecting to e.g. a user-space switch that has an accelerator. Signed-off-by: Tiwei Bie --- docs/interop/vhost-user.txt| 57 hw/virtio/vhost-user.c | 198 + include/hw/virtio/vhost-user.h | 17 3 files changed, 272 insertions(+) diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index cb3a7595aa..264a58a800 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -132,6 +132,15 @@ Depending on the request type, payload can be: Payload: Size bytes array holding the contents of the virtio device's configuration space + * Vring area description + --- + | u64 | size | offset | + --- + + u64: a 64-bit unsigned integer + Size: a 64-bit size + Offset: a 64-bit offset + In QEMU the vhost-user message is implemented with the following struct: typedef struct VhostUserMsg { @@ -146,6 +155,7 @@ typedef struct VhostUserMsg { VhostUserLog log; struct vhost_iotlb_msg iotlb; VhostUserConfig config; +VhostUserVringArea area; }; } QEMU_PACKED VhostUserMsg; @@ -358,6 +368,17 @@ The fd is provided via VHOST_USER_SET_SLAVE_REQ_FD ancillary data. A slave may then send VHOST_USER_SLAVE_* messages to the master using this fd communication channel. +VFIO based accelerators +--- + +The VFIO based accelerators feature is a protocol extension. It is supported +when the protocol feature VHOST_USER_PROTOCOL_F_VFIO (bit 7) is set. + +The vhost-user backend will set the accelerator context via slave channel, +and QEMU just needs to handle those messages passively. The accelerator +context will be set for each queue independently. So the page-per-vq property +should also be enabled. + Protocol features - @@ -369,6 +390,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN 6 #define VHOST_USER_PROTOCOL_F_CRYPTO_SESSION 7 +#define VHOST_USER_PROTOCOL_F_VFIO 8 Master message types @@ -722,6 +744,41 @@ Slave message types respond with zero when operation is successfully completed, or non-zero otherwise. + * VHOST_USER_SLAVE_VRING_VFIO_GROUP_MSG + + Id: 3 + Equivalent ioctl: N/A + Slave payload: u64 + Master payload: N/A + + Sets the VFIO group file descriptor which is passed as ancillary data + for a specified queue (queue index is carried in t
Re: [Qemu-devel] [PATCH 3/8] migration: support to detect compressionand decompression errors
Hi, guangrong > @@ -1051,11 +1052,13 @@ static int do_compress_ram_page(QEMUFile *f, z_stream > *stream, RAMBlock *block, > { > RAMState *rs = ram_state; > int bytes_sent, blen; > -uint8_t *p = block->host + (offset & TARGET_PAGE_MASK); > +uint8_t buf[TARGET_PAGE_SIZE], *p; > +p = block->host + (offset & TARGET_PAGE_MASK); > bytes_sent = save_page_header(rs, f, block, offset | > RAM_SAVE_FLAG_COMPRESS_PAGE); > -blen = qemu_put_compression_data(f, stream, p, TARGET_PAGE_SIZE); > +memcpy(buf, p, TARGET_PAGE_SIZE); > +blen = qemu_put_compression_data(f, stream, buf, TARGET_PAGE_SIZE); Memory copy operation for every page to be compressed is not cheap, especially when the page number is huge, and it may be not necessary for pages never updated during migration. Is there any possibility that we can distinguish the real compress/decompress errors from those being caused by source VM updating? Such as the return value of qemu_uncompress(distinguish Z_DATA_ERROR and other error codes returned by inflate())? Jiang Regards,
Re: [Qemu-devel] [PATCH 3/8] migration: support to detect compressionand decompression errors
On 03/19/2018 03:56 PM, jiang.bi...@zte.com.cn wrote: Hi, guangrong @@ -1051,11 +1052,13 @@ static int do_compress_ram_page(QEMUFile *f, z_stream *stream, RAMBlock *block, { RAMState *rs = ram_state; int bytes_sent, blen; -uint8_t *p = block->host + (offset & TARGET_PAGE_MASK); +uint8_t buf[TARGET_PAGE_SIZE], *p; +p = block->host + (offset & TARGET_PAGE_MASK); bytes_sent = save_page_header(rs, f, block, offset | RAM_SAVE_FLAG_COMPRESS_PAGE); -blen = qemu_put_compression_data(f, stream, p, TARGET_PAGE_SIZE); +memcpy(buf, p, TARGET_PAGE_SIZE); +blen = qemu_put_compression_data(f, stream, buf, TARGET_PAGE_SIZE); Memory copy operation for every page to be compressed is not cheap, especially when the page number is huge, and it may be not necessary for pages never updated during migration. This is only for 4k page. Is there any possibility that we can distinguish the real compress/decompress errors from those being caused by source VM updating? Such as the return value of qemu_uncompress(distinguish Z_DATA_ERROR and other error codes returned by inflate())? Unfortunately, no. :(
[Qemu-devel] [PATCH v2] qcow2: add overlap check for bitmap directory
Signed-off-by: Vladimir Sementsov-Ogievskiy --- If it appropriate for 2.12, let's push it. If not - then for 2.13. v2: - squash 02 (indentation fix) to 01 - drop comment from qcow2_check_metadata_overlap() - set @ign to QCOW2_OL_BITMAP_DIRECTORY for in-place case in bitmap_list_store. I don't think non-inplace case should be changed, as it don't touch active bitmap directory. block/qcow2.h | 45 - block/qcow2-bitmap.c | 7 ++- block/qcow2-refcount.c | 10 ++ block/qcow2.c | 22 ++ 4 files changed, 54 insertions(+), 30 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 6f0ff15dd0..896ad08e5b 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -98,6 +98,7 @@ #define QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE "overlap-check.snapshot-table" #define QCOW2_OPT_OVERLAP_INACTIVE_L1 "overlap-check.inactive-l1" #define QCOW2_OPT_OVERLAP_INACTIVE_L2 "overlap-check.inactive-l2" +#define QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY "overlap-check.bitmap-directory" #define QCOW2_OPT_CACHE_SIZE "cache-size" #define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size" #define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size" @@ -398,34 +399,36 @@ typedef enum QCow2ClusterType { } QCow2ClusterType; typedef enum QCow2MetadataOverlap { -QCOW2_OL_MAIN_HEADER_BITNR= 0, -QCOW2_OL_ACTIVE_L1_BITNR = 1, -QCOW2_OL_ACTIVE_L2_BITNR = 2, -QCOW2_OL_REFCOUNT_TABLE_BITNR = 3, -QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4, -QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5, -QCOW2_OL_INACTIVE_L1_BITNR= 6, -QCOW2_OL_INACTIVE_L2_BITNR= 7, - -QCOW2_OL_MAX_BITNR= 8, - -QCOW2_OL_NONE = 0, -QCOW2_OL_MAIN_HEADER= (1 << QCOW2_OL_MAIN_HEADER_BITNR), -QCOW2_OL_ACTIVE_L1 = (1 << QCOW2_OL_ACTIVE_L1_BITNR), -QCOW2_OL_ACTIVE_L2 = (1 << QCOW2_OL_ACTIVE_L2_BITNR), -QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR), -QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR), -QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR), -QCOW2_OL_INACTIVE_L1= (1 << QCOW2_OL_INACTIVE_L1_BITNR), +QCOW2_OL_MAIN_HEADER_BITNR = 0, +QCOW2_OL_ACTIVE_L1_BITNR= 1, +QCOW2_OL_ACTIVE_L2_BITNR= 2, +QCOW2_OL_REFCOUNT_TABLE_BITNR = 3, +QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4, +QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5, +QCOW2_OL_INACTIVE_L1_BITNR = 6, +QCOW2_OL_INACTIVE_L2_BITNR = 7, +QCOW2_OL_BITMAP_DIRECTORY_BITNR = 8, + +QCOW2_OL_MAX_BITNR = 9, + +QCOW2_OL_NONE = 0, +QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR), +QCOW2_OL_ACTIVE_L1= (1 << QCOW2_OL_ACTIVE_L1_BITNR), +QCOW2_OL_ACTIVE_L2= (1 << QCOW2_OL_ACTIVE_L2_BITNR), +QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR), +QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR), +QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR), +QCOW2_OL_INACTIVE_L1 = (1 << QCOW2_OL_INACTIVE_L1_BITNR), /* NOTE: Checking overlaps with inactive L2 tables will result in bdrv * reads. */ -QCOW2_OL_INACTIVE_L2= (1 << QCOW2_OL_INACTIVE_L2_BITNR), +QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR), +QCOW2_OL_BITMAP_DIRECTORY = (1 << QCOW2_OL_BITMAP_DIRECTORY_BITNR), } QCow2MetadataOverlap; /* Perform all overlap checks which can be done in constant time */ #define QCOW2_OL_CONSTANT \ (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_REFCOUNT_TABLE | \ - QCOW2_OL_SNAPSHOT_TABLE) + QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_BITMAP_DIRECTORY) /* Perform all overlap checks which don't require disk access */ #define QCOW2_OL_CACHED \ diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index f45e46cfbd..fb750ba8d3 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -776,7 +776,12 @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list, } } -ret = qcow2_pre_write_overlap_check(bs, 0, dir_offset, dir_size); +/* Actually, even in in-place case ignoring QCOW2_OL_BITMAP_DIRECTORY is not + * necessary, because we drop QCOW2_AUTOCLEAR_BITMAPS when updating bitmap + * directory in-place (actually, turn-off the extension), which is checked + * in qcow2_check_metadata_overlap() */ +ret = qcow2_pre_write_overlap_check( +bs, in_place ? QCOW2_OL_BITMAP_DIRECTORY : 0, dir_offset, dir_size); if (ret < 0) { goto fail; } diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 3de1ab51ba..275a303cfa 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -2585,6 +2585,16 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, } } +if ((chk & QCOW2_OL_BITMAP_DIRECTORY) && +(s->autoclear_features &
Re: [Qemu-devel] [PATCH 3/3] nvram: at24c: use a sane default for "rom-size"
Hi Philippe, > > I don't mind much, but why? My reasoning was "let's first fix the cause > > and then the symptom"? > > The '0' case is worst than incorrect, it segfaults, so you are right :) Ok, thanks. > >> Can you add a #define for this value? Such AT24C_ROMSIZE_MIN. > > > > Can do, of course. But won't that give room for regressions because > > people are already using it with lower values? > > Your patch already introduce the regression :) Not really. The current default setting (0) segfaults. This is how I discovered it. So, there can't be any active user of that. > I prefer self-explanatory #defines than magic value, but I see your > point, so if we can not decide a value, can you add a comment to explain > the magic value? I think the clearer is to add a #define with a comment. I wouldn't mind a macro AT24C_ROMSIZE_DEFAULT and a comment explaining why we chose this value. This is totally fine with me. It was just the MIN value I saw potential usage regressions with. > IMO there are too many AT24C eeproms to model, so the "rom-size" > variable is the easiest way. Your patch #2 is simple enough to avoid the > #DIV/0! Fine with me, too. I will update my series accordingly. Thanks, Wolfram signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] target/m68k: add a mechanism to automatically free TCGv
On 03/19/2018 12:12 AM, Laurent Vivier wrote: > SRC_EA() and gen_extend() can return either a temporary > TCGv or a memory allocated one. Mark them when they are > allocated, and free them automatically at end of the > instruction translation. > > We want to free locally allocated TCGv to avoid > overflow in sequence like: > > 0xc00ae406: movel %fp@(-132),%fp@(-268) > 0xc00ae40c: movel %fp@(-128),%fp@(-264) > 0xc00ae412: movel %fp@(-20),%fp@(-212) > 0xc00ae418: movel %fp@(-16),%fp@(-208) > 0xc00ae41e: movel %fp@(-60),%fp@(-220) > 0xc00ae424: movel %fp@(-56),%fp@(-216) > 0xc00ae42a: movel %fp@(-124),%fp@(-252) > 0xc00ae430: movel %fp@(-120),%fp@(-248) > 0xc00ae436: movel %fp@(-12),%fp@(-260) > 0xc00ae43c: movel %fp@(-8),%fp@(-256) > 0xc00ae442: movel %fp@(-52),%fp@(-276) > 0xc00ae448: movel %fp@(-48),%fp@(-272) > ... > > That can fill a lot of TCGv entries in a sequence, > especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps") > we have no limit to fill the TCGOps cache and we can fill > the entire TCG variables array and overflow it. > > Suggested-by: Richard Henderson > Signed-off-by: Laurent Vivier > --- This is a good start. It's hard to see all of where else might have been missed; at least the call to gen_load in gen_lea_indexed. For next development cycle it would be good to convert the translator loop and enable TCGv leak detection. That said, this looks good so far. Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [virtio-dev] Re: [PATCH v5 4/5] virtio-balloon: VIRTIO_BALLOON_F_FREE_PAGE_HINT
On 03/19/2018 12:24 PM, Michael S. Tsirkin wrote: On Sun, Mar 18, 2018 at 06:36:20PM +0800, Wei Wang wrote: On 03/16/2018 11:16 PM, Michael S. Tsirkin wrote: On Fri, Mar 16, 2018 at 06:48:28PM +0800, Wei Wang wrote: OTOH it seems that if thread stops nothing will wake it up whem vm is restarted. Such bahaviour change across vmstop/vmstart is unexpected. I do not understand why we want to increment the counter on vm stop though. It does make sense to stop the thread but why not resume where we left off when vm is resumed? I'm not sure which counter we incremented. But it would be clear if we have a high level view of how it works (it is symmetric actually). Basically, we start the optimization when each round starts and stop it at the end of each round (i.e. before we do the bitmap sync), as shown below: 1) 1st Round starts --> free_page_start 2) 1st Round in progress.. 3) 1st Round ends --> free_page_stop 4) 2nd Round starts --> free_page_start 5) 2nd Round in progress.. 6) 2nd Round ends --> free_page_stop .. For example, in 2), the VM is stopped. virtio_balloon_poll_free_page_hints finds the vq is empty (i.e. elem == NULL) and the runstate is stopped, the optimization thread exits immediately. That is, this optimization thread is gone forever (the optimization we can do for this round is done). We won't know when would the VM be woken up: A) If the VM is woken up very soon when the migration thread is still in progress of 2), then in 4) a new optimization thread (not the same one for the first round) will be created and start the optimization for the 2nd round as usual (If you have questions about 3) in this case, that free_page_stop will do nothing than just return, since the optimization thread has exited) ; B) If the VM is woken up after the whole migration has ended, there is still no point in resuming the optimization. I think this would be the simple design for the first release of this optimization. There are possibilities to improve case A) above by continuing optimization for the 1st Round as it is still in progress, but I think adding that complexity for this rare case wouldn't be worthwhile (at least for now). What would you think? Best, Wei
[Qemu-devel] [PATCH v3 3/3] iotests: enable shared migration cases in 169
Shared migration for dirty bitmaps is fixed by previous patches, so we can enable the test. Signed-off-by: Vladimir Sementsov-Ogievskiy --- tests/qemu-iotests/169 | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/169 b/tests/qemu-iotests/169 index 3a8db91f6f..153b10b6e7 100755 --- a/tests/qemu-iotests/169 +++ b/tests/qemu-iotests/169 @@ -140,16 +140,14 @@ def inject_test_case(klass, name, method, *args, **kwargs): mc = operator.methodcaller(method, *args, **kwargs) setattr(klass, 'test_' + name, new.instancemethod(mc, None, klass)) -for cmb in list(itertools.product((True, False), repeat=3)): +for cmb in list(itertools.product((True, False), repeat=4)): name = ('_' if cmb[0] else '_not_') + 'persistent_' name += ('_' if cmb[1] else '_not_') + 'migbitmap_' name += '_online' if cmb[2] else '_offline' - -# TODO fix shared-storage bitmap migration and enable cases for it -args = list(cmb) + [False] +name += '_shared' if cmb[3] else '_nonshared' inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration', - *args) + *list(cmb)) if __name__ == '__main__': -- 2.11.1
[Qemu-devel] [PATCH v3 2/3] qcow2: handle reopening bitmaps on bdrv_invalidate_cache
Consider migration with shared storage. Persistent bitmaps are stored on bdrv_inactivate. Then, on destination process_incoming_migration_bh() calls bdrv_invalidate_cache_all() which leads to qcow2_load_autoloading_dirty_bitmaps() which fails if bitmaps are already loaded on destination start. In this case we should call qcow2_reopen_bitmaps_rw instead. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow --- block/qcow2.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 7472af6931..6219666d4a 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1480,7 +1480,13 @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, s->autoclear_features &= QCOW2_AUTOCLEAR_MASK; } -if (qcow2_load_dirty_bitmaps(bs, &local_err)) { +if (bdrv_has_readonly_bitmaps(bs)) { +if (!bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE)) { +bool header_updated = false; +qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err); +update_header = update_header && !header_updated; +} +} else if (qcow2_load_dirty_bitmaps(bs, &local_err)) { update_header = false; } if (local_err != NULL) { -- 2.11.1
[Qemu-devel] [PATCH v3 1/3] qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint()
Add version of qcow2_reopen_bitmaps_rw, which do the same work but also return a hint about was header updated or not. This will be used in the following fix for bitmaps reloading after migration. Signed-off-by: Vladimir Sementsov-Ogievskiy Reviewed-by: John Snow --- block/qcow2.h| 2 ++ block/qcow2-bitmap.c | 15 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/block/qcow2.h b/block/qcow2.h index ccb92a9696..d301f77cea 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -671,6 +671,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void **refcount_table, int64_t *refcount_table_size); bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp); +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, + Error **errp); int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp); void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp); int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 3010adb909..6e93ec43e1 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1004,7 +1004,8 @@ fail: return false; } -int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) +int qcow2_reopen_bitmaps_rw_hint(BlockDriverState *bs, bool *header_updated, + Error **errp) { BDRVQcow2State *s = bs->opaque; Qcow2BitmapList *bm_list; @@ -1012,6 +1013,10 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) GSList *ro_dirty_bitmaps = NULL; int ret = 0; +if (header_updated != NULL) { +*header_updated = false; +} + if (s->nb_bitmaps == 0) { /* No bitmaps - nothing to do */ return 0; @@ -1055,6 +1060,9 @@ int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) error_setg_errno(errp, -ret, "Can't update bitmap directory"); goto out; } +if (header_updated != NULL) { +*header_updated = true; +} g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false); } @@ -1065,6 +1073,11 @@ out: return ret; } +int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp) +{ +return qcow2_reopen_bitmaps_rw_hint(bs, NULL, errp); +} + /* store_bitmap_data() * Store bitmap to image, filling bitmap table accordingly. */ -- 2.11.1
[Qemu-devel] [PATCH v3 for 2.12 0/3] fix bitmaps migration through shared storage
Hi all. This fixes bitmaps migration through shared storage. Look at 02 for details. The bug introduced in 2.10 with the whole qcow2 bitmaps feature, so qemu-stable in CC. However I doubt that someone really suffered from this. Do we need dirty bitmaps at all in inactive case? - that was a question in v2. And, keeping in mind that we are going to use inactive mode not only for incoming migration, I'm not sure that answer is NO (but, it may be "NO" for 2.10, 2.11), so let's fix it in proposed here manner at least for 2.12. v3: tiny context changes in 01,02 03: instead of a separate test, enable corresponding case in existent one v2: John, thank you for reviewing v1. changes: add John's r-bs, change s/timeout=10/timeout=10.0/ in last patch and drop old 03 patch, related to this timeout fix. Vladimir Sementsov-Ogievskiy (3): qcow2-bitmap: add qcow2_reopen_bitmaps_rw_hint() qcow2: handle reopening bitmaps on bdrv_invalidate_cache iotests: enable shared migration cases in 169 block/qcow2.h | 2 ++ block/qcow2-bitmap.c | 15 ++- block/qcow2.c | 8 +++- tests/qemu-iotests/169 | 8 +++- 4 files changed, 26 insertions(+), 7 deletions(-) -- 2.11.1
Re: [Qemu-devel] [PATCH] target/m68k: add a mechanism to automatically free TCGv
Le 19/03/2018 à 09:39, Richard Henderson a écrit : > On 03/19/2018 12:12 AM, Laurent Vivier wrote: >> SRC_EA() and gen_extend() can return either a temporary >> TCGv or a memory allocated one. Mark them when they are >> allocated, and free them automatically at end of the >> instruction translation. >> >> We want to free locally allocated TCGv to avoid >> overflow in sequence like: >> >> 0xc00ae406: movel %fp@(-132),%fp@(-268) >> 0xc00ae40c: movel %fp@(-128),%fp@(-264) >> 0xc00ae412: movel %fp@(-20),%fp@(-212) >> 0xc00ae418: movel %fp@(-16),%fp@(-208) >> 0xc00ae41e: movel %fp@(-60),%fp@(-220) >> 0xc00ae424: movel %fp@(-56),%fp@(-216) >> 0xc00ae42a: movel %fp@(-124),%fp@(-252) >> 0xc00ae430: movel %fp@(-120),%fp@(-248) >> 0xc00ae436: movel %fp@(-12),%fp@(-260) >> 0xc00ae43c: movel %fp@(-8),%fp@(-256) >> 0xc00ae442: movel %fp@(-52),%fp@(-276) >> 0xc00ae448: movel %fp@(-48),%fp@(-272) >> ... >> >> That can fill a lot of TCGv entries in a sequence, >> especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps") >> we have no limit to fill the TCGOps cache and we can fill >> the entire TCG variables array and overflow it. >> >> Suggested-by: Richard Henderson >> Signed-off-by: Laurent Vivier >> --- > > This is a good start. It's hard to see all of where else might have been > missed; at least the call to gen_load in gen_lea_indexed. > > For next development cycle it would be good to convert the translator loop and > enable TCGv leak detection. I agree. > That said, this looks good so far. > Reviewed-by: Richard Henderson I'm going to update the patch by splitting it in two as Philippe asked and mark the missing gen_load() in gen_lea_indexed(). Thanks, Laurent
Re: [Qemu-devel] [PATCH v2 0/3] vfio/pci: ioeventfd support
On 16/3/18 8:31 am, Alex Williamson wrote: > A vfio ioeventfd will perform the pre-specified device write on > triggering of an eventfd. When coupled with KVM ioeventfds, this > feature allows a VM to trap a device page for virtualization, while > also registering targeted ioeventfds to maintain performance of high > frequency register writes within the trapped range. Much like the > existing interrupt eventfd/irqfd coupling, such writes can be handled > entirely in the host kernel. > > The new VFIO device ioctl may be supported by any vfio bus driver, > including mdev drivers, but the implementation here only enables > vfio-pci. This is intended as an acceleration path, bus drivers > may choose which regions to support and userspace should always > intend to fall back to non-accelerated handling when unavailable. > > v1->v2: > * Peter & Eric Sign-offs on 1/3 > * mutex_destroy() in 3/3 > * Slight enhancement to uapi description > * sparse clean - Sparse didn't like that we dropped the __iomem >address space when calling iowriteXX, re-adding it via the opaque >and data pointers of the virq was crude, and that was not a 32-bit >friendly soluion anyway, so add the iomem address to our ioeventfd >struct, pass that, and use a more simple, common handler. > > RFC->v1: > * An arbitrary limit is added for the number of ioeventfds supported >per device. The intention is to set this high enough to allow any >reasonable use case, but limit malicious user behavior. > * Split patches, including adding a patch for endian neutral io reads >and writes. This should be a nop for little-endian and avoid >redundant swap on big-endian, and hopefully resolves Alexey's >comments regarding the endian nature of this interface. > * Rebase to v4.16-rc3 > > Thanks, > Alex > > --- > > Alex Williamson (3): > vfio/pci: Pull BAR mapping setup from read-write path > vfio/pci: Use endian neutral helpers > vfio/pci: Add ioeventfd support > > > drivers/vfio/pci/vfio_pci.c | 35 +++ > drivers/vfio/pci/vfio_pci_private.h | 19 > drivers/vfio/pci/vfio_pci_rdwr.c| 184 > +++ > include/uapi/linux/vfio.h | 27 + > 4 files changed, 245 insertions(+), 20 deletions(-) > LGTM, I gave it a try to make sure 2/3 does not break and it does not in all combinations of le/be host <=> le/be guest but I did not try the actual feature - cannot make up a test quickly... Reviewed-by: Alexey Kardashevskiy -- Alexey
[Qemu-devel] [PATCH] replay: finish record/replay before closing the disks
After recent updates block devices cannot be closed on qemu exit. This happens due to the block request polling when replay is not finished. Therefore now we stop execution recording before closing the block devices. Signed-off-by: Pavel Dovgalyuk --- replay/replay.c |2 ++ vl.c|1 + 2 files changed, 3 insertions(+) diff --git a/replay/replay.c b/replay/replay.c index 8228261..58a986f 100644 --- a/replay/replay.c +++ b/replay/replay.c @@ -366,6 +366,8 @@ void replay_finish(void) g_free(replay_snapshot); replay_snapshot = NULL; +replay_mode = REPLAY_MODE_NONE; + replay_finish_events(); } diff --git a/vl.c b/vl.c index e8bebda..f4d9153 100644 --- a/vl.c +++ b/vl.c @@ -4733,6 +4733,7 @@ int main(int argc, char **argv, char **envp) /* No more vcpu or device emulation activity beyond this point */ vm_shutdown(); +replay_finish(); bdrv_close_all();
Re: [Qemu-devel] [PULL 00/38] QAPI patches for 2018-03-12, 2.12 softfreeze
On Sat, Mar 17, 2018 at 12:10:35PM +, Peter Maydell wrote: > On 16 March 2018 at 14:04, Eric Blake wrote: > > The following changes since commit 3788c7b6e56fa34ee2a73e41706eb2a2447ba75a: > > > > Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into > > staging (2018-03-16 11:05:03 +) > > > > are available in the Git repository at: > > > > git://repo.or.cz/qemu/ericb.git tags/pull-qapi-2018-03-12-v2 > > > > for you to fetch changes up to 75eb57e3ed3682f011a6694863044e8b143a9821: > > > > qapi: Pass '-u' when doing non-silent diff (2018-03-16 09:00:07 -0500) > > > > v2: rebase on Paolo's queue (should fix tests that failed on v1), > > fix rebase conflicts, add two more related patches > > Sending only the changed patches from v1 > > > > > > qapi patches for 2018-03-12, 2.12 softfreeze > > > > - Marc-André Lureau: 0/4 qapi: generate a literal qobject for introspection > > - Max Reitz: 0/7 block: Handle null backing link > > - Daniel P. Berrange: chardev: tcp: postpone TLS work until machine done > > - Peter Xu: 00/23 QMP: out-of-band (OOB) execution support > > - Vladimir Sementsov-Ogievskiy: 0/2 block latency histogram > > - Eric Blake: qapi: Pass '-u' when doing non-silent diff > > > > > > Hi. I get a bunch of test assertion failures with this: > > ppc64 host: > > QTEST_QEMU_BINARY=nios2-softmmu/qemu-system-nios2 > QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( > ${RANDOM:-0} % 255 + 1) > )} gtester -k --verbose -m=quick tests/qmp-test > tests/device-introspect-test tests/qom-test tests/test-hmp > TEST: tests/qmp-test... (pid=49431) > /nios2/qmp/protocol: OK > /nios2/qmp/oob: OK > /nios2/qmp/query-status: OK > /nios2/qmp/query-block: OK > /nios2/qmp/query-blockstats: OK > /nios2/qmp/query-block-jobs: OK > /nios2/qmp/query-named-block-nodes: > qemu-system-nios2: /home/pm215/qemu/chardev/char-io.c:91: io_watc > h_poll_finalize: Assertion `iwp->src == ((void *)0)' failed. > Broken pipe > FAIL > > FreeBSD host: > > TEST: tests/qmp-test... (pid=68428) > /aarch64/qmp/protocol: OK > /aarch64/qmp/oob:OK > [...] > /aarch64/qmp/query-iothreads: > Assertion failed: (iwp->src == NULL), function io_watch_poll_finalize, > file /root/qemu/chardev/char-io.c, line 91. > Broken pipe > FAIL > GTester: last random seed: R02S60296bacb6aea7a3d748811fc486c71e > (pid=68462) > > OpenBSD host: > /cris/qmp/qom-list-types: > assertion "iwp->src == NULL" failed: file "/home/qemu/chardev/cha > r-io.c", line 91, function "io_watch_poll_finalize" > Broken pipe > FAIL > > > NetBSD host: > > TEST: tests/tpm-crb-test... (pid=21337) > /i386/tpm-crb/test: OK > Unexpected error in qio_channel_socket_readv() at > /root/qemu/io/channel-socket.c:494: > FAIL: tests/tpm-crb-test > TEST: tests/tpm-tis-test... (pid=14763) > /i386/tpm-tis/test_check_localities: OK > /i386/tpm-tis/test_check_access_reg: OK > /i386/tpm-tis/test_check_access_reg_seize: OK > /i386/tpm-tis/test_check_access_reg_release: OK > /i386/tpm-tis/test_check_transmit: OK > Unexpected error in qio_channel_socket_readv() at > /root/qemu/io/channel-socket.c:494: > FAIL: tests/tpm-tis-test > > SPARC host: > > /cris/qmp/query-memory-size-summary: > qemu-system-cris: /srv/pm215/qemu/chardev/char-io.c:91: io_watch_ > poll_finalize: Assertion `iwp->src == NULL' failed. > Broken pipe > FAIL > > > x86/Linux host: > /hppa/qmp/query-memdev: > qemu-system-hppa: /home/petmay01/linaro/qemu-for-merges/chardev/c > har-io.c:91: io_watch_poll_finalize: Assertion `iwp->src == NULL' failed. > Broken pipe > FAIL > > aarch64 host: > qemu-system-alpha: /home/pm215/qemu/chardev/char-io.c:91: > io_watch_poll_finalize: Assertion `iwp->src == ((void *)0)' failed. > > One or two of the build hosts did pass, so that plus the varying > tests which failed suggests that the iwp->src assert is an > intermittent or timing based one. The tpm error on NetBSD > is probably a separate issue. I think I still need this to be squashed into "monitor: allow using IO thread for parsing", which I dropped during respin from v7 to v8: diff --git a/monitor.c b/monitor.c index f9ef3e5266..121194111f 100644 --- a/monitor.c +++ b/monitor.c @@ -4556,6 +4556,11 @@ void monitor_init(Chardev *chr, int flags) qemu_chr_fe_set_echo(&mon->chr, true);
[Qemu-devel] [Bug 1756519] Re: qemu linux-user crash in QOM path canonicalization during do_fork() call to cpu_create
** Summary changed: - qemu linux-user glib hash table crash in qom/object.c + qemu linux-user crash in QOM path canonicalization during do_fork() call to cpu_create -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1756519 Title: qemu linux-user crash in QOM path canonicalization during do_fork() call to cpu_create Status in QEMU: New Bug description: qemu-riscv64 version 2.11.50 (v2.11.0-2491-g2bb39a657a) crashes running gcc libgomp.c/sort-1.c testsuite test case with the following message: (process:11683): GLib-CRITICAL **: g_hash_table_iter_next: assertion 'ri->version == ri->hash_table->version' failed ** ERROR:qom/object.c:1665:object_get_canonical_path_component: code should not be reached qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x60139c16 Backtrace obtained via gdb: #0 raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x60139b21 in abort () at abort.c:79 #2 0x60100505 in g_assertion_message (domain=domain@entry=0x0, file=file@entry=0x60213ca1 "qom/object.c", line=line@entry=1665, func=func@entry=0x60214420 <__func__.18106> "object_get_canonical_path_component", message=message@entry=0x7fffe8000cd0 "code should not be reached") at gtestutils.c:2430 #3 0x60100586 in g_assertion_message_expr (domain=0x0, file=0x60213ca1 "qom/object.c", line=1665, func=0x60214420 <__func__.18106> "object_get_canonical_path_component", expr=) at gtestutils.c:2453 #4 0x60098334 in object_get_canonical_path_component (obj=0x7fffe81340b0) at qom/object.c:1665 #5 0x60098366 in object_get_canonical_path (obj=0x7fffe81340b0) at qom/object.c:1675 #6 0x6008e152 in device_set_realized (obj=0x7fffe81340b0, value=true, errp=0x7762fe68) at hw/core/qdev.c:874 #7 0x60098bf4 in property_set_bool (obj=0x7fffe81340b0, v=0x7fffe80fd3c0, name=0x60213694 "realized", opaque=0x7fffe80fd140, errp=0x7762fe68) at qom/object.c:1926 #8 0x60096fee in object_property_set (obj=0x7fffe81340b0, v=0x7fffe80fd3c0, name=0x60213694 "realized", errp=0x7762fe68) at qom/object.c:1122 #9 0x60099ebd in object_property_set_qobject (obj=0x7fffe81340b0, value=0x7fffe80fd310, name=0x60213694 "realized", errp=0x7762fe68) at qom/qom-qobject.c:27 #10 0x60097274 in object_property_set_bool (obj=0x7fffe81340b0, value=true, name=0x60213694 "realized", errp=0x7762fe68) at qom/object.c:1191 #11 0x60092ec5 in cpu_create (typename=0x6250e1a0 "any-riscv-cpu") at qom/cpu.c:61 #12 0x6009301a in cpu_generic_init (typename=0x601dd58f "riscv-cpu", cpu_model=0x601dd527 "any") at qom/cpu.c:98 #13 0x6004cb61 in cpu_copy (env=0x7008cd60) at /opt/qemu/linux-user/main.c:3881 #14 0x6005b79a in do_fork (env=0x7008cd60, flags=4001536, newsp=275531880704, parent_tidptr=275531882704, newtls=275531884288, child_tidptr=275531882704) at /opt/qemu/linux-user/syscall.c:6348 #15 0x60063e56 in do_syscall (cpu_env=0x7008cd60, num=220, arg1=4001536, arg2=275531880704, arg3=275531882704, arg4=275531884288, arg5=275531882704, arg6=275531884288, arg7=0, arg8=0) at /opt/qemu/linux-user/syscall.c:10001 #16 0x6004c89f in cpu_loop (env=0x7008cd60) at /opt/qemu/linux-user/main.c:3600 #17 0x6005b68f in clone_func (arg=0x77775050) at /opt/qemu/linux-user/syscall.c:6311 #18 0x60121797 in start_thread (arg=0x77632700) at pthread_create.c:463 #19 0x6019b4fb in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Attached is a test case source code extracted from libgomp test suite. Note that it is a multi-threaded and requires 5 or more threads to fail. Number of launched threads is controlled by OMP_NUM_THREADS evironment variable, defaulting to number of hardware threads. Changing constants in the test case makes it fail with different numbers of threads. I will attach statically linked riscv64 binary executable if size limits permit. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1756519/+subscriptions
[Qemu-devel] [PATCH v1 0/1] iotests: fix test case 185
Hi, This patch is to remove the redundant call to bdrv_drain_all in vm_shutdown. Thanks! Test case 185 failed as below: 185 2s ... - output mismatch (see 185.out.bad) --- /home/mc/gitcheck/work/qemu-master/tree/qemu/tests/qemu-iotests/185.out 2018-03-09 01:00:40.451603189 +0100 +++ /home/mc/gitcheck/work/qemu-master/tree/qemu/tests/qemu-iotests/185.out.bad 2018-03-19 09:40:22.781603189 +0100 @@ -20,7 +20,7 @@ {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 524288, "speed": 65536, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 1048576, "speed": 65536, "type": "commit"}} === Start active commit job and exit qemu === @@ -28,7 +28,8 @@ {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "commit"}} === Start mirror job and exit qemu === @@ -37,7 +38,8 @@ {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "disk", "len": 4194304, "offset": 4194304, "speed": 65536, "type": "mirror"}} === Start backup job and exit qemu === @@ -46,7 +48,7 @@ {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 65536, "speed": 65536, "type": "backup"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 131072, "speed": 65536, "type": "backup"}} === Start streaming job and exit qemu === @@ -54,6 +56,6 @@ {"return": {}} {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 524288, "speed": 65536, "type": "stream"}} +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "disk", "len": 67108864, "offset": 1048576, "speed": 65536, "type": "stream"}} No errors were found on the image. *** done Failures: 185 Failed 1 of 1 tests QingFeng Hao (1): iotests: fix test case 185 cpus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) -- 2.13.5
[Qemu-devel] [PATCH v1 1/1] iotests: fix test case 185
Test case 185 failed since commit 4486e89c219 --- "vl: introduce vm_shutdown()". It's because of the newly introduced function vm_shutdown calls bdrv_drain_all, which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs that doubles the speed and offset is doubled. Some jobs' status are changed as well. Thus, let's not call bdrv_drain_all in vm_shutdown. Signed-off-by: QingFeng Hao --- cpus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cpus.c b/cpus.c index 2e6701795b..ae2962508c 100644 --- a/cpus.c +++ b/cpus.c @@ -1006,8 +1006,9 @@ static int do_vm_stop(RunState state, bool send_stop) qapi_event_send_stop(&error_abort); } } - -bdrv_drain_all(); +if (send_stop) { +bdrv_drain_all(); +} replay_disable_events(); ret = bdrv_flush_all(); -- 2.13.5
[Qemu-devel] [RFC PATCH] tests/device-introspect: Test devices with all machines, not only with "none"
Many device introspection crashes only happen if you are using a certain machine, e.g.: $ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped) To be able to catch these problems, let's extend the device-introspect test to check the devices on all machine types. Since this is a rather slow operation, the test is only run in "SPEED=slow" mode. Signed-off-by: Thomas Huth --- In case someone wants to help with creating some bug fix patches during the QEMU hard freeze phase: This test can now be used to trigger lots of introspection bugs that we were not aware of yet. I think most of the bugs are due to wrong handling of instance_init vs. realize functions. For Example: $ make check-qtest SPEED=slow GTESTER check-qtest-aarch64 RAMBlock "integrator.flash" already registered, abort! Broken pipe GTester: last random seed: R02S8e52709605790d290d2c8261cefb8b0e Unsupported NIC model: lan9118 Broken pipe GTester: last random seed: R02S326d4ea43bfce860ebe2d554192540f7 qemu-system-aarch64: warning: nic lan9118.0 has no peer Unsupported NIC model: smc91c111 Broken pipe GTester: last random seed: R02Se9783b450806f350a14e757b175e3dc4 qemu-system-aarch64: missing SecureDigital device Broken pipe GTester: last random seed: R02S5c718b8f4c4fd48a358de8daafcf1b6f qemu-system-aarch64: warning: nic lan9118.0 has no peer Unexpected error in error_set_from_qdev_prop_error() at hw/core/qdev-properties.c:1095: Property 'allwinner-emac.netdev' can't take value 'hub0port0', it's in use Broken pipe GTester: last random seed: R02S597848ddcfdc76a695a946a9d4e50146 qemu-system-aarch64: warning: nic ftgmac100.0 has no peer GTester: last random seed: R02Seea0f0b769a2161fa53a50479fd68d84 qemu-system-aarch64: warning: nic imx.fec.0 has no peer qemu-system-aarch64: missing SecureDigital device Broken pipe GTester: last random seed: R02S9c2d3e34427162e7a56aa4ac859f1a6b Unsupported NIC model: virtio-net-pci Broken pipe GTester: last random seed: R02Sd61c0e9ed52d50a17c784213e5c6590c Unsupported NIC model: mv88w8618 Broken pipe GTester: last random seed: R02Sbfaecfe58dd643f2faca218e3051d464 qemu-system-aarch64: warning: nic mv88w8618_eth.0 has no peer qemu-system-aarch64: missing SecureDigital device Broken pipe Unsupported NIC model: xgmac Broken pipe GTester: last random seed: R02Sc61e65e884e364652c3a0c4190023565 fsl,imx7: Only 2 CPUs are supported (4 requested) Broken pipe GTester: last random seed: R02S0cfda43bc17e3e052d5a994b2c96457b etc. tests/device-introspect-test.c | 33 ++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c index b80058f..a9b9cf7 100644 --- a/tests/device-introspect-test.c +++ b/tests/device-introspect-test.c @@ -105,6 +105,8 @@ static void test_one_device(const char *type) QDict *resp; char *help, *qom_tree; +g_debug("Testing device '%s'", type); + resp = qmp("{'execute': 'device-list-properties'," " 'arguments': {'typename': %s}}", type); @@ -206,13 +208,13 @@ static void test_device_intro_abstract(void) qtest_end(); } -static void test_device_intro_concrete(void) +static void test_device_intro_concrete(gconstpointer args) { QList *types; QListEntry *entry; const char *type; -qtest_start(common_args); +qtest_start((const char *)args); types = device_type_list(false); QLIST_FOREACH_ENTRY(types, entry) { @@ -224,6 +226,7 @@ static void test_device_intro_concrete(void) QDECREF(types); qtest_end(); +g_free((void *)args); } static void test_abstract_interfaces(void) @@ -260,6 +263,26 @@ static void test_abstract_interfaces(void) qtest_end(); } +static void add_machine_test_case(const char *mname) +{ +char *path, *args; + +/* Ignore blacklisted machines */ +if (g_str_equal("xenfv", mname) || g_str_equal("xenpv", mname)) { +return; +} + +path = g_strdup_printf("device/introspect/concrete-defaults-%s", mname); +args = g_strdup_printf("-machine %s", mname); +qtest_add_data_func(path, args, test_device_intro_concrete); +g_free(path); + +path = g_strdup_printf("device/introspect/concrete-nodefaults-%s", mname); +args = g_strdup_printf("-nodefaults -machine %s", mname); +qtest_add_data_func(path, args, test_device_intro_concrete); +g_free(path); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -268,8 +291,12 @@ int main(int argc, char **argv) qtest_add_func("device/introspect/list-fields
Re: [Qemu-devel] [PATCH v3 10/24] RISC-V: Hold rcu_read_lock when accessing memory
On 16/03/2018 20:41, Michael Clark wrote: > From reading other code that accesses memory regions directly, > it appears that the rcu_read_lock needs to be held. Note: the > original code for accessing RAM directly was added because > there is no other way to use atomic_cmpxchg on guest physical > address space. > > Cc: Sagar Karandikar > Cc: Bastian Koppelmann > Signed-off-by: Michael Clark > Signed-off-by: Palmer Dabbelt > --- > target/riscv/helper.c | 14 -- > 1 file changed, 12 insertions(+), 2 deletions(-) I think the bug here is that rcu_read_lock/unlock is missing in cpu_memory_rw_debug. Are there any other callers you had in mind? Paolo > diff --git a/target/riscv/helper.c b/target/riscv/helper.c > index 02cbcea..e71633a 100644 > --- a/target/riscv/helper.c > +++ b/target/riscv/helper.c > @@ -209,6 +209,9 @@ restart: > as the PTE is no longer valid */ > MemoryRegion *mr; > hwaddr l = sizeof(target_ulong), addr1; > +enum { success, translate_fail, restart_walk} action = > success; > + > +rcu_read_lock(); > mr = address_space_translate(cs->as, pte_addr, > &addr1, &l, false); > if (memory_access_is_direct(mr, true)) { > @@ -222,7 +225,7 @@ restart: > target_ulong old_pte = > atomic_cmpxchg(pte_pa, pte, updated_pte); > if (old_pte != pte) { > -goto restart; > +action = restart_walk; > } else { > pte = updated_pte; > } > @@ -230,7 +233,14 @@ restart: > } else { > /* misconfigured PTE in ROM (AD bits are not preset) or > * PTE is in IO space and can't be updated atomically */ > -return TRANSLATE_FAIL; > +action = translate_fail; > +} > +rcu_read_unlock(); > + > +switch (action) { > +case success: break; > +case translate_fail: return TRANSLATE_FAIL; > +case restart_walk: goto restart; > } > } > >
Re: [Qemu-devel] [PATCH] gdbstub: send a terminaison packet instead of crashing gdb
Hi Philippe, Thanks for the review! BTW I forgot the for 2.12 tag can this be included in 2.12 or is it too late? Thanks, Fred On 03/19/2018 12:30 AM, Philippe Mathieu-Daudé wrote: On 03/16/2018 05:23 PM, KONRAD Frederic wrote: Since the commit: commit 4486e89c219c0d1b9bd8dfa0b1dd5b0d51ff2268 Author: Stefan Hajnoczi Date: Wed Mar 7 14:42:05 2018 + vl: introduce vm_shutdown() GDB crash when qemu exits (at least on sparc-softmmu): Remote communication error. Target disconnected.: Connection reset by peer. Quitting: putpkt: write failed: Broken pipe. So send a packet to kill GDB before we exit QEMU: [Inferior 1 (Thread 0) exited normally] Signed-off-by: KONRAD Frederic Reviewed-by: Philippe Mathieu-Daudé --- gdbstub.c | 7 +++ include/exec/gdbstub.h | 2 ++ vl.c | 2 ++ 3 files changed, 11 insertions(+) diff --git a/gdbstub.c b/gdbstub.c index f1d5148..a76b2fa 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -2052,6 +2052,13 @@ int gdbserver_start(const char *device) return 0; } +void gdbserver_cleanup(void) +{ +if (gdbserver_state) { +put_packet(gdbserver_state, "W00"); +} +} + static void register_types(void) { type_register_static(&char_gdb_type_info); diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h index 9aa7756..2e8a4b8 100644 --- a/include/exec/gdbstub.h +++ b/include/exec/gdbstub.h @@ -103,6 +103,8 @@ int gdbserver_start(int); int gdbserver_start(const char *port); #endif +void gdbserver_cleanup(void); + /** * gdb_has_xml: * This is an ugly hack to cope with both new and old gdb. diff --git a/vl.c b/vl.c index 3ef04ce..0427b15 100644 --- a/vl.c +++ b/vl.c @@ -4723,6 +4723,8 @@ int main(int argc, char **argv, char **envp) main_loop(); +gdbserver_cleanup(); + /* No more vcpu or device emulation activity beyond this point */ vm_shutdown();
[Qemu-devel] [PULL 0/1] Seabios 1.11.1 20180319 patches
The following changes since commit e1e44a9916b4318e943aecd669e096222cb3eaeb: Merge remote-tracking branch 'remotes/xtensa/tags/20180316-xtensa' into staging (2018-03-17 14:15:03 +) are available in the git repository at: git://git.kraxel.org/qemu tags/seabios-1.11.1-20180319-pull-request for you to fetch changes up to 9cdd2a736b99bad19fb4f88d2230c75f680c31ec: update seabios to 1.11.1 (2018-03-19 11:18:29 +0100) update seabios to 1.11.1 Gerd Hoffmann (1): update seabios to 1.11.1 pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes pc-bios/bios.bin | Bin 131072 -> 131072 bytes pc-bios/vgabios-cirrus.bin | Bin 38400 -> 38400 bytes pc-bios/vgabios-qxl.bin| Bin 38912 -> 38912 bytes pc-bios/vgabios-stdvga.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios-virtio.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios-vmware.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios.bin| Bin 38400 -> 38400 bytes roms/seabios | 2 +- 9 files changed, 1 insertion(+), 1 deletion(-) -- 2.9.3
[Qemu-devel] [PULL 1/1] update seabios to 1.11.1
git shortlog rel-1.11.0..rel-1.11.1 === Kevin O'Connor (3): build: Use git describe --always shadow: Don't invoke a shutdown on reboot unless in a reboot loop paravirt: Only enable sercon in NOGRAPHIC mode if no other console specified Marcel Apfelbaum (1): pci: fix 'io hints' capability for RedHat PCI bridges Signed-off-by: Gerd Hoffmann --- pc-bios/bios-256k.bin | Bin 262144 -> 262144 bytes pc-bios/bios.bin | Bin 131072 -> 131072 bytes pc-bios/vgabios-cirrus.bin | Bin 38400 -> 38400 bytes pc-bios/vgabios-qxl.bin| Bin 38912 -> 38912 bytes pc-bios/vgabios-stdvga.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios-virtio.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios-vmware.bin | Bin 38912 -> 38912 bytes pc-bios/vgabios.bin| Bin 38400 -> 38400 bytes roms/seabios | 2 +- 9 files changed, 1 insertion(+), 1 deletion(-) diff --git a/pc-bios/bios-256k.bin b/pc-bios/bios-256k.bin index e1d6b159277844320e3b1dfcbc74946ddef29026..0061dc99288bee61e60e31d1e24a52b97b0c5fb6 100644 GIT binary patch delta 39899 zcma%j2V7J~*Z!TMir|8PBBFq-ii!#<3WAD&qJoM6drz#vs@9TZ+eVT~+e*@(?IdYdv?NW9k)$7izz&l13|Jp0Nwqpj z(mh~yXGvNF1ay(4jINSYw;O1IXTWFO5eK{h?tCUmSxJ(#2G|1dWJzkEQ&F4YZn=CP|wnN|Nu_k`x6@ z2XZD!(l0=>N5kC04Xyi=>hWfNSCCAz!KmO;P@O#x(Wno!3gjz zuoZX(Xfh;82V4SzG9@VuaGWbiE@}8P7+40B0_opK(hcD2EJ@l1+yyK^lunWoft$c< zz<-`3H3#|uV}T4{8L$h;2WslkYM=wq8^{2b0s8{oD)yazc`9PD+vEvC+--AK| zUH*iknULR%E_ejSAEUcUF*Ki}0YKSbp!pkV|47mWp!N$%Y6`q7lcege(f=v0S*JnoJpy`0iXal34CeCqzS-eU!}xb$(l#Ke3X?K{(<&x~x-#iF(6cI&1_9Z?V^1cH@M6+JU>}g} z&7@y}^|hIF19%U7=F6mAz-{0kV3{A2!UDhmP)p6E=D;^VbX_L>1Q_e4fn&TB0o&>^ z={j&5xCh9-%0Nn@j#ln!hK4gu$Zdq5fB9K$3( zpaswg7ywL2!=DUbIglUAq)R|TM4c87dg)27C=<18adjfJZwF6<{1N8`uY&1zrLJqhO_gCBSY# z2J+iud`C-C4PXuM18^010K~;$SOJHCGcjpsK`f>&U=WZ3tOf1?ZXIBKfN0Hr}?6tE*slAJohU;zG|VO)T9zMBWXfN{VQU@LG9sN4;r1bPDFfI{GRplWw?3Gg{E8Tbx31DJv5 zfL)>_senKrZ65xt`Am}XfJ=ZGcn%~cAtUexsGkfo*Aw$D&05v>yr)0x~e@b4haj z0+j-zfa$<~;0j;@T!z8E4~G%~{{YoTz`+1AfhE8>z&Hw091UiGtS{l6jD>Ci7g2 zbwD2=8~7P;odQn>n3#%kftgd`FaU?9!4pcupNBxl=@@Fj1Yi|#2KamiIGzdZ1{MPk zf$&+F^nq`HmB3!$8c-h%d7O?e0lu9rNk@P_bKu(nEwz#q3uFQ|8BlB>80ZY74Z@$B zz$3sZ69jYNg#d2fK>L9QKpEhWg?SmM4@3aHfH6Qn9UMfU!F-4lh%&%#0AB%nf#<*t zI3c>l=vp8TaQYUb3^=<4{Xc6x6a#3s0sR7G0qz@7AP@@_1KC?p@mA;p@H0>bEZ7DK z0NuAka==>PDe(Ib7~?x&gn;flVIP3AfZr~30k93&58MVkcSCG#cY|Xf9XJ5I0z&to z5Fit%v==r2=myLK9sxCf#JC2EfJ?wz;PZX(bbzP(p^QJl-3DF&bq_#Vz(^nusG0*u z0$2-NNyDEWxzKRnFyNL4tp&~lPDb<}@D8YS5ZnTjfFFRnK(#}X)D_qP{0g`nhUWov zIU>OX!e=jn_fw3~0epP|dI8)8oKC_xoyMs91^wR}e?|f+z)Ij3fS-ZG2gpF~SxNd0 z=y49p2uuPN04srQKrZmhc?{PJXapdD4Zvx@47>pV)|fBYpF zmCN81_!F@C9d0x5Jx~mIl%R0nF7OVhdIjbd2n1RKU4gHF&Q~RAJ8e=tIT1mGLs8t@2c`2tD@c)i3t0kkiJH3Y^11%SgV zC>dY^l3qjPKpBwn28{(ufZK1;|Bc^4V}XIdQJ~g)Nooho0#*P8Ku3!tjR3|2I$$wS z58mn%pfbGF3&2~T8vLF)KsGSXmPtE-KkeW~*)u5)_!3wFoCRt-FsZ2noWzZIIROYq zCS3s@0sjD%o#1-`BY^LLqkyV1G67k@R^V@-hBJIQAQRXQ*udlU02%?2KpJoikb#%L zTfhz8@Hikn4S)6lMZi;l!$qzE1OYvPQGgNn2T;RpP5_1j%#BIyfnmTNAOLRkTp%B~ z0XzZX-I+8KxCy)ls(CP}C6EY=0KNv&=Hkz4zzF;bJOfnl$D0BPKtEs&FrgZgb^}*{ zKY>TU#Oh44hrb>M^arj2t~KDH0)v6gKo9uwvw%%N&6-TgtHq?D@aG-j!`Jl1A|W0x z1A&u3DbNm%eP5smFau@4JD{CE`VDvhIM#u~55xf7fn~s2AP<-y012p>GzETto1dA~ zaV(d9n8c+K*&qKwo3bV4znq3&vj&y;51mBM>8v&H(^)K<&U~8>?~Fb!2zfF4Jv2>9 zyP9!Qb6Iu3q;pp59_mWuj!4$M4m+=RR%=v-o(xgVbN-ml>};H_cEsYRaEi~m6XN+S zHkmJr6SdiqW)J>nRVw0=sCwYXw zpW>x^j?(UT5q?>$GoKYH#$>UYd|_lxRu=1M$DGAE0~^2xMvBG@m^W)LIxS$n_WqHm zBT|GcV4h;m0??Zy#F_=H0c$8uE?}{IdW29dWI?Q}h+N2;RIeQY<{2^Xt}w4X=Gfmf zDO}86$l9}LQMi!JuDYQGlPbxooD;g&y3#>|2bwC0UfC?#b4?pm6|43tj#c|0v#qX7 zp5I37%w~016>&bBRp&`$zM@)LzV%G2fh@7PCmkGvdTw-?FA(R>O)gNmqz|$twlRMpslSGeoL# zZ1VoLL5v5*JZ)-?nLspN6mz3IMng2jYJbxyB?`SRWuz+wnQ{@+yH}E~NY*sXnZJZ3 zu&S4!Zsx~@s8bsuX&mHZoyEhYEQl}bEWDPn#-2&?&RB?`5WVycFPZr%4sw!sw~Xy` z=}AQ$*Im|~mX~xApDkm9(>haWbyPV=jjnbW2u~rdd!3l1|26|0WnR=c=t^Z<(CD=k zp@^yfpuvT`lC+Nk!vlSjL>Je@Z%D$$VabnDJ=Ivc8fk|Mu2L$4wmIuGHSkN(gyw(?Mmb1qEa=bXZ zoWT8yP)`j1j)gQI_8CU&8{f%p#{7*{q}%}Nt3jrl7#w3x zJL~q2Lb7i5So*tY0{+J6Y7G-7zhmurdVG%a_bi_C6OBaQm8>0amnptq$-d^ZI*BT) z*gsHz+tsW;J1>T;X5D$(8nJgZGgkG5)dRGTYy-d4TGUy~>NSb( z08!M+Bb`C&?=W$?+PYbey63ss!>UN~L$oo@d{8^xi`1i)n7o#?<(Ff{fwgRK)fCc= zU|p)CZj^22wG_L9-;4U|*aerxWEPfKOKc$Q*0V<3X1Qp#o+Wv|=g?LO>EKBVio8H8 zS+>7evz~2WokYwA*1W0*lp!SF+%hl7n!LqNWFe6a6j2*l9r4!&=FMle7pjdcm_P9q z?KiRrjL4ZA*-uUSUvWZRQ588=qo0tzj z*-pIL#2R_@2xU^&l*dNRc61SGqCCEx2;0mWdb%U!-jMhdvoV>{;wY`Dn6jCT3jU!j zlT0XI%A;Y6+GRNcQ&MpUiL6g?2glKAw=Ly@24}uPwT0DScA~)+*2|-D7`mY}Pp#yH zdOON1eZ;q0SXbsI?rdQbqt`cstZmi$7+1r14}I)b+pVseW~8Bak&v0M@ii{r28QB^ zq53de`i_qJDA!^TGKkzGmRkcB$Pg`-ThDhAb zqWRVcv1B`2Qq>L`ZM{PLb?G_LoAg zMhPrKF3$4Tkz&^mtg-Lcux4nthw;!>@Dc^XC+CDS34_|~Chu=9UjM*+-L5r30mXS_ zBbBMfMl{^Pya$$n3$&A4i*PeO+KeJFP$W4HMVKArxaN55FSe!ru=1;9&nNClQc7q& zLg2s+-SRPVRSmIj2a63>QO(Eo4*sR#C7OEBh2Drz_)VnB5Rl}R=yc+@lKfpW;klEA zse86Uy-C_4XI+8Op6V%;cOwcCsA4>^zKXQxtlUgY+{xUX!r;(=i`#M)v0x{QX%)~| zsjs80zK@TtM0=Zs7i;Rt$I)TwfAADPFwhSaVBos(Itt3YM+~fKB^-CLFPN_wyNlKA z^A#*7raN1GjEA0-Z7bI_(IYJHL;|vU8wXIo!?~B_=Pj9JaY-nwBAJFzQ6|h%<|5=Q zj&el0
[Qemu-devel] qapi escape-too-big test doesn't work if LANG=C ?
I recently tweaked my build scripts to run with LANG=C (trying to suppress gcc's irritating habit of using smartquotes rather than plain old ''). This seems to result in an error running the qapi-schema/escape-too-big test: PYTHONPATH=/home/petmay01/linaro/qemu-for-merges/scripts python3 -B /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/test-qapi.py /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/escape-too-big.json >tests/qapi-schema/escape-too-big.test.out 2>tests/qapi-schema/escape-too-big.test.err; echo $? >tests/qapi-schema/escape-too-big.test.exit 1c1,10 < tests/qapi-schema/escape-too-big.json:3:14: For now, \u escape only supports non-zero values up to \u007f --- > Traceback (most recent call last): > File "tests/qapi-schema/test-qapi.py", line 64, in > schema = QAPISchema(sys.argv[1]) > File "scripts/qapi/common.py", line 1492, in __init__ > parser = QAPISchemaParser(open(fname, 'r')) > File "scripts/qapi/common.py", line 264, in __init__ > self.src = fp.read() > File "/usr/lib/python3.5/encodings/ascii.py", line 26, in decode > return codecs.ascii_decode(input, self.errors)[0] > UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 61: > ordinal not in range(128) /home/petmay01/linaro/qemu-for-merges/tests/Makefile.include:927: recipe for target 'check-tests/qapi-schema/escape-too-big.json' failed thanks -- PMM
Re: [Qemu-devel] [PATCH 1/2] char: i.MX: Simplify imx_update()
On 15 March 2018 at 19:11, Andrey Smirnov wrote: > Code of imx_update() is slightly confusing since the "flags" variable > doesn't really corespond to anything in real hardware and server as a > kitchensink accumulating events normally reported via USR1 and USR2 > registers. > > Change the code to explicitly evaluate state of interrupts reported > via USR1 and USR2 against corresponding masking bits and use the to > detemine if IRQ line should be asserted or not. > > NOTE: Check for UTS1_TXEMPTY being set has been dropped for two > reasons: > > 1. Emulation code implements a single character FIFO, so this flag >will always be set since characters are trasmitted as a part of >the code emulating "push" into the FIFO > > 2. imx_update() is really just a function doing ORing and maksing >of reported events, so checking for UTS1_TXEMPTY should happen, >if it's ever really needed should probably happen outside of >it. > > Cc: qemu-devel@nongnu.org > Cc: qemu-...@nongnu.org > Cc: Bill Paul > Cc: Peter Maydell > Signed-off-by: Andrey Smirnov > --- > hw/char/imx_serial.c | 24 Thanks; I've applied this patch and patch 2 to target-arm.next. As bugfixes they'll go into 2.12. PS: if you could provide cover letters for patchsets that have more than one patch in them that would help me in finding and processing them. thanks -- PMM
Re: [Qemu-devel] HELP
On Sat, Mar 17, 2018 at 8:37 PM, Projat Banerjee wrote: > What is the type of proposal should I submit here ? What kind or on what > basis should I build my proposal so that I may get easily selected or chances > for my selection is high ? Are you referring to Google Summer of Code? Outreachy? Something else? For Google Summer of Code, please read the GSoC 2018 wiki page: https://wiki.qemu.org/Google_Summer_of_Code_2018 It contains a list of project ideas that you can choose to apply for. Please contact the mentor for the project idea you are interested in. You do not need to send a proposal to qemu-devel@nongnu.org. How to apply is covered on the wiki page above. I have written up some advice for applicants which answers some of your questions (it's also linked from the wiki page above): http://blog.vmsplice.net/2011/03/advice-for-students-applying-to-google.html For Outreachy, please follow the instructions at: https://www.outreachy.org/apply/ Stefan
Re: [Qemu-devel] [PATCH 2/8] migration: stop allocating and freeing memory frequently
* Xiao Guangrong (guangrong.x...@gmail.com) wrote: > > > On 03/15/2018 07:03 PM, Dr. David Alan Gilbert wrote: > > > > +static int compress_threads_load_setup(void) > > > +{ > > > +int i, thread_count; > > > + > > > +if (!migrate_use_compression()) { > > > +return 0; > > > +} > > > + > > > +thread_count = migrate_decompress_threads(); > > > +decompress_threads = g_new0(QemuThread, thread_count); > > > +decomp_param = g_new0(DecompressParam, thread_count); > > > +qemu_mutex_init(&decomp_done_lock); > > > +qemu_cond_init(&decomp_done_cond); > > > +for (i = 0; i < thread_count; i++) { > > > +if (inflateInit(&decomp_param[i].stream) != Z_OK) { > > > +goto exit; > > > +} > > > +decomp_param[i].stream.opaque = &decomp_param[i]; > > > + > > > +qemu_mutex_init(&decomp_param[i].mutex); > > > +qemu_cond_init(&decomp_param[i].cond); > > > +decomp_param[i].compbuf = > > > g_malloc0(compressBound(TARGET_PAGE_SIZE)); > > > +decomp_param[i].done = true; > > > +decomp_param[i].quit = false; > > > +qemu_thread_create(decompress_threads + i, "decompress", > > > + do_data_decompress, decomp_param + i, > > > + QEMU_THREAD_JOINABLE); > > > +} > > > +return 0; > > > +exit: > > > +compress_threads_load_cleanup(); > > > > I don't think this is safe; if inflateInit(..) fails in not-the-last > > thread, compress_threads_load_cleanup() will try and destroy all the > > mutex's and condition variables, even though they've not yet all been > > _init'd. > > > > That is exactly why we used 'opaque', please see more below... > > > However, other than that I think the patch is OK; a chat with Dan > > Berrange has convinced me this probably doesn't affect the stream > > format, so that's OK. > > > > One thing I would like is a comment as to how the 'opaque' field is > > being used; I don't think I quite understand what you're doing there. > > The zlib.h file says that: > " The opaque value provided by the application will be passed as the first >parameter for calls of zalloc and zfree. This can be useful for custom >memory management. The compression library attaches no meaning to the >opaque value." > So we can use it to store our private data. > > Here, we use it as a indicator which shows if the thread is properly init'd > or not. If inflateInit() is successful we will set it to non-NULL, otherwise > it is NULL, so that the cleanup path can figure out the first thread failed > to do inflateInit(). OK, so I think you just need to add a comment to explain that. Put it above the 'if (!decomp_param[i].stream.opaque) {' in compress_threads_load_cleanup so it'll be easy to understand. Dave > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PULL 0/8] ppc-for-2.12 queue 20180319
On 19 March 2018 at 00:35, David Gibson wrote: > The following changes since commit e1e44a9916b4318e943aecd669e096222cb3eaeb: > > Merge remote-tracking branch 'remotes/xtensa/tags/20180316-xtensa' into > staging (2018-03-17 14:15:03 +) > > are available in the Git repository at: > > git://github.com/dgibson/qemu.git tags/ppc-for-2.12-20180319 > > for you to fetch changes up to 91c60f124e682a78c9a2ef951e8e58ebab6441d0: > > target/ppc: fix tlbsync to check privilege level depending on GTSE > (2018-03-18 21:03:20 +1100) > > > ppc patch queue for 2018-03-19 > > This pull request supersedes the one for 2018-03-15. The only > difference is one patch is removed, since it exposed some code which > triggers ubsan warnings. > > Here's the set of accumulated patches now that we're into soft freeze. > I've split new functionality into a ppc-for-2.13 branch, so this only > has bugfixes. Well.. and a couple of simple cleanups to make bugfixes > easier, some test improvements and a trivial change to make command > line options more obvious. I think those are all acceptable for soft > freeze. > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type
On 13 March 2018 at 23:16, Philippe Mathieu-Daudé wrote: > On 03/13/2018 06:09 PM, Peter Maydell wrote: >> On 13 March 2018 at 16:55, Andrew Baumann >> wrote: >>> At some point I remember seeing a patch to change this to cortex-a7. Is >>> there a reason we didn't make that change? >>> >>> (Background: the real Pi2 has an A7. When I first implemented the machine >>> model there was no A7 emulation in QEMU, so I used the A15 which was the >>> closest equivalent.) >> >> Yeah, we should do that. I'd forgotten about that, I think >> things just got lost in the shuffle of having several >> patchsets that tried to change the same things at once. >> >> I guess the simplest thing is to add a patch at the end of >> the series that fixes the cpu type for bcm2836. > > Peter, here is the patch Andrew remembered (maybe can be applied at the > end): > http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg04286.html Thanks for finding that. It doesn't apply after this refactoring, but I'll send out a patch which does the equivalent thing in the new code. In the meantime I'm going to apply this patchset to target-arm.next since I'm going to send out a pullreq with bugfixes for rc0 today. thanks -- PMM
[Qemu-devel] [PATCH for-2.12] hw/arm/bcm2836: Use the Cortex-A7 instead of Cortex-A15
The BCM2836 uses a Cortex-A7, not a Cortex-A15. Update the device to use the correct CPU. https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2836/QA7_rev3.4.pdf When the BCM2836 was introduced (bad5623690b) the Cortex-A7 was not available, so the very similar Cortex-A15 was used. Since dcf578ed8ce we can model the correct core. Signed-off-by: Peter Maydell --- This was originally done by a patch from Alistair: http://lists.nongnu.org/archive/html/qemu-devel/2018-02/msg00311.html The refactor of bcm2836 means the code is different, but I'm using the same commit message. Based-on: <20180313153458.26822-1-peter.mayd...@linaro.org> --- hw/arm/bcm2836.c | 2 +- hw/arm/raspi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c index 25e74b67d9..8c1e707d9c 100644 --- a/hw/arm/bcm2836.c +++ b/hw/arm/bcm2836.c @@ -32,7 +32,7 @@ struct BCM283XInfo { static const BCM283XInfo bcm283x_socs[] = { { .name = TYPE_BCM2836, -.cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"), +.cpu_type = ARM_CPU_TYPE_NAME("cortex-a7"), .clusterid = 0xf, }, { diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 06f1e08ca9..955a7c4e80 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -226,7 +226,7 @@ static void raspi2_machine_init(MachineClass *mc) mc->no_parallel = 1; mc->no_floppy = 1; mc->no_cdrom = 1; -mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"); +mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a7"); mc->max_cpus = BCM283X_NCPUS; mc->min_cpus = BCM283X_NCPUS; mc->default_cpus = BCM283X_NCPUS; -- 2.16.2
[Qemu-devel] [PULL v3 00/46] Block layer patches
The following changes since commit e1e44a9916b4318e943aecd669e096222cb3eaeb: Merge remote-tracking branch 'remotes/xtensa/tags/20180316-xtensa' into staging (2018-03-17 14:15:03 +) are available in the git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 63ca8406beac44aa59c389ed8578d0c7b3da3402: iotests: Avoid realpath, for CentOS 6 (2018-03-19 12:01:39 +0100) Block layer patches Eric Blake (1): iotests: Avoid realpath, for CentOS 6 Fam Zheng (4): block: Fix flags in reopen queue iotests: Add regression test for commit base locking vvfat: Fix inherit_options flags block: Fix leak of ignore_children in error path Jeff Cody (1): block: fix iotest 146 output expectations John Snow (21): blockjobs: fix set-speed kick blockjobs: model single jobs as transactions Blockjobs: documentation touchup blockjobs: add status enum blockjobs: add state transition table iotests: add pause_wait blockjobs: add block_job_verb permission table blockjobs: add ABORTING state blockjobs: add CONCLUDED state blockjobs: add NULL state blockjobs: add block_job_dismiss blockjobs: ensure abort is called for cancelled jobs blockjobs: add commit, abort, clean helpers blockjobs: add block_job_txn_apply function blockjobs: add prepare callback blockjobs: add waiting status blockjobs: add PENDING status and event blockjobs: add block-job-finalize blockjobs: Expose manual property iotests: test manual job dismissal tests/test-blockjob: test cancellations Kevin Wolf (14): luks: Separate image file creation from formatting luks: Create block_crypto_co_create_generic() luks: Support .bdrv_co_create luks: Turn invalid assertion into check luks: Catch integer overflow for huge sizes qemu-iotests: Test luks QMP image creation parallels: Support .bdrv_co_create qemu-iotests: Enable write tests for parallels qcow: Support .bdrv_co_create qed: Support .bdrv_co_create vdi: Make comments consistent with other drivers vhdx: Support .bdrv_co_create vpc: Support .bdrv_co_create vpc: Require aligned size in .bdrv_co_create Liang Li (1): block/mirror: change the semantic of 'force' of block-job-cancel Max Reitz (3): vdi: Pull option parsing from vdi_co_create vdi: Move file creation to vdi_co_create_opts vdi: Implement .bdrv_co_create Paolo Bonzini (1): iscsi: fix iSER compilation qapi/block-core.json | 363 -- include/block/blockjob.h | 71 - include/block/blockjob_int.h | 17 +- block.c | 10 +- block/backup.c| 5 +- block/commit.c| 2 +- block/crypto.c| 150 - block/iscsi.c | 2 +- block/mirror.c| 12 +- block/parallels.c | 199 +-- block/qcow.c | 196 +++ block/qed.c | 204 block/stream.c| 2 +- block/vdi.c | 147 + block/vhdx.c | 216 +++-- block/vpc.c | 241 +--- block/vvfat.c | 2 +- blockdev.c| 71 +++-- blockjob.c| 358 +++-- tests/test-bdrv-drain.c | 5 +- tests/test-blockjob-txn.c | 27 ++-- tests/test-blockjob.c | 233 ++- block/trace-events| 7 + hmp-commands.hx | 3 +- tests/qemu-iotests/030| 6 +- tests/qemu-iotests/055| 17 +- tests/qemu-iotests/056| 187 ++ tests/qemu-iotests/056.out| 4 +- tests/qemu-iotests/109.out| 24 +-- tests/qemu-iotests/146.out| 2 +- tests/qemu-iotests/153| 12 ++ tests/qemu-iotests/153.out| 5 + tests/qemu-iotests/181| 2 +- tests/qemu-iotests/210| 210 tests/qemu-iotests/210.out| 136 tests/qemu-iotests/check | 13 +- tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/group | 1 + tests/qemu-iotests/iotests.py | 12 +- 39 files changed, 2652 insertions(+), 524 deletions(-) create mode 100755 tests/qemu-iotests/210 create mode 100644 tests/qemu-iotests/210.out
Re: [Qemu-devel] qapi escape-too-big test doesn't work if LANG=C ?
On Mon, Mar 19, 2018 at 10:37:12AM +, Peter Maydell wrote: > I recently tweaked my build scripts to run with LANG=C (trying > to suppress gcc's irritating habit of using smartquotes rather > than plain old ''). This seems to result in an error running > the qapi-schema/escape-too-big test: > > PYTHONPATH=/home/petmay01/linaro/qemu-for-merges/scripts python3 -B > /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/test-qapi.py > /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/escape-too-big.json > >tests/qapi-schema/escape-too-big.test.out > 2>tests/qapi-schema/escape-too-big.test.err; echo $? > >tests/qapi-schema/escape-too-big.test.exit > 1c1,10 > < tests/qapi-schema/escape-too-big.json:3:14: For now, \u escape only > supports non-zero values up to \u007f > --- > > Traceback (most recent call last): > > File "tests/qapi-schema/test-qapi.py", line 64, in > > schema = QAPISchema(sys.argv[1]) > > File "scripts/qapi/common.py", line 1492, in __init__ > > parser = QAPISchemaParser(open(fname, 'r')) > > File "scripts/qapi/common.py", line 264, in __init__ > > self.src = fp.read() > > File "/usr/lib/python3.5/encodings/ascii.py", line 26, in decode > > return codecs.ascii_decode(input, self.errors)[0] > > UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 61: > > ordinal not in range(128) > /home/petmay01/linaro/qemu-for-merges/tests/Makefile.include:927: > recipe for target 'check-tests/qapi-schema/escape-too-big.json' failed So your "C" locale will be non-UTF-8, except on OS-X where the "C" locale is UTF-8 by default. Unfortunately while POSIX expects the "C" locale to be 8-bit cleanup, Python by default will reject any characters outside the 7-bit range with its "ascii" codec. So this is ultimately a python bug, but there's little we can do about that given how widely deployed the bug is. To workaround this problem in other applications what I have done is add the following to Makefiles before invoking python: LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 The LC_ALL= bit is needed because if the user has set LC_ALL themselves it will override LANG and all other LC_* variables. Setting LANG=C is not strictly needed, as LC_CTYPE will override it. CC'ing Eric since he was involved in the discussions about this bug in other libvirt related apps. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PATCH v2 0/2] target/m68k: add a mechanism to automatically free TCGv
SRC_EA() and gen_extend() can return either a temporary TCGv or a memory allocated one. Mark them when they are allocated, and free them automatically at end of the instruction translation. We want to free locally allocated TCGv to avoid overflow in sequence like: 0xc00ae406: movel %fp@(-132),%fp@(-268) 0xc00ae40c: movel %fp@(-128),%fp@(-264) 0xc00ae412: movel %fp@(-20),%fp@(-212) 0xc00ae418: movel %fp@(-16),%fp@(-208) 0xc00ae41e: movel %fp@(-60),%fp@(-220) 0xc00ae424: movel %fp@(-56),%fp@(-216) 0xc00ae42a: movel %fp@(-124),%fp@(-252) 0xc00ae430: movel %fp@(-120),%fp@(-248) 0xc00ae436: movel %fp@(-12),%fp@(-260) 0xc00ae43c: movel %fp@(-8),%fp@(-256) 0xc00ae442: movel %fp@(-52),%fp@(-276) 0xc00ae448: movel %fp@(-48),%fp@(-272) ... That can fill a lot of TCGv entries in a sequence, especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps") we have no limit to fill the TCGOps cache and we can fill the entire TCG variables array and overflow it. v2: split patch in two (separate the patch to add parameter to gen_exten()) mark to release missed gen_load() in gen_lea_indexed() Laurent Vivier (2): target/m68k: add DisasContext parameter to gen_extend() target/m68k: add a mechanism to automatically free TCGv target/m68k/translate.c | 102 +++- 1 file changed, 66 insertions(+), 36 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH v2 1/2] target/m68k: add DisasContext parameter to gen_extend()
This parameter will be needed to manage automatic release of temporary allocated TCG variables. Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 46 +++--- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index cef6f663ad..1c2ff56305 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -617,7 +617,7 @@ static void gen_flush_flags(DisasContext *s) s->cc_op = CC_OP_FLAGS; } -static inline TCGv gen_extend(TCGv val, int opsize, int sign) +static inline TCGv gen_extend(DisasContext *s, TCGv val, int opsize, int sign) { TCGv tmp; @@ -811,7 +811,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, gen_partset_reg(opsize, reg, val); return store_dummy; } else { -return gen_extend(reg, opsize, what == EA_LOADS); +return gen_extend(s, reg, opsize, what == EA_LOADS); } case 1: /* Address register direct. */ reg = get_areg(s, reg0); @@ -819,7 +819,7 @@ static TCGv gen_ea_mode(CPUM68KState *env, DisasContext *s, int mode, int reg0, tcg_gen_mov_i32(reg, val); return store_dummy; } else { -return gen_extend(reg, opsize, what == EA_LOADS); +return gen_extend(s, reg, opsize, what == EA_LOADS); } case 2: /* Indirect register */ reg = get_areg(s, reg0); @@ -1759,8 +1759,8 @@ DISAS_INSN(abcd_reg) gen_flush_flags(s); /* !Z is sticky */ -src = gen_extend(DREG(insn, 0), OS_BYTE, 0); -dest = gen_extend(DREG(insn, 9), OS_BYTE, 0); +src = gen_extend(s, DREG(insn, 0), OS_BYTE, 0); +dest = gen_extend(s, DREG(insn, 9), OS_BYTE, 0); bcd_add(dest, src); gen_partset_reg(OS_BYTE, DREG(insn, 9), dest); @@ -1794,8 +1794,8 @@ DISAS_INSN(sbcd_reg) gen_flush_flags(s); /* !Z is sticky */ -src = gen_extend(DREG(insn, 0), OS_BYTE, 0); -dest = gen_extend(DREG(insn, 9), OS_BYTE, 0); +src = gen_extend(s, DREG(insn, 0), OS_BYTE, 0); +dest = gen_extend(s, DREG(insn, 9), OS_BYTE, 0); bcd_sub(dest, src); @@ -1856,7 +1856,7 @@ DISAS_INSN(addsub) add = (insn & 0x4000) != 0; opsize = insn_opsize(insn); -reg = gen_extend(DREG(insn, 9), opsize, 1); +reg = gen_extend(s, DREG(insn, 9), opsize, 1); dest = tcg_temp_new(); if (insn & 0x100) { SRC_EA(env, tmp, opsize, 1, &addr); @@ -2386,7 +2386,7 @@ DISAS_INSN(cas) return; } -cmp = gen_extend(DREG(ext, 0), opsize, 1); +cmp = gen_extend(s, DREG(ext, 0), opsize, 1); /* if == Dc then * = Du @@ -3055,7 +3055,7 @@ DISAS_INSN(or) int opsize; opsize = insn_opsize(insn); -reg = gen_extend(DREG(insn, 9), opsize, 0); +reg = gen_extend(s, DREG(insn, 9), opsize, 0); dest = tcg_temp_new(); if (insn & 0x100) { SRC_EA(env, src, opsize, 0, &addr); @@ -3120,8 +3120,8 @@ DISAS_INSN(subx_reg) opsize = insn_opsize(insn); -src = gen_extend(DREG(insn, 0), opsize, 1); -dest = gen_extend(DREG(insn, 9), opsize, 1); +src = gen_extend(s, DREG(insn, 0), opsize, 1); +dest = gen_extend(s, DREG(insn, 9), opsize, 1); gen_subx(s, src, dest, opsize); @@ -3176,7 +3176,7 @@ DISAS_INSN(cmp) opsize = insn_opsize(insn); SRC_EA(env, src, opsize, 1, NULL); -reg = gen_extend(DREG(insn, 9), opsize, 1); +reg = gen_extend(s, DREG(insn, 9), opsize, 1); gen_update_cc_cmp(s, reg, src, opsize); } @@ -3329,8 +3329,8 @@ DISAS_INSN(addx_reg) opsize = insn_opsize(insn); -dest = gen_extend(DREG(insn, 9), opsize, 1); -src = gen_extend(DREG(insn, 0), opsize, 1); +dest = gen_extend(s, DREG(insn, 9), opsize, 1); +src = gen_extend(s, DREG(insn, 0), opsize, 1); gen_addx(s, src, dest, opsize); @@ -3369,7 +3369,7 @@ static inline void shift_im(DisasContext *s, uint16_t insn, int opsize) int logical = insn & 8; int left = insn & 0x100; int bits = opsize_bytes(opsize) * 8; -TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical); +TCGv reg = gen_extend(s, DREG(insn, 0), opsize, !logical); if (count == 0) { count = 8; @@ -3419,7 +3419,7 @@ static inline void shift_reg(DisasContext *s, uint16_t insn, int opsize) int logical = insn & 8; int left = insn & 0x100; int bits = opsize_bytes(opsize) * 8; -TCGv reg = gen_extend(DREG(insn, 0), opsize, !logical); +TCGv reg = gen_extend(s, DREG(insn, 0), opsize, !logical); TCGv s32; TCGv_i64 t64, s64; @@ -3556,7 +3556,7 @@ DISAS_INSN(shift_mem) while M68000 sets if the most significant bit is changed at any time during the shift operation */ if (!logical && m68k_feature(s->env, M68K_FEATURE_M68000)) { -src = gen_extend(src, OS_WORD, 1); +src = gen_extend(s, src, OS_WORD, 1);
[Qemu-devel] [PATCH v2 2/2] target/m68k: add a mechanism to automatically free TCGv
SRC_EA() and gen_extend() can return either a temporary TCGv or a memory allocated one. Mark them when they are allocated, and free them automatically at end of the instruction translation. We want to free locally allocated TCGv to avoid overflow in sequence like: 0xc00ae406: movel %fp@(-132),%fp@(-268) 0xc00ae40c: movel %fp@(-128),%fp@(-264) 0xc00ae412: movel %fp@(-20),%fp@(-212) 0xc00ae418: movel %fp@(-16),%fp@(-208) 0xc00ae41e: movel %fp@(-60),%fp@(-220) 0xc00ae424: movel %fp@(-56),%fp@(-216) 0xc00ae42a: movel %fp@(-124),%fp@(-252) 0xc00ae430: movel %fp@(-120),%fp@(-248) 0xc00ae436: movel %fp@(-12),%fp@(-260) 0xc00ae43c: movel %fp@(-8),%fp@(-256) 0xc00ae442: movel %fp@(-52),%fp@(-276) 0xc00ae448: movel %fp@(-48),%fp@(-272) ... That can fill a lot of TCGv entries in a sequence, especially since 15fa08f845 ("tcg: Dynamically allocate TCGOps") we have no limit to fill the TCGOps cache and we can fill the entire TCG variables array and overflow it. Suggested-by: Richard Henderson Signed-off-by: Laurent Vivier --- target/m68k/translate.c | 56 + 1 file changed, 43 insertions(+), 13 deletions(-) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 1c2ff56305..6beaf9ed66 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -123,8 +123,34 @@ typedef struct DisasContext { int done_mac; int writeback_mask; TCGv writeback[8]; +#define MAX_TO_RELEASE 8 +int release_count; +TCGv release[MAX_TO_RELEASE]; } DisasContext; +static void init_release_array(DisasContext *s) +{ +#ifdef CONFIG_DEBUG_TCG +memset(s->release, 0, sizeof(s->release)); +#endif +s->release_count = 0; +} + +static void do_release(DisasContext *s) +{ +int i; +for (i = 0; i < s->release_count; i++) { +tcg_temp_free(s->release[i]); +} +init_release_array(s); +} + +static TCGv mark_to_release(DisasContext *s, TCGv tmp) +{ +g_assert(s->release_count < MAX_TO_RELEASE); +return s->release[s->release_count++] = tmp; +} + static TCGv get_areg(DisasContext *s, unsigned regno) { if (s->writeback_mask & (1 << regno)) { @@ -347,7 +373,8 @@ static TCGv gen_ldst(DisasContext *s, int opsize, TCGv addr, TCGv val, gen_store(s, opsize, addr, val, index); return store_dummy; } else { -return gen_load(s, opsize, addr, what == EA_LOADS, index); +return mark_to_release(s, gen_load(s, opsize, addr, + what == EA_LOADS, index)); } } @@ -439,7 +466,7 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) } else { bd = 0; } -tmp = tcg_temp_new(); +tmp = mark_to_release(s, tcg_temp_new()); if ((ext & 0x44) == 0) { /* pre-index */ add = gen_addr_index(s, ext, tmp); @@ -449,7 +476,7 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) if ((ext & 0x80) == 0) { /* base not suppressed */ if (IS_NULL_QREG(base)) { -base = tcg_const_i32(offset + bd); +base = mark_to_release(s, tcg_const_i32(offset + bd)); bd = 0; } if (!IS_NULL_QREG(add)) { @@ -465,11 +492,11 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) add = tmp; } } else { -add = tcg_const_i32(bd); +add = mark_to_release(s, tcg_const_i32(bd)); } if ((ext & 3) != 0) { /* memory indirect */ -base = gen_load(s, OS_LONG, add, 0, IS_USER(s)); +base = mark_to_release(s, gen_load(s, OS_LONG, add, 0, IS_USER(s))); if ((ext & 0x44) == 4) { add = gen_addr_index(s, ext, tmp); tcg_gen_add_i32(tmp, add, base); @@ -494,7 +521,7 @@ static TCGv gen_lea_indexed(CPUM68KState *env, DisasContext *s, TCGv base) } } else { /* brief extension word format */ -tmp = tcg_temp_new(); +tmp = mark_to_release(s, tcg_temp_new()); add = gen_addr_index(s, ext, tmp); if (!IS_NULL_QREG(base)) { tcg_gen_add_i32(tmp, add, base); @@ -624,7 +651,7 @@ static inline TCGv gen_extend(DisasContext *s, TCGv val, int opsize, int sign) if (opsize == OS_LONG) { tmp = val; } else { -tmp = tcg_temp_new(); +tmp = mark_to_release(s, tcg_temp_new()); gen_ext(tmp, val, opsize, sign); } @@ -746,7 +773,7 @@ static TCGv gen_lea_mode(CPUM68KState *env, DisasContext *s, return NULL_QREG; } reg = get_areg(s, reg0); -tmp = tcg_temp_new(); +tmp = mark_to_release(s, tcg_temp_new()); if (reg0 == 7 && opsize == OS_BYTE && m68k_feature(s->env, M68K_FEATURE_M68000)) { tcg_gen_sub
Re: [Qemu-devel] [PULL 0/1] Seabios 1.11.1 20180319 patches
On 19 March 2018 at 10:26, Gerd Hoffmann wrote: > The following changes since commit e1e44a9916b4318e943aecd669e096222cb3eaeb: > > Merge remote-tracking branch 'remotes/xtensa/tags/20180316-xtensa' into > staging (2018-03-17 14:15:03 +) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/seabios-1.11.1-20180319-pull-request > > for you to fetch changes up to 9cdd2a736b99bad19fb4f88d2230c75f680c31ec: > > update seabios to 1.11.1 (2018-03-19 11:18:29 +0100) > > > update seabios to 1.11.1 > > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH v8 5/9] block: treat BDRV_REQ_ALLOCATE as serialising
On Mon 12 Mar 2018 11:16:54 AM CET, Anton Nefedov wrote: > The idea is that ALLOCATE requests may overlap with other requests. > Reuse the existing block layer infrastructure for serialising requests. > Use the following approach: > - mark ALLOCATE serialising, so subsequent requests to the area wait > - ALLOCATE request itself must never wait if another request is in flight > already. Return EAGAIN, let the caller reconsider. > > Signed-off-by: Anton Nefedov Reviewed-by: Alberto Garcia > @@ -1498,8 +1507,13 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild > *child, > max_transfer = QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_transfer, > INT_MAX), > align); > > -waited = wait_serialising_requests(req); > -assert(!waited || !req->serialising); > +found = find_or_wait_serialising_requests(req, > + !(flags & BDRV_REQ_ALLOCATE)); > +if (found && (flags & BDRV_REQ_ALLOCATE)) { > +return -EAGAIN; > +} > + Another alternative (perhaps a bit more readable): if (flags & BDRV_REQ_ALLOCATE) { if (find_or_wait_serialising_requests(req, false)) { return -EAGAIN; } } else { bool found = wait_serialising_requests(req); assert(!found || !req->serialising); } but yours is fine, so keep it if you prefer. Berto
Re: [Qemu-devel] [Qemu-arm] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type
On 13 March 2018 at 15:34, Peter Maydell wrote: > Now we have separate types for BCM2386 and BCM2387, we might as well > just hard-code the CPU type they use rather than having it passed > through as an object property. This then lets us put the initialization > of the CPU object in init rather than realize. > > Signed-off-by: Peter Maydell > --- > hw/arm/bcm2836.c | 22 +- > hw/arm/raspi.c | 2 -- > 2 files changed, 13 insertions(+), 11 deletions(-) > > diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c > index 7140257c98..12f75b55a7 100644 > --- a/hw/arm/bcm2836.c > +++ b/hw/arm/bcm2836.c > @@ -25,16 +25,19 @@ > > struct BCM283XInfo { > const char *name; > +const char *cpu_type; > int clusterid; > }; > > static const BCM283XInfo bcm283x_socs[] = { > { > .name = TYPE_BCM2836, > +.cpu_type = ARM_CPU_TYPE_NAME("cortex-a15"), > .clusterid = 0xf, > }, > { > .name = TYPE_BCM2837, > +.cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"), > .clusterid = 0x0, > }, > }; With this change we need to also add #ifdef TARGET_AARCH64 #endif around the entry in the array for bcm2837. Otherwise the device-introspection tests in 'make check' will fail trying to create the bcm2837 in qemu-system-arm, because there there is no cortex-a53 device. I'll just squash that into this patch... thanks -- PMM
[Qemu-devel] [PATCH] scripts/decodetree: Fix insnmask not marked as global in main()
if '-w 16' was given as a cmdline args a local copy of insnmask is set and not the global one. Signed-off-by: Peer Adelt Signed-off-by: Bastian Koppelmann --- scripts/decodetree.py | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/decodetree.py b/scripts/decodetree.py index 41301c84aa..277f9a9bba 100755 --- a/scripts/decodetree.py +++ b/scripts/decodetree.py @@ -972,6 +972,7 @@ def main(): global input_file global insnwidth global insntype +global insnmask decode_function = 'decode' decode_scope = 'static ' -- 2.16.2
Re: [Qemu-devel] [PATCH] migration: Fix rate limiting issue on RDMA migration
ping. On Thu, Mar 15, 2018 at 1:33 PM, 858585 jemmy wrote: > On Thu, Mar 15, 2018 at 4:19 AM, Dr. David Alan Gilbert > wrote: >> * Lidong Chen (jemmy858...@gmail.com) wrote: >>> RDMA migration implement save_page function for QEMUFile, but >>> ram_control_save_page do not increase bytes_xfer. So when doing >>> RDMA migration, it will use whole bandwidth. >> >> Hi, >> Thanks for this, >> >>> Signed-off-by: Lidong Chen >>> --- >>> migration/qemu-file.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c >>> index 2ab2bf3..217609d 100644 >>> --- a/migration/qemu-file.c >>> +++ b/migration/qemu-file.c >>> @@ -253,7 +253,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t >>> block_offset, >>> if (f->hooks && f->hooks->save_page) { >>> int ret = f->hooks->save_page(f, f->opaque, block_offset, >>>offset, size, bytes_sent); >>> - >>> +f->bytes_xfer += size; >> >> I'm a bit confused, because I know rdma.c calls acct_update_position() >> and I'd always thought that was enough. >> That calls qemu_update_position(...) which increases f->pos but not >> f->bytes_xfer. >> >> f_pos is used to calculate the 'transferred' value in >> migration_update_counters and thus the current bandwidth and downtime - >> but as you say, not the rate_limit. >> >> So really, should this f->bytes_xfer += size go in >> qemu_update_position ? > > For tcp migration, bytes_xfer is updated before qemu_fflush(f) which > actually send data. > but qemu_update_position is invoked by qemu_rdma_write_one, which > after call ibv_post_send. > and qemu_rdma_save_page is asynchronous, it may merge the page. > I think it's more safe to limiting rate before send data > >> >> Juan: I'm not sure I know why we have both bytes_xfer and pos. > > Maybe the reasion is bytes_xfer is updated before send data, > and bytes_xfer will be reset by migration_update_counters. > >> >> Dave >> >>> if (ret != RAM_SAVE_CONTROL_DELAYED) { >>> if (bytes_sent && *bytes_sent > 0) { >>> qemu_update_position(f, *bytes_sent); >>> -- >>> 1.8.3.1 >>> >> -- >> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 1/8] migration: stop compressing page in migration thread
* Xiao Guangrong (guangrong.x...@gmail.com) wrote: > > Hi David, > > Thanks for your review. > > On 03/15/2018 06:25 PM, Dr. David Alan Gilbert wrote: > > > > migration/ram.c | 32 > > > > Hi, > >Do you have some performance numbers to show this helps? Were those > > taken on a normal system or were they taken with one of the compression > > accelerators (which I think the compression migration was designed for)? > > Yes, i have tested it on my desktop, i7-4790 + 16G, by locally live migrate > the VM which has 8 vCPUs + 6G memory and the max-bandwidth is limited to 350. > > During the migration, a workload which has 8 threads repeatedly written total > 6G memory in the VM. Before this patchset, its bandwidth is ~25 mbps, after > applying, the bandwidth is ~50 mbps. OK, that's good - worth adding those notes to your cover letter. I wonder how well it works with compression acceleration hardware; I can't see anything in this series making it worse. > BTW, Compression will use almost all valid bandwidth after all of our work > which i will post it out part by part. Oh, that will be very nice. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 2/8] migration: stop allocating and freeing memory frequently
On 03/19/2018 06:54 PM, Dr. David Alan Gilbert wrote: +return 0; +exit: +compress_threads_load_cleanup(); I don't think this is safe; if inflateInit(..) fails in not-the-last thread, compress_threads_load_cleanup() will try and destroy all the mutex's and condition variables, even though they've not yet all been _init'd. That is exactly why we used 'opaque', please see more below... However, other than that I think the patch is OK; a chat with Dan Berrange has convinced me this probably doesn't affect the stream format, so that's OK. One thing I would like is a comment as to how the 'opaque' field is being used; I don't think I quite understand what you're doing there. The zlib.h file says that: " The opaque value provided by the application will be passed as the first parameter for calls of zalloc and zfree. This can be useful for custom memory management. The compression library attaches no meaning to the opaque value." So we can use it to store our private data. Here, we use it as a indicator which shows if the thread is properly init'd or not. If inflateInit() is successful we will set it to non-NULL, otherwise it is NULL, so that the cleanup path can figure out the first thread failed to do inflateInit(). OK, so I think you just need to add a comment to explain that. Put it above the 'if (!decomp_param[i].stream.opaque) {' in compress_threads_load_cleanup so it'll be easy to understand. Yes, indeed, i will do. Thanks!
Re: [Qemu-devel] [PULL v3 00/46] Block layer patches
On 19 March 2018 at 11:04, Kevin Wolf wrote: > The following changes since commit e1e44a9916b4318e943aecd669e096222cb3eaeb: > > Merge remote-tracking branch 'remotes/xtensa/tags/20180316-xtensa' into > staging (2018-03-17 14:15:03 +) > > are available in the git repository at: > > git://repo.or.cz/qemu/kevin.git tags/for-upstream > > for you to fetch changes up to 63ca8406beac44aa59c389ed8578d0c7b3da3402: > > iotests: Avoid realpath, for CentOS 6 (2018-03-19 12:01:39 +0100) > > > Block layer patches > Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 5/8] migration: move calling control_save_page to the common place
* Xiao Guangrong (guangrong.x...@gmail.com) wrote: > > > On 03/15/2018 07:47 PM, Dr. David Alan Gilbert wrote: > > > > /* Check the pages is dirty and if it is send it */ > > > if (migration_bitmap_clear_dirty(rs, pss->block, pss->page)) { > > > +RAMBlock *block = pss->block; > > > +ram_addr_t offset = pss->page << TARGET_PAGE_BITS; > > > + > > > +if (control_save_page(rs, block, offset, &res)) { > > > +goto page_saved; > > > > OK, but I'd prefer if you avoided this forward goto; we do use goto but > > we tend to keep it just for error cases. > > > > There is a common operation, clearing unsentmap, for save_control, > save_zero, save_compressed and save_normal, if we do not use 'goto', > the operation would to be duplicated several times or we will have > big if...elseif...elseif... section. > > So it may be not too bad to have 'goto' under this case? :) The problem is it always tends to creep a bit, and then you soon have a knot of goto's. I suggest you add a 'page_saved' bool, set it instead of taking the goto, and then add a if (!page_saved) around the next section. It doesn't need to nest for the last section; you just do another if (!page_saved) if around that. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH for-2.12] gitmodules: Use the QEMU mirror of qemu-palcode
We have a mirror of the qemu-palcode repository on git.qemu.org; use that instead of the upstream github, in line with our general policy of keeping and using a mirror for submodules. Signed-off-by: Peter Maydell --- We also currently have two submodules we don't have mirroring for: seabios-hppa and u-boot-sam460ex. .gitmodules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitmodules b/.gitmodules index b76fb450a4..c613722e3c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -18,7 +18,7 @@ url = git://git.qemu-project.org/openhackware.git [submodule "roms/qemu-palcode"] path = roms/qemu-palcode - url = git://github.com/rth7680/qemu-palcode.git + url = git://git.qemu.org/qemu-palcode.git [submodule "roms/sgabios"] path = roms/sgabios url = git://git.qemu-project.org/sgabios.git -- 2.16.2
Re: [Qemu-devel] Moving seabios-hppa git submodule to use a qemu.org mirror
On 21 February 2018 at 05:27, Jeff Cody wrote: > On Tue, Feb 20, 2018 at 06:43:22PM +, Peter Maydell wrote: >> I just noticed that we seem to have acquired another git >> submodule that isn't pointing to a qemu.org git url: >> >> [submodule "roms/seabios-hppa"] >> path = roms/seabios-hppa >> url = git://github.com/hdeller/seabios-hppa.git >> >> Jeff, could we set up so we can mirror this repo on qemu.org? >> Then we can send a patch to update the .gitmodules to point to it. >> > > Yes, of course... Although Paolo has suggested making it a branch of the > existing SeaBIOS repo instead. But I can go ahead and get the clone setup, > in case we go that route. Hi; can we get the mirror set up for seabios-hppa, and also for the other new repo: git://github.com/zbalaton/u-boot-sam460ex please? (I've just sent a patch which moves the qemu-palcode module to the already-existing mirror.) thanks -- PMM
Re: [Qemu-devel] Moving seabios-hppa git submodule to use a qemu.org mirror
On Mon, 19 Mar 2018, Peter Maydell wrote: On 21 February 2018 at 05:27, Jeff Cody wrote: On Tue, Feb 20, 2018 at 06:43:22PM +, Peter Maydell wrote: I just noticed that we seem to have acquired another git submodule that isn't pointing to a qemu.org git url: [submodule "roms/seabios-hppa"] path = roms/seabios-hppa url = git://github.com/hdeller/seabios-hppa.git Jeff, could we set up so we can mirror this repo on qemu.org? Then we can send a patch to update the .gitmodules to point to it. Yes, of course... Although Paolo has suggested making it a branch of the existing SeaBIOS repo instead. But I can go ahead and get the clone setup, in case we go that route. Hi; can we get the mirror set up for seabios-hppa, and also for the other new repo: git://github.com/zbalaton/u-boot-sam460ex I'd suggest to move the u-boot-sam460ex repo to qemu repo completely instead of mirrorring it. I don't intend to change it in the future (apart from maybe occasional bugfixes if needed which chould be submitted as patches) so no need to keep an external repo and mirror it when it could be held in qemu as primary. Please let me know how you decide, I can then remove the the above repo and refer to the qemu one in the future if you move it there or keep the current one up if you choose to mirror it. Regards, BALATON Zoltan
[Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
Bug fix: checkpatch.pl stops complaining about following pattern: """ do { //do somethins; } while (conditions); """ Signed-off-by: Su Hang --- scripts/checkpatch.pl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d1fe79bcc47c..2ca833f22e5a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2355,6 +2355,18 @@ sub process { # check for missing bracing around if etc if ($line =~ /(^.*)\b(?:if|while|for)\b/ && $line !~ /\#\s*if/) { + my $allowed = 0; + + # Check the pre-context. + if ($line =~ /(\}.*?)$/) { + my $pre = $1; + + if ($line !~ /else/) { + print "APW: ALLOWED: pre<$pre> line<$line>\n" + if $dbg_adv_apw; + $allowed = 1; + } + } my ($level, $endln, @chunks) = ctx_statement_full($linenr, $realcnt, 1); if ($dbg_adv_apw) { @@ -2363,7 +2375,6 @@ sub process { if $#chunks >= 1; } if ($#chunks >= 0 && $level == 0) { - my $allowed = 0; my $seen = 0; my $herectx = $here . "\n"; my $ln = $linenr - 1; @@ -2407,7 +2418,7 @@ sub process { $allowed = 1; } } - if ($seen != ($#chunks + 1)) { + if ($seen != ($#chunks + 1) && !$allowed) { ERROR("braces {} are necessary for all arms of this statement\n" . $herectx); } } -- 2.7.4
[Qemu-devel] [PATCH v2] target-mips: Add initrd support for the Boston board
From: Aleksandar Rikalo Add support for initial ramdisk loading for the Mips Boston board. Signed-off-by: Aleksandar Rikalo Reviewed-by: Philippe Mathieu-Daudé --- Changes since previous version according to Philippe's comments: - 'long inird_size' is changed to 'target_ulong initrd_size', as it should be - error_report() is used instead of fprintf() hw/mips/boston.c | 54 +- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/hw/mips/boston.c b/hw/mips/boston.c index fb23161..6960a60 100644 --- a/hw/mips/boston.c +++ b/hw/mips/boston.c @@ -30,6 +30,7 @@ #include "hw/loader-fit.h" #include "hw/mips/cps.h" #include "hw/mips/cpudevs.h" +#include "hw/mips/mips.h" #include "hw/pci-host/xilinx-pcie.h" #include "qapi/error.h" #include "qemu/cutils.h" @@ -333,10 +334,12 @@ static const void *boston_fdt_filter(void *opaque, const void *fdt_orig, { BostonState *s = BOSTON(opaque); MachineState *machine = s->mach; -const char *cmdline; +GString *cmdline; int err; void *fdt; size_t fdt_sz, ram_low_sz, ram_high_sz; +target_ulong initrd_size; +ram_addr_t initrd_offset; fdt_sz = fdt_totalsize(fdt_orig) * 2; fdt = g_malloc0(fdt_sz); @@ -347,20 +350,53 @@ static const void *boston_fdt_filter(void *opaque, const void *fdt_orig, return NULL; } -cmdline = (machine->kernel_cmdline && machine->kernel_cmdline[0]) -? machine->kernel_cmdline : " "; -err = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline); -if (err < 0) { -fprintf(stderr, "couldn't set /chosen/bootargs\n"); -return NULL; -} - ram_low_sz = MIN(256 * M_BYTE, machine->ram_size); ram_high_sz = machine->ram_size - ram_low_sz; qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg", 1, 0x, 1, ram_low_sz, 1, 0x9000, 1, ram_high_sz); +cmdline = g_string_new(machine->kernel_cmdline); + +/* load initrd */ +initrd_offset = 0; +if (machine->initrd_filename) { +initrd_size = get_image_size(machine->initrd_filename); +if (initrd_size != (target_ulong) -1) { +/* The kernel allocates the bootmap memory in the low memory after + the initrd. It takes at most 128kiB for 2GB RAM and 4kiB + pages. */ +initrd_offset = (ram_low_sz - initrd_size - 131072 + - ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK; + +if ((int64_t)cpu_mips_kseg0_to_phys(NULL, *load_addr + fdt_sz) +>= (int64_t)initrd_offset) { +error_report("memory too small for initial ram disk '%s'", + machine->initrd_filename); +exit(1); +} + +initrd_size = load_image_targphys(machine->initrd_filename, + initrd_offset, + initrd_size); +} +if (initrd_size == (target_ulong) -1) { +error_report("could not load initial ram disk '%s'", + machine->initrd_filename); +exit(1); +} +g_string_append_printf(cmdline, " rd_start=0x%" PRIx64 " rd_size=%li", + cpu_mips_phys_to_kseg0(NULL, initrd_offset), + initrd_size); +} + +err = qemu_fdt_setprop_string(fdt, "/chosen", "bootargs", cmdline->str); +g_string_free(cmdline, true); +if (err < 0) { +error_report("couldn't set /chosen/bootargs"); +exit(1); +} + fdt = g_realloc(fdt, fdt_totalsize(fdt)); qemu_fdt_dumpdtb(fdt, fdt_sz); -- 1.9.1
[Qemu-devel] [PATCH for-2.12] hw/misc/macio: Fix crash when listing device properties of macio device
The macio-newworld device can currently be used to abort QEMU unexpectedly: $ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped) qdev properties should be set during realize(), not during instance_init(), so move the related code there to fix this problem. Signed-off-by: Thomas Huth --- hw/misc/macio/macio.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c index 454244f..b74a657 100644 --- a/hw/misc/macio/macio.c +++ b/hw/misc/macio/macio.c @@ -115,6 +115,13 @@ static void macio_common_realize(PCIDevice *d, Error **errp) memory_region_add_subregion(&s->bar, 0x16000, sysbus_mmio_get_region(sysbus_dev, 0)); +qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0); +qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK); +qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4); +qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hds[0]); +qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hds[1]); +qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial); +qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial); object_property_set_bool(OBJECT(&s->escc), true, "realized", &err); if (err) { error_propagate(errp, err); @@ -341,13 +348,6 @@ static void macio_instance_init(Object *obj) object_property_add_child(obj, "dbdma", OBJECT(&s->dbdma), NULL); object_initialize(&s->escc, sizeof(s->escc), TYPE_ESCC); -qdev_prop_set_uint32(DEVICE(&s->escc), "disabled", 0); -qdev_prop_set_uint32(DEVICE(&s->escc), "frequency", ESCC_CLOCK); -qdev_prop_set_uint32(DEVICE(&s->escc), "it_shift", 4); -qdev_prop_set_chr(DEVICE(&s->escc), "chrA", serial_hds[0]); -qdev_prop_set_chr(DEVICE(&s->escc), "chrB", serial_hds[1]); -qdev_prop_set_uint32(DEVICE(&s->escc), "chnBtype", escc_serial); -qdev_prop_set_uint32(DEVICE(&s->escc), "chnAtype", escc_serial); qdev_set_parent_bus(DEVICE(&s->escc), sysbus_get_default()); object_property_add_child(obj, "escc", OBJECT(&s->escc), NULL); } -- 1.8.3.1
[Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR
From: Qin Chao Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit 8, which is BSP flag and should be set to 1 for the bootstrap processor and set to 0 for the application processors, but it's set to 0 for all processors in HAXM. So guest OSes that expect a valid BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot boot with "-accel hax". To solve this problem, HAXM (which lacks APIC virtualization) and QEMU must notify each other of any change to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into HAXM source. QEMU needs to use the new HAXM API (apic_base in "struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR, and then, update its own copy at every return from HAX_VCPU_IOCTL_RUN. There will be a backward compatility issue caused by the new field "apic_base" added into "struct hax_tunnel". In order to fix the problem, the validation for size of "struct hax_tunnel" is removed and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which means that one page (4KB) is allocated in HAXM kernel to store "struct hax_tunnel", instead of the size of "struct hax_tunnel". Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3 Signed-off-by: Qin Chao --- target/i386/hax-all.c | 24 +++- target/i386/hax-darwin.c| 6 -- target/i386/hax-i386.h | 2 +- target/i386/hax-interface.h | 3 +++ target/i386/hax-windows.c | 5 - 5 files changed, 23 insertions(+), 17 deletions(-) diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c index cad7531..6a840d9 100644 --- a/target/i386/hax-all.c +++ b/target/i386/hax-all.c @@ -62,11 +62,6 @@ int hax_enabled(void) return hax_allowed; } -int valid_hax_tunnel_size(uint16_t size) -{ -return size >= sizeof(struct hax_tunnel); -} - hax_fd hax_vcpu_get_fd(CPUArchState *env) { struct hax_vcpu_state *vcpu = ENV_GET_CPU(env)->hax_vcpu; @@ -104,6 +99,7 @@ static int hax_get_capability(struct hax_state *hax) } hax->supports_64bit_ramblock = !!(cap->winfo & HAX_CAP_64BIT_RAMBLOCK); +hax->supports_tunnel_page = !!(cap->winfo & HAX_CAP_TUNNEL_PAGE); if (cap->wstatus & HAX_CAP_MEMQUOTA) { if (cap->mem_quota < hax->mem_quota) { @@ -520,6 +516,21 @@ static int hax_vcpu_hax_exec(CPUArchState *env) cpu_exec_end(cpu); qemu_mutex_lock_iothread(); +/* + * Every time HAXM exits to QEMU, sync IA32_APIC_BASE MSR from HAXM and + * pass it to the emulated APIC. + */ +if (hax_global.supports_tunnel_page) { +/* + * ht->apic_base is not available in HAXM kernel module if HAXM does + * not support HAX_CAP_SUPPORT_TUNNEL_PAGE. + * TODO: HAX_CAP_SUPPORT_TUNNEL_PAGE is used for backward + * compatibility with HAXM kernel module. Remove this check when we + * drop support for HAXM versions that lack this feature. + */ +cpu_set_apic_base(x86_cpu->apic_state, ht->apic_base); +} + /* Simply continue the vcpu_run if system call interrupted */ if (hax_ret == -EINTR || hax_ret == -EAGAIN) { DPRINTF("io window interrupted\n"); @@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase); #endif +hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \ + cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); + md.nr_msr = n; md.done = 0; diff --git a/target/i386/hax-darwin.c b/target/i386/hax-darwin.c index acdde47..3e2fd4f 100644 --- a/target/i386/hax-darwin.c +++ b/target/i386/hax-darwin.c @@ -244,12 +244,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state *vcpu) return ret; } -if (!valid_hax_tunnel_size(info.size)) { -fprintf(stderr, "Invalid hax tunnel size %x\n", info.size); -ret = -EINVAL; -return ret; -} - vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va); vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va); return 0; diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h index 6abc156..b04bf24 100644 --- a/target/i386/hax-i386.h +++ b/target/i386/hax-i386.h @@ -38,6 +38,7 @@ struct hax_state { struct hax_vm *vm; uint64_t mem_quota; bool supports_64bit_ramblock; +bool supports_tunnel_page; }; #define HAX_MAX_VCPU 0x10 @@ -53,7 +54,6 @@ struct hax_vm { #ifdef NEED_CPU_H /* Functions exported to host specific mode */ hax_fd hax_vcpu_get_fd(CPUArchState *env); -int valid_hax_tunnel_size(uint16_t size); /* Host specific functions */ int hax_mod_version(struct hax_state *hax, struct hax_module_version *version); diff --git a/target/i386/hax-interface.h b/target/i386/hax-interface.h index 93d5fcb..715a64a 100644 --- a/target/i386/hax-interface.h +++ b/target/i386/hax-interface.h @@ -280,6 +280,7 @@ s
[Qemu-devel] [Bug 1756807] Re: performance regression in qemu-user + proot
Thanks for the check Alistar, Lets add a Qemu (upstream) bug task so this one is mirrored to the ML. I'm not familiar with that area, but on the ML one can decide if it is a dup to https://bugs.launchpad.net/qemu/+bug/1740219 or not. ** Also affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1756807 Title: performance regression in qemu-user + proot Status in QEMU: New Status in qemu package in Ubuntu: New Bug description: To reproduce: 1. Install qemu-user-static and proot 2. Enter some arm chroot using them: proot -0 -q qemu-arm-static -w / -r chroot/ /bin/bash 3. Run a command which normally takes a short but measurable amount of time: cd /usr/share/doc && time grep -R hello Result: This is over 100 times slower on 18.04 than it was on 16.04. I am not sure if proot or qemu is causing the problem, but proot version has not changed. Also note that on 16.04 I am using the cloud repo version of qemu, which is newer than 16.04 stock, but still older than 18.04. Although system 2 is lower spec than system 1, it should not be this much slower. No other software seems to be affected. If required I can supply a chroot tarball. It is essentially just a Debian bootstrap though. Logs: System 1: i7-6700 3.4GHz, 32GB RAM, 512GB Crucial MX100 SSD, Ubuntu 16.04 qemu-arm version 2.10.1(Debian 1:2.10+dfsg-0ubuntu3.4~cloud0) proot 5.1.0 al@al-desktop:~/rpi-ramdisk/raspbian$ proot -0 -q qemu-arm-static -w / -r root/ /bin/bash root@al-desktop:/# cd /usr/share/doc root@al-desktop:/usr/share/doc# time grep -R hello dash/copyright:Debian GNU/Linux hello source package as the file COPYING. If not, real 0m0.066s user 0m0.040s sys 0m0.008s System 2: i5-5300U 2.30GHz, 8GB RAM, 256GB Crucial MX300 SSD, Ubuntu 18.04 qemu-arm version 2.11.1(Debian 1:2.11+dfsg-1ubuntu4) proot 5.1.0 al@al-thinkpad:~/rpi-ramdisk/raspbian$ proot -0 -q qemu-arm-static -w / -r root/ /bin/bash root@al-thinkpad:/# cd /usr/share/doc root@al-thinkpad:/usr/share/doc# time grep -R hello dash/copyright:Debian GNU/Linux hello source package as the file COPYING. If not, real 0m24.176s user 0m0.366s sys 0m11.352s ProblemType: Bug DistroRelease: Ubuntu 18.04 Package: qemu (not installed) ProcVersionSignature: Ubuntu 4.15.0-12.13-generic 4.15.7 Uname: Linux 4.15.0-12-generic x86_64 ApportVersion: 2.20.8-0ubuntu10 Architecture: amd64 Date: Mon Mar 19 07:13:25 2018 InstallationDate: Installed on 2018-03-18 (0 days ago) InstallationMedia: Xubuntu 18.04 LTS "Bionic Beaver" - Alpha amd64 (20180318) SourcePackage: qemu UpgradeStatus: No upgrade log present (probably fresh install) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1756807/+subscriptions
[Qemu-devel] [PATCH 2/2] arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR
It should skip to getting/putting the registers banked by GICR so that it could get/put the correct ones. Signed-off-by: Shannon Zhao --- hw/intc/arm_gicv3_kvm.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index 7000716..e967e4f 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -136,6 +136,8 @@ static void kvm_dist_get_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) int irq; field = (uint32_t *)bmp; +/* The first 8 GICD_IPRIORITYR should skip. */ +offset += (8 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 8) { kvm_gicd_access(s, offset, ®, false); *field = reg; @@ -150,6 +152,8 @@ static void kvm_dist_put_priority(GICv3State *s, uint32_t offset, uint8_t *bmp) int irq; field = (uint32_t *)bmp; +/* The first 8 GICD_IPRIORITYR should skip. */ +offset += (8 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 8) { reg = *field; kvm_gicd_access(s, offset, ®, true); @@ -164,6 +168,8 @@ static void kvm_dist_get_edge_trigger(GICv3State *s, uint32_t offset, uint32_t reg; int irq; +/* The first 2 GICD_ICFGR should skip. */ +offset += (2 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 2) { kvm_gicd_access(s, offset, ®, false); reg = half_unshuffle32(reg >> 1); @@ -181,6 +187,8 @@ static void kvm_dist_put_edge_trigger(GICv3State *s, uint32_t offset, uint32_t reg; int irq; +/* The first 2 GICD_ICFGR should skip. */ +offset += (2 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 2) { reg = *gic_bmp_ptr32(bmp, irq); if (irq % 32 != 0) { @@ -222,6 +230,8 @@ static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp) uint32_t reg; int irq; +/* The first 1 GICD_ should skip. */ +offset += (1 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 1) { kvm_gicd_access(s, offset, ®, false); *gic_bmp_ptr32(bmp, irq) = reg; @@ -236,6 +246,10 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, int irq; clroffset_index = clroffset; +/* The first 1 GICD_ should skip. */ +offset += (1 * sizeof(uint32_t)); +if (clroffset != 0) +clroffset_index += (1 * sizeof(uint32_t)); for_each_dist_irq_reg(irq, s->num_irq, 1) { /* If this bitmap is a set/clear register pair, first write to the * clear-reg to clear all bits before using the set-reg to write -- 2.0.4
[Qemu-devel] [PATCH 1/2] arm_gicv3_kvm: increase clroffset accordingly
It forgot to increase clroffset during the loop. So it only clear the first 4 bytes. Signed-off-by: Shannon Zhao --- hw/intc/arm_gicv3_kvm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index ec37177..7000716 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -232,9 +232,10 @@ static void kvm_dist_getbmp(GICv3State *s, uint32_t offset, uint32_t *bmp) static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, uint32_t clroffset, uint32_t *bmp) { -uint32_t reg; +uint32_t reg, clroffset_index; int irq; +clroffset_index = clroffset; for_each_dist_irq_reg(irq, s->num_irq, 1) { /* If this bitmap is a set/clear register pair, first write to the * clear-reg to clear all bits before using the set-reg to write @@ -242,7 +243,8 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, */ if (clroffset != 0) { reg = 0; -kvm_gicd_access(s, clroffset, ®, true); +kvm_gicd_access(s, clroffset_index, ®, true); +clroffset_index += 4; } reg = *gic_bmp_ptr32(bmp, irq); kvm_gicd_access(s, offset, ®, true); -- 2.0.4
[Qemu-devel] [PATCH 0/2] two fixes for KVM GICv3 dist get/put functions
Shannon Zhao (2): arm_gicv3_kvm: increase clroffset accordingly arm_gicv3_kvm: kvm_dist_get/put: skip the registers banked by GICR hw/intc/arm_gicv3_kvm.c | 20 ++-- 1 file changed, 18 insertions(+), 2 deletions(-) -- 2.0.4
Re: [Qemu-devel] [PATCH 8/9] hw/arm/bcm2836: Hardcode correct CPU type
On Thu, 15 Mar 2018 17:13:03 + Peter Maydell wrote: > On 13 March 2018 at 16:55, Andrew Baumann > wrote: > >> From: Qemu-devel >> bounces+andrew.baumann=microsoft@nongnu.org> On Behalf Of Peter > >> Maydell > >> Sent: Tuesday, 13 March 2018 08:35 > >> > >> Now we have separate types for BCM2386 and BCM2387, we might as well > >> just hard-code the CPU type they use rather than having it passed > >> through as an object property. This then lets us put the initialization > >> of the CPU object in init rather than realize. > > > What about the default_cpu_type field of MachineClass set in > > raspi[23]_machine_init? That seems irrelevant now... > > Igor, can you help with this question? For a board that always > has one CPU type (because the real hardware only ever has > one SoC, and that SoC QOM object hard codes the CPU type) > does it still need to set the mc->default_cpu_type field in > its MachineClass, or does that become pointless ? Since board ignores whatever were specified on '-cpu' and there aren't any checks in board code for current_machine->cpu_type, removing mc->default_cpu_type won't really affect anything. With current code in vl.c and with default_cpu_type set, user will get error if he specified non existing cpu_model with -cpu. If default_cpu_type were NULL, -cpu is completely ignored. But once http://patchwork.ozlabs.org/patch/870297/ is applied it will error out the same way as if default_cpu_type were set since vl.c won't rely on it anymore for parsing cpu_model. So in short it's ok to remove mc->default_cpu_type here. > > thanks > -- PMM
Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi
On 03/16/2018 09:40 AM, Nia Alarie wrote: If qemu_strtoi indicates an error, return the default value. Would it be better to diagnose the error instead of silently returning a default value? Signed-off-by: Nia Alarie --- audio/audio.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index 6eccdb17ee..d6e91901aa 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int defval, int *defaultp) char *strval; strval = getenv (key); -if (strval) { +if (strval && !qemu_strtoi(strval, NULL, 10, &val)) { *defaultp = 0; -val = atoi (strval); return val; } else { -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PULL 00/38] QAPI patches for 2018-03-12, 2.12 softfreeze
On 03/19/2018 04:26 AM, Peter Xu wrote: for you to fetch changes up to 75eb57e3ed3682f011a6694863044e8b143a9821: qapi: Pass '-u' when doing non-silent diff (2018-03-16 09:00:07 -0500) Hi. I get a bunch of test assertion failures with this: ppc64 host: QTEST_QEMU_BINARY=nios2-softmmu/qemu-system-nios2 QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1) )} gtester -k --verbose -m=quick tests/qmp-test tests/device-introspect-test tests/qom-test tests/test-hmp TEST: tests/qmp-test... (pid=49431) /nios2/qmp/protocol: OK /nios2/qmp/oob: OK /nios2/qmp/query-status: OK /nios2/qmp/query-block: OK /nios2/qmp/query-blockstats: OK /nios2/qmp/query-block-jobs: OK /nios2/qmp/query-named-block-nodes: qemu-system-nios2: /home/pm215/qemu/chardev/char-io.c:91: io_watc h_poll_finalize: Assertion `iwp->src == ((void *)0)' failed. Broken pipe FAIL I haven't been able to reproduce the testsuite failures on my Linux box, but if it's a race, then that doesn't make me all the more confident on what it takes to reproduce and/or fix the race. One or two of the build hosts did pass, so that plus the varying tests which failed suggests that the iwp->src assert is an intermittent or timing based one. The tpm error on NetBSD is probably a separate issue. I think I still need this to be squashed into "monitor: allow using IO thread for parsing", which I dropped during respin from v7 to v8: diff --git a/monitor.c b/monitor.c index f9ef3e5266..121194111f 100644 --- a/monitor.c +++ b/monitor.c @@ -4556,6 +4556,11 @@ void monitor_init(Chardev *chr, int flags) qemu_chr_fe_set_echo(&mon->chr, true); json_message_parser_init(&mon->qmp.parser, handle_qmp_command); if (mon->use_io_thr) { +/* + * Make sure the old iowatch be gone. It's possible when + * e.g. the chardev is in client mode, with wait=on. + */ +remove_fd_in_watch(chr); /* * We can't call qemu_chr_fe_set_handlers() directly here * since during the procedure the chardev will be active I thought there should be no pending task on main thread after the QIO and CHARDEV fixes, but I missed the most general io watch and we still possibly need the line. So, should I squash in the fix and keep OOB as part of my v3 attempt, or are we getting close enough to rc0 that my qapi v3 pull request should just drop OOB, and save that as a feature for 2.13 instead? We should fix the assertion problem with above, but not sure about whether it can fix the QIO issue since I haven't seen that before (and I can't reproduce that too in my environment). I hope the fix can work for us. But in all cases, please feel free to drop the series if needed. Sorry for the trouble. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH for-2.12] hw/char/serial: Fix crash when serial_mm_init() is used with -nodefaults
Quite a lot of boards call serial_mm_init() directly with a value from the serial_hds[] table. However, this table is only containing NULL if QEMU has been started with "-nodefaults": $ gdb --args arm-softmmu/qemu-system-arm -S -nodefaults -M cubieboard (gdb) r Program received signal SIGSEGV, Segmentation fault. qemu_chr_fe_init (b=b@entry=0x56cc4260, s=s@entry=0x0, errp=0x5697eb40 ) at chardev/char-fe.c:210 210 } else if (s->be) { Since calling serial_mm_init with a NULL pointer seems to be a common pattern between many boards, let's simply fix this by creating a "null" chardev on the fly in this case. Signed-off-by: Thomas Huth --- hw/char/serial.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/char/serial.c b/hw/char/serial.c index eb72191..c1f7ff7 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -1040,6 +1040,10 @@ SerialState *serial_mm_init(MemoryRegion *address_space, { SerialState *s; +if (!chr) { +chr = qemu_chardev_new(NULL, TYPE_CHARDEV_NULL, NULL, &error_abort); +} + s = g_malloc0(sizeof(SerialState)); s->it_shift = it_shift; -- 1.8.3.1
Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi
On Mon, Mar 19, 2018 at 2:47 PM, Eric Blake wrote: > On 03/16/2018 09:40 AM, Nia Alarie wrote: >> >> If qemu_strtoi indicates an error, return the default value. > > > Would it be better to diagnose the error instead of silently returning a > default value? > >> >> Signed-off-by: Nia Alarie >> --- >> audio/audio.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/audio/audio.c b/audio/audio.c >> index 6eccdb17ee..d6e91901aa 100644 >> --- a/audio/audio.c >> +++ b/audio/audio.c >> @@ -335,9 +335,8 @@ static int audio_get_conf_int (const char *key, int >> defval, int *defaultp) >> char *strval; >> strval = getenv (key); >> -if (strval) { >> +if (strval && !qemu_strtoi(strval, NULL, 10, &val)) { >> *defaultp = 0; >> -val = atoi (strval); >> return val; >> } >> else { >> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org Possibly, while writing these patches I was just going by what was already there. I can see how that would be good. Should the code provide a warning to the user and continue with the default, or provide the warning and exit? And is it more correct to use dolog() or AUD_log() in this context?
Re: [Qemu-devel] [PULL v2 0/7] Machine queue, 2018-03-15
On Fri, 16 Mar 2018 16:28:54 -0300 Eduardo Habkost wrote: > On Fri, Mar 16, 2018 at 07:05:29PM +, Peter Maydell wrote: > > On 15 March 2018 at 18:14, Eduardo Habkost wrote: > > > Changes in v2 (v1 was 2018-03-12): > > > * Fix bsd-user build error > > > > > > The following changes since commit > > > 56e8698ffa8aba9f762f980bc21b5340b006f24b: > > > > > > Merge remote-tracking branch > > > 'remotes/stsquad/tags/pull-travis-speedup-130318-1' into staging > > > (2018-03-15 14:48:09 +) > > > > > > are available in the Git repository at: > > > > > > git://github.com/ehabkost/qemu.git tags/machine-next-pull-request > > > > > > for you to fetch changes up to 7cbb6fd8926b9b590c0725b9b7d0a11db6aefd08: > > > > > > cpu: drop unnecessary NULL check and cpu_common_class_by_name() > > > (2018-03-15 14:52:40 -0300) > > > > > > > > > Machine queue, 2018-03-15 > > > > > > > Hi. This produces several warning messages running make check: > > > > WARNING: cpu name for target 'riscv32' isn't defined, add it to cpus_map > > WARNING: cpu name for target 'riscv64' isn't defined, add it to cpus_map > > Ouch, another conflict with the commits that added target/riscv > after the original series was submitted. :( > > I will drop all the patches in this pull request except for the > only bug fix there ("pc: correct misspelled CPU model-id for pc > 2.2"). > > Igor, can you resubmit the cpu_model/cpu_type series fixing the > warnings so I can queue it for v2.13? Fixup should be trivial, I'll post it here as reply to offending patch. This kind of tree wide changes tend to break often if not merged quickly and leaving old infrastructure around till 2.13 doesn't look like good idea as someone will copy it and queued for 2.13 tree well be broken again. So I'd prefer if you'd send fixed up pull request instead of dropping patches.
Re: [Qemu-devel] [PATCH] audio: Convert use of atoi to qemu_strtoi
On 03/19/2018 10:01 AM, nee wrote: On Mon, Mar 19, 2018 at 2:47 PM, Eric Blake wrote: On 03/16/2018 09:40 AM, Nia Alarie wrote: If qemu_strtoi indicates an error, return the default value. Would it be better to diagnose the error instead of silently returning a default value? Possibly, while writing these patches I was just going by what was already there. I can see how that would be good. Should the code provide a warning to the user and continue with the default, or provide the warning and exit? And is it more correct to use dolog() or AUD_log() in this context? I'll defer to the audio maintainer's opinion on what might be best here. But my personal preference is that if the only time you can give invalid input is via a bad command line argument, then print the error and exit immediately (the VM never starts), so that the user can then fix their bad command line and get the value they wanted instead of silently running with a different value all because of a typo that caused us to fail to parse a number. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH 0/2] vhost-user-scsi: add message for device reset
When a virtio scsi device is reset by a guest, the reset is not passed on to the vhost-user backend. This reset may be necessary if a device driver is restarted or the device is handed off between (for example) SeaBIOS and the OS. Iff the vhost-user-scsi backend reports that it supports a new "reset device" feature, QEMU will send a VHOST_USER_RESET_DEVICE message when the guest resets the virtio device. Existing backends are unaffected. David
[Qemu-devel] [PATCH 1/2] vhost-user: add VHOST_USER_RESET_DEVICE to reset devices
Add a VHOST_USER_RESET_DEVICE message which will reset the vhost user backend. Disabling all rings, and resetting all internal state, ready for the backend to be reinitialized. A backend has to report it supports this features with the VHOST_USER_PROTOCOL_F_RESET_DEVICE protocol feature bit. If it does so, the new message is used instead of sending a RESET_OWNER which has had inconsistent implementations. Signed-off-by: David Vrabel --- docs/interop/vhost-user.txt | 16 hw/virtio/vhost-user.c | 8 +++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/interop/vhost-user.txt b/docs/interop/vhost-user.txt index cb3a7595aa..c5faa9fff8 100644 --- a/docs/interop/vhost-user.txt +++ b/docs/interop/vhost-user.txt @@ -369,6 +369,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_SLAVE_REQ 5 #define VHOST_USER_PROTOCOL_F_CROSS_ENDIAN 6 #define VHOST_USER_PROTOCOL_F_CRYPTO_SESSION 7 +#define VHOST_USER_PROTOCOL_F_RESET_DEVICE 8 Master message types @@ -689,6 +690,21 @@ Master message types feature has been successfully negotiated. It's a required feature for crypto devices. + * VHOST_USER_RESET_DEVICE + + Id: 28 + Equivalent ioctl: N/A + Master payload: N/A + Slave payload: N/A + + Ask the vhost user backend to disable all rings and reset all + internal device state to the initial state, ready to be + reinitialized. The backend retains ownership of the device + throughout the reset operation. + + Only valid if the VHOST_USER_PROTOCOL_F_RESET_DEVICE protocol + feature is set by the backend. + Slave message types --- diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 41ff5cff41..67207bfe8a 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -41,6 +41,7 @@ enum VhostUserProtocolFeature { VHOST_USER_PROTOCOL_F_SLAVE_REQ = 5, VHOST_USER_PROTOCOL_F_CROSS_ENDIAN = 6, VHOST_USER_PROTOCOL_F_CRYPTO_SESSION = 7, +VHOST_USER_PROTOCOL_F_RESET_DEVICE = 8, VHOST_USER_PROTOCOL_F_MAX }; @@ -76,6 +77,7 @@ typedef enum VhostUserRequest { VHOST_USER_SET_CONFIG = 25, VHOST_USER_CREATE_CRYPTO_SESSION = 26, VHOST_USER_CLOSE_CRYPTO_SESSION = 27, +VHOST_USER_RESET_DEVICE = 28, VHOST_USER_MAX } VhostUserRequest; @@ -641,10 +643,14 @@ static int vhost_user_set_owner(struct vhost_dev *dev) static int vhost_user_reset_device(struct vhost_dev *dev) { VhostUserMsg msg = { -.hdr.request = VHOST_USER_RESET_OWNER, .hdr.flags = VHOST_USER_VERSION, }; +msg.hdr.request = virtio_has_feature(dev->protocol_features, + VHOST_USER_PROTOCOL_F_RESET_DEVICE) +? VHOST_USER_RESET_DEVICE +: VHOST_USER_RESET_OWNER; + if (vhost_user_write(dev, &msg, NULL, 0) < 0) { return -1; } -- 2.11.0
[Qemu-devel] [PATCH 2/2] vhost-user-scsi: reset the device if supported
If the vhost-user-scsi backend supports the VHOST_USER_F_RESET_DEVICE protocol feature, then the device can be reset when requested. If this feature is not supported, do not try a reset as this will send a VHOST_USER_RESET_OWNER that the backend is not expecting, potentially putting into an inoperable state. Signed-off-by: David Vrabel --- hw/scsi/vhost-user-scsi.c | 24 1 file changed, 24 insertions(+) diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c index 9389ed48e0..15e2dabebb 100644 --- a/hw/scsi/vhost-user-scsi.c +++ b/hw/scsi/vhost-user-scsi.c @@ -37,6 +37,10 @@ static const int user_feature_bits[] = { VHOST_INVALID_FEATURE_BIT }; +enum VhostUserProtocolFeature { +VHOST_USER_PROTOCOL_F_RESET_DEVICE = 8, +}; + static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) { VHostUserSCSI *s = (VHostUserSCSI *)vdev; @@ -60,6 +64,25 @@ static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status) } } +static void vhost_user_scsi_reset(VirtIODevice *vdev) +{ +VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev); +struct vhost_dev *dev = &vsc->dev; + +/* + * Historically, reset was not implemented so only reset devices + * that are expecting it. + */ +if (!virtio_has_feature(dev->protocol_features, +VHOST_USER_PROTOCOL_F_RESET_DEVICE)) { +return; +} + +if (dev->vhost_ops->vhost_reset_device) { +dev->vhost_ops->vhost_reset_device(dev); +} +} + static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq) { } @@ -172,6 +195,7 @@ static void vhost_user_scsi_class_init(ObjectClass *klass, void *data) vdc->get_features = vhost_user_scsi_get_features; vdc->set_config = vhost_scsi_common_set_config; vdc->set_status = vhost_user_scsi_set_status; +vdc->reset = vhost_user_scsi_reset; fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path; } -- 2.11.0
Re: [Qemu-devel] qapi escape-too-big test doesn't work if LANG=C ?
On 03/19/2018 06:20 AM, Daniel P. Berrangé wrote: On Mon, Mar 19, 2018 at 10:37:12AM +, Peter Maydell wrote: I recently tweaked my build scripts to run with LANG=C (trying to suppress gcc's irritating habit of using smartquotes rather than plain old ''). This seems to result in an error running the qapi-schema/escape-too-big test: PYTHONPATH=/home/petmay01/linaro/qemu-for-merges/scripts python3 -B /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/test-qapi.py /home/petmay01/linaro/qemu-for-merges/tests/qapi-schema/escape-too-big.json tests/qapi-schema/escape-too-big.test.out 2>tests/qapi-schema/escape-too-big.test.err; echo $? tests/qapi-schema/escape-too-big.test.exit 1c1,10 < tests/qapi-schema/escape-too-big.json:3:14: For now, \u escape only supports non-zero values up to \u007f --- Traceback (most recent call last): File "tests/qapi-schema/test-qapi.py", line 64, in schema = QAPISchema(sys.argv[1]) File "scripts/qapi/common.py", line 1492, in __init__ parser = QAPISchemaParser(open(fname, 'r')) File "scripts/qapi/common.py", line 264, in __init__ self.src = fp.read() File "/usr/lib/python3.5/encodings/ascii.py", line 26, in decode return codecs.ascii_decode(input, self.errors)[0] UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 61: ordinal not in range(128) /home/petmay01/linaro/qemu-for-merges/tests/Makefile.include:927: recipe for target 'check-tests/qapi-schema/escape-too-big.json' failed So your "C" locale will be non-UTF-8, except on OS-X where the "C" locale is UTF-8 by default. Unfortunately while POSIX expects the "C" locale to be 8-bit cleanup, Python by default will reject any characters outside the 7-bit range with its "ascii" codec. So this is ultimately a python bug, but there's little we can do about that given how widely deployed the bug is. Yep, the bug is in python. And we've already worked around it elsewhere in qemu; see commit d4e5ec877. To workaround this problem in other applications what I have done is add the following to Makefiles before invoking python: LC_ALL= LANG=C LC_CTYPE=en_US.UTF-8 The LC_ALL= bit is needed because if the user has set LC_ALL themselves it will override LANG and all other LC_* variables. Setting LANG=C is not strictly needed, as LC_CTYPE will override it. CC'ing Eric since he was involved in the discussions about this bug in other libvirt related apps. So the question is how to fix the test to not trip the python bugs. And the only non-7-bit-clean byte in that file is in a comment, so maybe the simplest stupid thing that would work for now is to rewrite the comment to drop the é and instead use pure ASCII. A nicer fix would be teaching scripts/qapi/common.py to read input in an 8-bit-clean manner regardless of locale, but that's more invasive (and I'd rather have Markus' help on that front), so for 2.12, I'll just do the obvious bug fix of avoiding the problem by skipping the unfortunate comment. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v9 13/14] hw/arm/virt-acpi-build: Add smmuv3 node in IORT table
On 2018/3/12 20:48, Peter Maydell wrote: > On 17 February 2018 at 18:46, Eric Auger wrote: >> From: Prem Mallappa >> >> This patch builds the smmuv3 node in the ACPI IORT table. >> >> The RID space of the root complex, which spans 0x0-0x1 >> maps to streamid space 0x0-0x1 in smmuv3, which in turn >> maps to deviceid space 0x0-0x1 in the ITS group. >> >> The guest must feature the IOMMU probe deferral series >> (https://lkml.org/lkml/2017/4/10/214) which fixes streamid >> multiple lookup. This bug is not related to the SMMU emulation. >> >> Signed-off-by: Prem Mallappa >> Signed-off-by: Eric Auger >> >> --- >> >> v2 -> v3: >> - integrate into the existing IORT table made up of ITS, RC nodes >> - take into account vms->smmu >> - match linux actbl2.h acpi_iort_smmu_v3 field names >> --- >> hw/arm/virt-acpi-build.c| 56 >> +++-- >> include/hw/acpi/acpi-defs.h | 15 >> 2 files changed, 64 insertions(+), 7 deletions(-) > > Since ACPI is definitely not my area of expertise, I've cc'd > Shannon on this patch. Shannon, could you take a look at it, please? > Sure. >> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c >> index f7fa795..4b5ad91 100644 >> --- a/hw/arm/virt-acpi-build.c >> +++ b/hw/arm/virt-acpi-build.c >> @@ -393,19 +393,26 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, >> unsigned xsdt_tbl_offset) >> } >> >> static void >> -build_iort(GArray *table_data, BIOSLinker *linker) >> +build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) >> { >> -int iort_start = table_data->len; >> +int nb_nodes, iort_start = table_data->len; >> AcpiIortIdMapping *idmap; >> AcpiIortItsGroup *its; >> AcpiIortTable *iort; >> -size_t node_size, iort_length; >> +AcpiIortSmmu3 *smmu; >> +size_t node_size, iort_length, smmu_offset = 0; >> AcpiIortRC *rc; >> >> iort = acpi_data_push(table_data, sizeof(*iort)); >> >> +if (vms->iommu) { use if (vms->iommu == VIRT_IOMMU_SMMUV3) ? in case we support other types of SMMU. >> +nb_nodes = 3; /* RC, ITS, SMMUv3 */ >> +} else { >> +nb_nodes = 2; /* RC, ITS */ >> +} >> + >> iort_length = sizeof(*iort); >> -iort->node_count = cpu_to_le32(2); /* RC and ITS nodes */ >> +iort->node_count = cpu_to_le32(nb_nodes); >> iort->node_offset = cpu_to_le32(sizeof(*iort)); >> >> /* ITS group node */ >> @@ -418,6 +425,35 @@ build_iort(GArray *table_data, BIOSLinker *linker) >> its->its_count = cpu_to_le32(1); >> its->identifiers[0] = 0; /* MADT translation_id */ >> >> +if (vms->iommu == VIRT_IOMMU_SMMUV3) { >> +int irq = vms->irqmap[VIRT_SMMU]; >> + >> +/* SMMUv3 node */ >> +smmu_offset = cpu_to_le32(iort->node_offset + node_size); no need cpu_to_le32 here. Otherwise: Reviewed-by: Shannon Zhao >> +node_size = sizeof(*smmu) + sizeof(*idmap); >> +iort_length += node_size; >> +smmu = acpi_data_push(table_data, node_size); >> + >> + >> +smmu->type = ACPI_IORT_NODE_SMMU_V3; >> +smmu->length = cpu_to_le16(node_size); >> +smmu->mapping_count = cpu_to_le32(1); >> +smmu->mapping_offset = cpu_to_le32(sizeof(*smmu)); >> +smmu->base_address = cpu_to_le64(vms->memmap[VIRT_SMMU].base); >> +smmu->event_gsiv = cpu_to_le32(irq); >> +smmu->pri_gsiv = cpu_to_le32(irq + 1); >> +smmu->gerr_gsiv = cpu_to_le32(irq + 2); >> +smmu->sync_gsiv = cpu_to_le32(irq + 3); >> + >> +/* Identity RID mapping covering the whole input RID range */ >> +idmap = &smmu->id_mapping_array[0]; >> +idmap->input_base = 0; >> +idmap->id_count = cpu_to_le32(0x); >> +idmap->output_base = 0; >> +/* output IORT node is the ITS group node (the first node) */ >> +idmap->output_reference = cpu_to_le32(iort->node_offset); >> +} >> + >> /* Root Complex Node */ >> node_size = sizeof(*rc) + sizeof(*idmap); >> iort_length += node_size; >> @@ -438,8 +474,14 @@ build_iort(GArray *table_data, BIOSLinker *linker) >> idmap->input_base = 0; >> idmap->id_count = cpu_to_le32(0x); >> idmap->output_base = 0; >> -/* output IORT node is the ITS group node (the first node) */ >> -idmap->output_reference = cpu_to_le32(iort->node_offset); >> + >> +if (vms->iommu) { >> +/* output IORT node is the smmuv3 node */ >> +idmap->output_reference = cpu_to_le32(smmu_offset); >> +} else { >> +/* output IORT node is the ITS group node (the first node) */ >> +idmap->output_reference = cpu_to_le32(iort->node_offset); >> +} >> >> iort->length = cpu_to_le32(iort_length); >> >> @@ -786,7 +828,7 @@ void virt_acpi_build(VirtMachineState *vms, >> AcpiBuildTables *tables) >> >> if (its_class_name() && !vmc->no_its) { >> acpi_add_table(table_offsets, tables_blob); >> -build_i
Re: [Qemu-devel] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide
Phil, On Mar 19 00:14, Philippe Mathieu-Daudé wrote: > Hi Aaron, > > On 03/16/2018 09:31 PM, Aaron Lindsay wrote: > > This is a bug fix to ensure 64-bit reads of this register don't read > > adjacent data. > > > > Signed-off-by: Aaron Lindsay > > --- > > target/arm/cpu.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index 9c3b5ef..fb2f983 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -367,7 +367,7 @@ typedef struct CPUARMState { > > uint32_t c9_data; > > uint64_t c9_pmcr; /* performance monitor control register */ > > uint64_t c9_pmcnten; /* perf monitor counter enables */ > > -uint32_t c9_pmovsr; /* perf monitor overflow status */ > > +uint64_t c9_pmovsr; /* perf monitor overflow status */ > > This doesn't look correct, since this reg is 32b. > > I *think* the correct fix is in ARMCPRegInfo v7_cp_reginfo[]: > > { .name = "PMOVSR", ... > - ..., .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), > + ..., .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmovsr), > .accessfn = pmreg_access, > .writefn = pmovsr_write, > .raw_writefn = raw_write }, Nearly all of these PMU registers are 32 bits wide, but most of them are implemented as 64-bit registers (PMCR, PMCNTEN*, PMSELR, PMINTEN* are a few examples I see in this patch's context). My understanding is that AArch64 register accesses are handled as 64 bits, even if the register itself isn't that wide (though I haven't personally verified this). See an earlier email from Peter from v2 of this patchset: https://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg03983.html Does this still look wrong to you? If so, I'll take a more thorough look into how these accesses work. > > uint32_t c9_pmuserenr; /* perf monitor user enable */ Whatever we decide should likely be done to PMUSERENR too - I think I overlooked this one before. > > uint64_t c9_pmselr; /* perf monitor counter selection register */ > > uint64_t c9_pminten; /* perf monitor interrupt enables */ > > > > Regards, > > Phil. -Aaron -- Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
On 03/19/2018 08:56 AM, Su Hang wrote: Bug fix: checkpatch.pl stops complaining about following pattern: """ do { //do somethins; s/somethins/something/ } while (conditions); Having the commit message point to the commit id that introduced the bug is useful. The grammar is awkward (it sounds like the bug is that checkpatch.pl stopped complaining about the pattern, so the fix is to reinstate the complaint); better might be: Commit XYZ introduced a regression: checkpatch.pl started complaining about the following valid pattern: do { /* something */ } while (condition); Fix the script to once again permit this pattern. """ Signed-off-by: Su Hang --- Is this an updated revision to a patch posted earlier? If so, including 'v2' in the subject line (easy if you use 'git format-patch -v2' or 'git send-email -v2'), and then using this space after the --- separator to describe what changed since v1, makes life easier for reviewers. scripts/checkpatch.pl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d1fe79bcc47c..2ca833f22e5a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2355,6 +2355,18 @@ sub process { # check for missing bracing around if etc if ($line =~ /(^.*)\b(?:if|while|for)\b/ && $line !~ /\#\s*if/) { + my $allowed = 0; + + # Check the pre-context. + if ($line =~ /(\}.*?)$/) { + my $pre = $1; + + if ($line !~ /else/) { + print "APW: ALLOWED: pre<$pre> line<$line>\n" + if $dbg_adv_apw; + $allowed = 1; + } + } my ($level, $endln, @chunks) = ctx_statement_full($linenr, $realcnt, 1); if ($dbg_adv_apw) { @@ -2363,7 +2375,6 @@ sub process { if $#chunks >= 1; } if ($#chunks >= 0 && $level == 0) { - my $allowed = 0; my $seen = 0; my $herectx = $here . "\n"; my $ln = $linenr - 1; @@ -2407,7 +2418,7 @@ sub process { $allowed = 1; } } - if ($seen != ($#chunks + 1)) { + if ($seen != ($#chunks + 1) && !$allowed) { ERROR("braces {} are necessary for all arms of this statement\n" . $herectx); } } -- 2.7.4 -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PULL 00/38] QAPI patches for 2018-03-12, 2.12 softfreeze
On 03/19/2018 09:57 AM, Eric Blake wrote: On 03/19/2018 04:26 AM, Peter Xu wrote: for you to fetch changes up to 75eb57e3ed3682f011a6694863044e8b143a9821: qapi: Pass '-u' when doing non-silent diff (2018-03-16 09:00:07 -0500) Hi. I get a bunch of test assertion failures with this: I haven't been able to reproduce the testsuite failures on my Linux box, but if it's a race, then that doesn't make me all the more confident on what it takes to reproduce and/or fix the race. Okay, my simple builds on just x86_64-softmmu weren't hitting it, but my 'build all binaries' tree seems to be hitting the same thing: GTESTER check-qtest-ppcemb qemu-system-ppcemb: chardev/char-io.c:91: io_watch_poll_finalize: Assertion `iwp->src == NULL' failed. Broken pipe GTester: last random seed: R02S74d45e64b38428eddd131a5c1b4c878c make: *** [/home/eblake/qemu-tmp/tests/Makefile.include:878: check-qtest-ppcemb] Error 1 so I'm now testing if your squash makes a difference, now that it looks like I'm reproducing the problem. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [Qemu-devel] [PATCH v4] file-posix: specify expected filetypes
Am 13.03.2018 um 18:20 hat John Snow geschrieben: > > > On 01/19/2018 06:03 PM, Eric Blake wrote: > > On 01/19/2018 04:47 PM, John Snow wrote: > >> Adjust each caller of raw_open_common to specify if they are expecting > >> host and character devices or not. Tighten expectations of file types upon > >> open in the common code and refuse types that are not expected. > >> > >> This has two effects: > >> > >> (1) Character and block devices are now considered deprecated for the > >> 'file' driver, which expects only S_IFREG, and > >> (2) no file-posix driver (file, host_cdrom, or host_device) can open > >> directories now. > >> > >> I don't think there's a legitimate reason to open directories as if > >> they were files. This prevents QEMU from opening and attempting to probe > >> a directory inode, which can break in exciting ways. One of those ways > >> is lseek on ext4/xfs, which will return 0x7fff as the file > >> size instead of EISDIR. This can coax QEMU into responding with a > >> confusing "file too big" instead of "Hey, that's not a file". > >> > >> See: https://bugs.launchpad.net/qemu/+bug/1739304/ > >> Signed-off-by: John Snow > >> --- > > > > Reviewed-by: Eric Blake > > Whoops, I let this one rot. It could still be considered a bugfix for > next week. Yes, we should take this as a bugfix. Needs a rebase, though. Kevin
Re: [Qemu-devel] [PATCH v3 23/24] RISC-V: Convert cpu definition towards future model
On Fri, 16 Mar 2018 12:41:20 -0700 Michael Clark wrote: > - Model borrowed from target/sh4/cpu.c > - Rewrote riscv_cpu_list to use object_class_get_list > - Dropped 'struct RISCVCPUInfo' and used TypeInfo array > - Replaced riscv_cpu_register_types with DEFINE_TYPES > - Marked base class as abstract > > Cc: Igor Mammedov > Cc: Sagar Karandikar > Cc: Bastian Koppelmann > Signed-off-by: Palmer Dabbelt > Signed-off-by: Michael Clark > Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov > --- [...] > -static void riscv_cpu_register_types(void) > +static void riscv_cpu_list_entry(gpointer data, gpointer user_data) > { > -const RISCVCPUInfo *info = riscv_cpus; > +RISCVCPUListState *s = user_data; > +const char *typename = object_class_get_name(OBJECT_CLASS(data)); > +int len = strlen(typename) - strlen(RISCV_CPU_TYPE_SUFFIX); This also fixes "-cpu" output, before this patch: # qemu-system-riscv32 -cpu help any-riscv-cpu rv32gcsu-v1.9.1-riscv-cpu rv32gcsu-v1.10.0-riscv-cpu rv32imacu-nommu-riscv-cpu sifive-e31-riscv-cpu sifive-u34-riscv-cpu # qemu-system-riscv32 -cpu rv32gcsu-v1.9.1-riscv-cpu qemu-system-riscv32: unable to find CPU model 'rv32gcsu-v1.9.1-riscv-cpu' after this patch: # qemu-system-riscv32 -cpu help any rv32gcsu-v1.10.0 rv32gcsu-v1.9.1 rv32imacu-nommu sifive-e31 sifive-u34 which cpu model matches conversion rules of riscv_cpu_class_by_name() and matching cpu type is found as expected. > -type_register_static(&riscv_cpu_type_info); > +(*s->cpu_fprintf)(s->file, "%.*s\n", len, typename); > +} [...]
[Qemu-devel] [PULL v2 3/7] fixup! tests: add machine 'none' with -cpu test
Fixup test to account for 2 new targets that were merged since this test was written. Signed-off-by: Igor Mammedov --- tests/machine-none-test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/machine-none-test.c b/tests/machine-none-test.c index 596ad01..efdd4be 100644 --- a/tests/machine-none-test.c +++ b/tests/machine-none-test.c @@ -55,6 +55,8 @@ static struct arch2cpu cpus_map[] = { { "xtensa", "dc233c" }, { "xtensaeb", "fsf" }, { "hppa", "hppa" }, +{ "riscv64", "rv64gcsu-v1.10.0" }, +{ "riscv32", "rv32gcsu-v1.9.1" }, }; static const char *get_cpu_model_by_arch(const char *arch) -- 2.7.4
Re: [Qemu-devel] [PATCH] iotests: 163 is not quick
Am 10.03.2018 um 22:45 hat Eric Blake geschrieben: > Testing on ext4, most 'quick' qcow2 tests took less than 5 seconds, > but 163 took more than 20. Let's remove it from the quick set. > > Signed-off-by: Eric Blake Takes only 11 seconds for me, but that's still longer than most other tests. Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v3 14/22] target/arm: Make PMOVSCLR 64 bits wide
On 19 March 2018 at 15:24, Aaron Lindsay wrote: > Phil, > > On Mar 19 00:14, Philippe Mathieu-Daudé wrote: >> Hi Aaron, >> >> On 03/16/2018 09:31 PM, Aaron Lindsay wrote: >> > This is a bug fix to ensure 64-bit reads of this register don't read >> > adjacent data. >> > >> > Signed-off-by: Aaron Lindsay >> > --- >> > target/arm/cpu.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h >> > index 9c3b5ef..fb2f983 100644 >> > --- a/target/arm/cpu.h >> > +++ b/target/arm/cpu.h >> > @@ -367,7 +367,7 @@ typedef struct CPUARMState { >> > uint32_t c9_data; >> > uint64_t c9_pmcr; /* performance monitor control register */ >> > uint64_t c9_pmcnten; /* perf monitor counter enables */ >> > -uint32_t c9_pmovsr; /* perf monitor overflow status */ >> > +uint64_t c9_pmovsr; /* perf monitor overflow status */ >> >> This doesn't look correct, since this reg is 32b. >> >> I *think* the correct fix is in ARMCPRegInfo v7_cp_reginfo[]: >> >> { .name = "PMOVSR", ... >> - ..., .fieldoffset = offsetof(CPUARMState, cp15.c9_pmovsr), >> + ..., .fieldoffset = offsetoflow32(CPUARMState, cp15.c9_pmovsr), >> .accessfn = pmreg_access, >> .writefn = pmovsr_write, >> .raw_writefn = raw_write }, > > Nearly all of these PMU registers are 32 bits wide, but most of them are > implemented as 64-bit registers (PMCR, PMCNTEN*, PMSELR, PMINTEN* are a > few examples I see in this patch's context). My understanding is that > AArch64 register accesses are handled as 64 bits, even if the register > itself isn't that wide (though I haven't personally verified this). Correct. Technically there's no such thing as a 32-bit wide AArch64 system register -- that is just a shorthand in the Arm ARM for "64-bit wide with the top 32-bits being RES0". thanks -- PMM
Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix
> -Original Messages- > From: "Eric Blake" > Sent Time: 2018-03-19 23:25:20 (Monday) > To: "Su Hang" , vsement...@virtuozzo.com > Cc: qemu-devel@nongnu.org > Subject: Re: [Qemu-devel] [PATCH RFC] scripts/checkpatch.pl: Bug fix > Having the commit message point to the commit id that introduced the bug > is useful. The grammar is awkward (it sounds like the bug is that > checkpatch.pl stopped complaining about the pattern, so the fix is to > reinstate the complaint); better might be: > > Commit XYZ introduced a regression: checkpatch.pl started complaining > about the following valid pattern: > do { > /* something */ > } while (condition); > > Fix the script to once again permit this pattern. Thank you for correcting my mistakes. :-) > Is this an updated revision to a patch posted earlier? If so, including > 'v2' in the subject line (easy if you use 'git format-patch -v2' or 'git > send-email -v2'), and then using this space after the --- separator to > describe what changed since v1, makes life easier for reviewers. Yes, it's an updated revision to a earlier patch. But I wasn't about sending it, when I was using [ctrl + r] and [enter], I send it accidentally. Thanks for your kind suggestion. Best, Su Hang
Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR
On 19/03/2018 10:04, chao@linux.intel.com wrote: > From: Qin Chao > > Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit > 8, which is BSP flag and should be set to 1 for the bootstrap > processor and set to 0 for the application processors, but it's set > to 0 for all processors in HAXM. So guest OSes that expect a valid > BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot > boot with "-accel hax". To solve this problem, HAXM (which lacks > APIC virtualization) and QEMU must notify each other of any change > to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into > HAXM source. QEMU needs to use the new HAXM API (apic_base in > "struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR, > and then, update its own copy at every return from > HAX_VCPU_IOCTL_RUN. > > There will be a backward compatility issue caused by the new field > "apic_base" added into "struct hax_tunnel". In order to fix the > problem, the validation for size of "struct hax_tunnel" is removed > and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which > means that one page (4KB) is allocated in HAXM kernel to store > "struct hax_tunnel", instead of the size of "struct hax_tunnel". > > Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3 > Signed-off-by: Qin Chao > --- > target/i386/hax-all.c | 24 +++- > target/i386/hax-darwin.c| 6 -- > target/i386/hax-i386.h | 2 +- > target/i386/hax-interface.h | 3 +++ > target/i386/hax-windows.c | 5 - > 5 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c > index cad7531..6a840d9 100644 > --- a/target/i386/hax-all.c > +++ b/target/i386/hax-all.c > @@ -62,11 +62,6 @@ int hax_enabled(void) > return hax_allowed; > } > > -int valid_hax_tunnel_size(uint16_t size) > -{ > -return size >= sizeof(struct hax_tunnel); > -} > - > hax_fd hax_vcpu_get_fd(CPUArchState *env) > { > struct hax_vcpu_state *vcpu = ENV_GET_CPU(env)->hax_vcpu; > @@ -104,6 +99,7 @@ static int hax_get_capability(struct hax_state *hax) > } > > hax->supports_64bit_ramblock = !!(cap->winfo & HAX_CAP_64BIT_RAMBLOCK); > +hax->supports_tunnel_page = !!(cap->winfo & HAX_CAP_TUNNEL_PAGE); > > if (cap->wstatus & HAX_CAP_MEMQUOTA) { > if (cap->mem_quota < hax->mem_quota) { > @@ -520,6 +516,21 @@ static int hax_vcpu_hax_exec(CPUArchState *env) > cpu_exec_end(cpu); > qemu_mutex_lock_iothread(); > > +/* > + * Every time HAXM exits to QEMU, sync IA32_APIC_BASE MSR from HAXM > and > + * pass it to the emulated APIC. > + */ > +if (hax_global.supports_tunnel_page) { > +/* > + * ht->apic_base is not available in HAXM kernel module if HAXM > does > + * not support HAX_CAP_SUPPORT_TUNNEL_PAGE. > + * TODO: HAX_CAP_SUPPORT_TUNNEL_PAGE is used for backward > + * compatibility with HAXM kernel module. Remove this check when > we > + * drop support for HAXM versions that lack this feature. > + */ > +cpu_set_apic_base(x86_cpu->apic_state, ht->apic_base); > +} > + > /* Simply continue the vcpu_run if system call interrupted */ > if (hax_ret == -EINTR || hax_ret == -EAGAIN) { > DPRINTF("io window interrupted\n"); > @@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) > hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); > hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase); > #endif > +hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \ > + cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); > + > md.nr_msr = n; > md.done = 0; > > diff --git a/target/i386/hax-darwin.c b/target/i386/hax-darwin.c > index acdde47..3e2fd4f 100644 > --- a/target/i386/hax-darwin.c > +++ b/target/i386/hax-darwin.c > @@ -244,12 +244,6 @@ int hax_host_setup_vcpu_channel(struct hax_vcpu_state > *vcpu) > return ret; > } > > -if (!valid_hax_tunnel_size(info.size)) { > -fprintf(stderr, "Invalid hax tunnel size %x\n", info.size); > -ret = -EINVAL; > -return ret; > -} > - > vcpu->tunnel = (struct hax_tunnel *) (intptr_t) (info.va); > vcpu->iobuf = (unsigned char *) (intptr_t) (info.io_va); > return 0; > diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h > index 6abc156..b04bf24 100644 > --- a/target/i386/hax-i386.h > +++ b/target/i386/hax-i386.h > @@ -38,6 +38,7 @@ struct hax_state { > struct hax_vm *vm; > uint64_t mem_quota; > bool supports_64bit_ramblock; > +bool supports_tunnel_page; > }; > > #define HAX_MAX_VCPU 0x10 > @@ -53,7 +54,6 @@ struct hax_vm { > #ifdef NEED_CPU_H > /* Functions exported to host specific mode */ > hax_fd hax_vcpu_get_fd(CPUArchState *env); > -int valid_hax_tunnel_size(uint16_t size
Re: [Qemu-devel] [PATCH] replay: finish record/replay before closing the disks
On 19/03/2018 10:25, Pavel Dovgalyuk wrote: > After recent updates block devices cannot be closed on qemu exit. > This happens due to the block request polling when replay is not finished. > Therefore now we stop execution recording before closing the block devices. > > Signed-off-by: Pavel Dovgalyuk > --- > replay/replay.c |2 ++ > vl.c|1 + > 2 files changed, 3 insertions(+) > > diff --git a/replay/replay.c b/replay/replay.c > index 8228261..58a986f 100644 > --- a/replay/replay.c > +++ b/replay/replay.c > @@ -366,6 +366,8 @@ void replay_finish(void) > g_free(replay_snapshot); > replay_snapshot = NULL; > > +replay_mode = REPLAY_MODE_NONE; > + > replay_finish_events(); > } > > diff --git a/vl.c b/vl.c > index e8bebda..f4d9153 100644 > --- a/vl.c > +++ b/vl.c > @@ -4733,6 +4733,7 @@ int main(int argc, char **argv, char **envp) > > /* No more vcpu or device emulation activity beyond this point */ > vm_shutdown(); > +replay_finish(); > > bdrv_close_all(); > > Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH v2] tcg: Really fix cpu_io_recompile
On 19/03/2018 04:15, Richard Henderson wrote: > We have confused the number of instructions that have been > executed in the TB with the number of instructions needed > to repeat the I/O instruction. > > We have used cpu_restore_state_from_tb, which means that > the guest pc is pointing to the I/O instruction. The only > time the answer to the later question is not 1 is when > MIPS or SH4 need to re-execute the branch for the delay > slot as well. > > We must rely on cpu->cflags_next_tb to generate the next TB, > as otherwise we have a race condition with other guest cpus > within the TB cache. > > Fixes: 0790f86861079b1932679d0f011e431aaf4ee9e2 > Signed-off-by: Richard Henderson > --- > > My v1 raced with Paolo's pull request, so v2 now fixes Pavel's fix. Thanks, let me know if you prefer to send a pull request yourself, or if I should include it in the next. Thanks, Paolo > > > r~ > > --- > accel/tcg/translate-all.c | 37 ++--- > 1 file changed, 10 insertions(+), 27 deletions(-) > > diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c > index 5ad1b919bc..d4190602d1 100644 > --- a/accel/tcg/translate-all.c > +++ b/accel/tcg/translate-all.c > @@ -1728,8 +1728,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) > CPUArchState *env = cpu->env_ptr; > #endif > TranslationBlock *tb; > -uint32_t n, flags; > -target_ulong pc, cs_base; > +uint32_t n; > > tb_lock(); > tb = tb_find_pc(retaddr); > @@ -1737,44 +1736,33 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t > retaddr) > cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p", >(void *)retaddr); > } > -n = cpu->icount_decr.u16.low + tb->icount; > cpu_restore_state_from_tb(cpu, tb, retaddr); > -/* Calculate how many instructions had been executed before the fault > - occurred. */ > -n = n - cpu->icount_decr.u16.low; > -/* Generate a new TB ending on the I/O insn. */ > -n++; > + > /* On MIPS and SH, delay slot instructions can only be restarted if > they were already the first instruction in the TB. If this is not > the first instruction in a TB then re-execute the preceding > branch. */ > +n = 1; > #if defined(TARGET_MIPS) > -if ((env->hflags & MIPS_HFLAG_BMASK) != 0 && n > 1) { > +if ((env->hflags & MIPS_HFLAG_BMASK) != 0 > +&& env->active_tc.PC != tb->pc) { > env->active_tc.PC -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4); > cpu->icount_decr.u16.low++; > env->hflags &= ~MIPS_HFLAG_BMASK; > +n = 2; > } > #elif defined(TARGET_SH4) > if ((env->flags & ((DELAY_SLOT | DELAY_SLOT_CONDITIONAL))) != 0 > -&& n > 1) { > +&& env->pc != tb->pc) { > env->pc -= 2; > cpu->icount_decr.u16.low++; > env->flags &= ~(DELAY_SLOT | DELAY_SLOT_CONDITIONAL); > +n = 2; > } > #endif > -/* This should never happen. */ > -if (n > CF_COUNT_MASK) { > -cpu_abort(cpu, "TB too big during recompile"); > -} > > -pc = tb->pc; > -cs_base = tb->cs_base; > -flags = tb->flags; > -tb_phys_invalidate(tb, -1); > - > -/* Execute one IO instruction without caching > - instead of creating large TB. */ > -cpu->cflags_next_tb = curr_cflags() | CF_LAST_IO | CF_NOCACHE | 1; > +/* Generate a new TB executing the I/O insn. */ > +cpu->cflags_next_tb = curr_cflags() | CF_LAST_IO | n; > > if (tb->cflags & CF_NOCACHE) { > if (tb->orig_tb) { > @@ -1785,11 +1773,6 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr) > tb_remove(tb); > } > > -/* Generate new TB instead of the current one. */ > -/* FIXME: In theory this could raise an exception. In practice > - we have already translated the block once so it's probably ok. */ > -tb_gen_code(cpu, pc, cs_base, flags, curr_cflags() | CF_LAST_IO | n); > - > /* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not > * the first in the TB) then we end up generating a whole new TB and > * repeating the fault, which is horribly inefficient. >
Re: [Qemu-devel] [PATCH] hax: Properly handle IA32_APIC_BASE MSR
On Mon, 19 Mar 2018 17:04:49 +0800 chao@linux.intel.com wrote: > From: Qin Chao > > Emulation of IA32_APIC_BASE MSR in HAXM is not correct, such as bit > 8, which is BSP flag and should be set to 1 for the bootstrap > processor and set to 0 for the application processors, but it's set > to 0 for all processors in HAXM. So guest OSes that expect a valid > BSP flag, such as Zircon (the core of Google Fuchsia OS), cannot > boot with "-accel hax". To solve this problem, HAXM (which lacks > APIC virtualization) and QEMU must notify each other of any change > to guest IA32_APIC_BASE MSR. The HAXM patch has been merged into > HAXM source. QEMU needs to use the new HAXM API (apic_base in > "struct hax_tunnel") to initialize the guest IA32_APIC_BASE MSR, > and then, update its own copy at every return from > HAX_VCPU_IOCTL_RUN. > > There will be a backward compatility issue caused by the new field > "apic_base" added into "struct hax_tunnel". In order to fix the > problem, the validation for size of "struct hax_tunnel" is removed > and a new capability flag "HAX_CAP_TUNNEL_PAGE" is added, which > means that one page (4KB) is allocated in HAXM kernel to store > "struct hax_tunnel", instead of the size of "struct hax_tunnel". > > Change-Id: I8505bc1d75c495dd2765e581d6014125dcb538f3 > Signed-off-by: Qin Chao > --- > target/i386/hax-all.c | 24 +++- > target/i386/hax-darwin.c| 6 -- > target/i386/hax-i386.h | 2 +- > target/i386/hax-interface.h | 3 +++ > target/i386/hax-windows.c | 5 - > 5 files changed, 23 insertions(+), 17 deletions(-) > > diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c > index cad7531..6a840d9 100644 > --- a/target/i386/hax-all.c > +++ b/target/i386/hax-all.c [...] > @@ -933,6 +944,9 @@ static int hax_set_msrs(CPUArchState *env) > hax_msr_entry_set(&msrs[n++], MSR_FMASK, env->fmask); > hax_msr_entry_set(&msrs[n++], MSR_KERNELGSBASE, env->kernelgsbase); > #endif > +hax_msr_entry_set(&msrs[n++], MSR_IA32_APICBASE, \ > + cpu_get_apic_base(x86_env_get_cpu(env)->apic_state)); > + > md.nr_msr = n; > md.done = 0; Does it work for you if you drop everything except of this chunk? There is no much point in syncing BPS flag from HAXM since Seabios nor OVMF do not implement BSP selection protocol, it's hard-coded in QEMU that BSP is the first CPU. Considering that typically no sane OS does change MSR_IA32_APICBASE there is no need to sync it back to QEMU. So no need to introduce all that complexity for no gain. [...]
Re: [Qemu-devel] [Qemu-block] [PATCH v1 1/1] iotests: fix test case 185
On Mon, Mar 19, 2018 at 9:35 AM, QingFeng Hao wrote: > Test case 185 failed since commit 4486e89c219 --- "vl: introduce > vm_shutdown()". > It's because of the newly introduced function vm_shutdown calls > bdrv_drain_all, > which is called later by bdrv_close_all. bdrv_drain_all resumes the jobs > that doubles the speed and offset is doubled. > Some jobs' status are changed as well. > > Thus, let's not call bdrv_drain_all in vm_shutdown. > > Signed-off-by: QingFeng Hao > --- > cpus.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/cpus.c b/cpus.c > index 2e6701795b..ae2962508c 100644 > --- a/cpus.c > +++ b/cpus.c > @@ -1006,8 +1006,9 @@ static int do_vm_stop(RunState state, bool send_stop) > qapi_event_send_stop(&error_abort); > } > } > - > -bdrv_drain_all(); > +if (send_stop) { > +bdrv_drain_all(); > +} Thanks for looking into this bug! This if statement breaks the contract of the function when send_stop is false. Drain ensures that pending I/O completes and that device emulation finishes before this function returns. This is important for live migration, where there must be no more guest-related activity after vm_stop(). This patch relies on the caller invoking bdrv_close_all() immediately after do_vm_stop(), which is not documented and could lead to more bugs in the future. I suggest a different solution: Test 185 relies on internal QEMU behavior which can change from time to time. Buffer sizes and block job iteration counts are implementation details that are not part of qapi-schema.json or other documented behavior. In fact, the existing test case was already broken in IOThread mode since iothread_stop_all() -> bdrv_set_aio_context() also includes a bdrv_drain() with the side-effect of an extra blockjob iteration. After 4486e89c219 ("vl: introduce vm_shutdown()") both IOThread and non-IOThread mode perform the drain. This has caused the test failure. Instead of modifying QEMU to keep the same arbitrary internal behavior as before, please send a patch that updates tests/qemu-iotests/185.out with the latest output. Stefan
[Qemu-devel] [PATCH for-2.12 0/2] bcm2835_sdhost: fix interrupt handling
This patchset fixes the code in the bcm2835_sdhost device so that it raises interrupts in more plausible places. The Linux bcm2835_sdhost driver doesn't work on QEMU at the moment, because our model raises spurious data interrupts. Our function bcm2835_sdhost_fifo_run() will flag an interrupt any time it is called with s->datacnt == 0, even if the host hasn't actually issued a data read or write command yet. This means that the driver gets a spurious data interrupt as soon as it enables IRQs and then does something else that causes us to call the fifo_run routine, like writing to SDHCFG, and before it does the write to SDCMD to issue the read. The driver's IRQ handler then spins forever complaining that there's no data and the SD controller isn't in a state where there's going to be any data: [ 41.040738] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts [ 41.042059] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts (continues forever). Move the interrupt flag setting to more plausible places: * for BUSY, raise this as soon as a BUSYWAIT command has executed * for DATA, raise this when the FIFO has any space free (for a write) or any data in it (for a read) * for BLOCK, raise this when the data count is 0 and we've actually done some reading or writing This is pure guesswork since the documentation for this hardware is not public, but it is sufficient to get the Linux bcm2835_sdhost driver to work. If anybody has other OSes than Linux that use the sdhost SD controller (not the 'Arasan' one, which is a different bit of the SoC) testing with those would be appreciated. If anybody has documentation for this hardware that would be even better. With these patches plus the previous set, I can get the Debian raspi3 image to boot (though there are a lot of warnings relating to the pl011 UART for some reason). thanks -- PMM Peter Maydell (2): hw/sd/bcm2835_sdhost: Add tracepoints hw/sd/bcm2835_sdhost: Don't raise spurious interrupts hw/sd/bcm2835_sdhost.c | 51 +++--- hw/sd/trace-events | 6 ++ 2 files changed, 38 insertions(+), 19 deletions(-) -- 2.16.2
[Qemu-devel] qemu:handle_cpu_signal received signal outside vCPU context
I'm seeing this error while building gedit for riscv64 with linux-user emulation: $ LD_LIBRARY_PATH=gedit/.libs qemu-riscv64 gedit/.libs/gedit --introspect-dump=/tmp/tmp-introspectnj0xla07/functions.txt,/tmp/tmp-introspectnj0xla07/dump.xml qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x6003d7d5 qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x60106a16 This is qemu as of today with the patches from git://github.com/riscv/riscv-qemu / riscv-all on top. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: [Qemu-devel] [PATCH v4] file-posix: specify expected filetypes
On 03/19/2018 11:29 AM, Kevin Wolf wrote: > Am 13.03.2018 um 18:20 hat John Snow geschrieben: >> >> >> On 01/19/2018 06:03 PM, Eric Blake wrote: >>> On 01/19/2018 04:47 PM, John Snow wrote: Adjust each caller of raw_open_common to specify if they are expecting host and character devices or not. Tighten expectations of file types upon open in the common code and refuse types that are not expected. This has two effects: (1) Character and block devices are now considered deprecated for the 'file' driver, which expects only S_IFREG, and (2) no file-posix driver (file, host_cdrom, or host_device) can open directories now. I don't think there's a legitimate reason to open directories as if they were files. This prevents QEMU from opening and attempting to probe a directory inode, which can break in exciting ways. One of those ways is lseek on ext4/xfs, which will return 0x7fff as the file size instead of EISDIR. This can coax QEMU into responding with a confusing "file too big" instead of "Hey, that's not a file". See: https://bugs.launchpad.net/qemu/+bug/1739304/ Signed-off-by: John Snow --- >>> >>> Reviewed-by: Eric Blake >> >> Whoops, I let this one rot. It could still be considered a bugfix for >> next week. > > Yes, we should take this as a bugfix. Needs a rebase, though. > > Kevin > You got it. --js
[Qemu-devel] [PATCH] tpm: Set tpmRegValidSts flag to '1' in initialization
Fix the initialization of the tpmRegValidSts flag and set it to '1' during device reset without expecting a write to another register. This seems to also be the default behavior of real hardware. Signed-off-by: Stefan Berger --- hw/tpm/tpm_crb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c index d8917cb..114b66e 100644 --- a/hw/tpm/tpm_crb.c +++ b/hw/tpm/tpm_crb.c @@ -145,8 +145,6 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, beenSeized, 0); ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, locAssigned, 1); -ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, - tpmRegValidSts, 1); break; } break; @@ -210,6 +208,8 @@ static void tpm_crb_reset(void *dev) tpm_backend_reset(s->tpmbe); +ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, + tpmRegValidSts, 1); ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE); ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, -- 2.5.5
[Qemu-devel] [PATCH] tpm: Set tpmRegValidSts flag to '1' in device reset
Fix the initialization of the tpmRegValidSts flag and set it to '1' during device reset without expecting a write to another register. This seems to also be the default behavior of real hardware. Signed-off-by: Stefan Berger --- hw/tpm/tpm_crb.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c index d8917cb..114b66e 100644 --- a/hw/tpm/tpm_crb.c +++ b/hw/tpm/tpm_crb.c @@ -145,8 +145,6 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, beenSeized, 0); ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, locAssigned, 1); -ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, - tpmRegValidSts, 1); break; } break; @@ -210,6 +208,8 @@ static void tpm_crb_reset(void *dev) tpm_backend_reset(s->tpmbe); +ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, + tpmRegValidSts, 1); ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE); ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, -- 2.5.5
[Qemu-devel] [PATCH RFC v2] scripts/checkpatch.pl: Bug fix
Commit 2b9aef6fcd96ba7ed8c1ee723e391901852d344c introduced a regression: checkpatch.pl started complaining about the following valid pattern: do { /* something */ } while (condition); Fix the script to once again permit this pattern. --- v1: fix bug. v2: correct inappropriate patch description. Signed-off-by: Su Hang --- scripts/checkpatch.pl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 57daae05ea18..d52207a3cc9d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2356,6 +2356,18 @@ sub process { # check for missing bracing around if etc if ($line =~ /(^.*)\b(?:if|while|for)\b/ && $line !~ /\#\s*if/) { + my $allowed = 0; + + # Check the pre-context. + if ($line =~ /(\}.*?)$/) { + my $pre = $1; + + if ($line !~ /else/) { + print "APW: ALLOWED: pre<$pre> line<$line>\n" + if $dbg_adv_apw; + $allowed = 1; + } + } my ($level, $endln, @chunks) = ctx_statement_full($linenr, $realcnt, 1); if ($dbg_adv_apw) { @@ -2364,7 +2376,6 @@ sub process { if $#chunks >= 1; } if ($#chunks >= 0 && $level == 0) { - my $allowed = 0; my $seen = 0; my $herectx = $here . "\n"; my $ln = $linenr - 1; @@ -2408,7 +2419,7 @@ sub process { $allowed = 1; } } - if ($seen != ($#chunks + 1)) { + if ($seen != ($#chunks + 1) && !$allowed) { ERROR("braces {} are necessary for all arms of this statement\n" . $herectx); } } -- 2.7.4
Re: [Qemu-devel] [PATCH 1/2] arm_gicv3_kvm: increase clroffset accordingly
On 19 March 2018 at 14:36, Shannon Zhao wrote: > It forgot to increase clroffset during the loop. So it only clear the > first 4 bytes. > > Signed-off-by: Shannon Zhao > --- > hw/intc/arm_gicv3_kvm.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c > index ec37177..7000716 100644 > --- a/hw/intc/arm_gicv3_kvm.c > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -232,9 +232,10 @@ static void kvm_dist_getbmp(GICv3State *s, uint32_t > offset, uint32_t *bmp) > static void kvm_dist_putbmp(GICv3State *s, uint32_t offset, > uint32_t clroffset, uint32_t *bmp) > { > -uint32_t reg; > +uint32_t reg, clroffset_index; > int irq; > > +clroffset_index = clroffset; > for_each_dist_irq_reg(irq, s->num_irq, 1) { > /* If this bitmap is a set/clear register pair, first write to the > * clear-reg to clear all bits before using the set-reg to write > @@ -242,7 +243,8 @@ static void kvm_dist_putbmp(GICv3State *s, uint32_t > offset, > */ > if (clroffset != 0) { > reg = 0; > -kvm_gicd_access(s, clroffset, ®, true); > +kvm_gicd_access(s, clroffset_index, ®, true); > +clroffset_index += 4; > } > reg = *gic_bmp_ptr32(bmp, irq); > kvm_gicd_access(s, offset, ®, true); You might as well just use 'clroffset' directly rather than creating a new clroffset_index variable, I think. thanks -- PMM
Re: [Qemu-devel] [PATCH for-2.12] gitmodules: Use the QEMU mirror of qemu-palcode
On Mon, Mar 19, 2018 at 01:17:43PM +, Peter Maydell wrote: > We have a mirror of the qemu-palcode repository on > git.qemu.org; use that instead of the upstream github, > in line with our general policy of keeping and using > a mirror for submodules. > > Signed-off-by: Peter Maydell > --- > We also currently have two submodules we don't have mirroring > for: seabios-hppa and u-boot-sam460ex. > > .gitmodules | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) The git.qemu.org mirror appears up-to-date: Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PATCH for-2.12 2/2] hw/sd/bcm2835_sdhost: Don't raise spurious interrupts
The Linux bcm2835_sdhost driver doesn't work on QEMU, because our model raises spurious data interrupts. Our function bcm2835_sdhost_fifo_run() will flag an interrupt any time it is called with s->datacnt == 0, even if the host hasn't actually issued a data read or write command yet. This means that the driver gets a spurious data interrupt as soon as it enables IRQs and then does something else that causes us to call the fifo_run routine, like writing to SDHCFG, and before it does the write to SDCMD to issue the read. The driver's IRQ handler then spins forever complaining that there's no data and the SD controller isn't in a state where there's going to be any data: [ 41.040738] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts [ 41.042059] sdhost-bcm2835 3f202000.mmc: fsm 1, hsts (continues forever). Move the interrupt flag setting to more plausible places: * for BUSY, raise this as soon as a BUSYWAIT command has executed * for DATA, raise this when the FIFO has any space free (for a write) or any data in it (for a read) * for BLOCK, raise this when the data count is 0 and we've actually done some reading or writing This is pure guesswork since the documentation for this hardware is not public, but it is sufficient to get the Linux bcm2835_sdhost driver to work. Signed-off-by: Peter Maydell --- hw/sd/bcm2835_sdhost.c | 43 +++ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c index 79f3c5ceeb..0fd0853fa3 100644 --- a/hw/sd/bcm2835_sdhost.c +++ b/hw/sd/bcm2835_sdhost.c @@ -137,6 +137,9 @@ static void bcm2835_sdhost_send_command(BCM2835SDHostState *s) } #undef RWORD } +if ((s->cmd & SDCMD_BUSYWAIT) && (s->config & SDHCFG_BUSY_IRPT_EN)) { +s->status |= SDHSTS_BUSY_IRPT; +} return; error: @@ -187,18 +190,27 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) n++; if (n == 4) { bcm2835_sdhost_fifo_push(s, value); +s->status |= SDHSTS_DATA_FLAG; +if (s->config & SDHCFG_DATA_IRPT_EN) { +s->status |= SDHSTS_SDIO_IRPT; +} n = 0; value = 0; } } if (n != 0) { bcm2835_sdhost_fifo_push(s, value); +s->status |= SDHSTS_DATA_FLAG; } } else { /* write */ n = 0; while (s->datacnt > 0 && (s->fifo_len > 0 || n > 0)) { if (n == 0) { value = bcm2835_sdhost_fifo_pop(s); +s->status |= SDHSTS_DATA_FLAG; +if (s->config & SDHCFG_DATA_IRPT_EN) { +s->status |= SDHSTS_SDIO_IRPT; +} n = 4; } n--; @@ -207,28 +219,19 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) value >>= 8; } } +if (s->datacnt == 0) { +s->edm &= ~0xf; +s->edm |= SDEDM_FSM_DATAMODE; +trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm); + +if ((s->cmd & SDCMD_WRITE_CMD) && +(s->config & SDHCFG_BLOCK_IRPT_EN)) { +s->status |= SDHSTS_BLOCK_IRPT; +} +} } -if (s->datacnt == 0) { -s->status |= SDHSTS_DATA_FLAG; -s->edm &= ~0xf; -s->edm |= SDEDM_FSM_DATAMODE; -trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm); - -if (s->config & SDHCFG_DATA_IRPT_EN) { -s->status |= SDHSTS_SDIO_IRPT; -} - -if ((s->cmd & SDCMD_BUSYWAIT) && (s->config & SDHCFG_BUSY_IRPT_EN)) { -s->status |= SDHSTS_BUSY_IRPT; -} - -if ((s->cmd & SDCMD_WRITE_CMD) && (s->config & SDHCFG_BLOCK_IRPT_EN)) { -s->status |= SDHSTS_BLOCK_IRPT; -} - -bcm2835_sdhost_update_irq(s); -} +bcm2835_sdhost_update_irq(s); s->edm &= ~(0x1f << 4); s->edm |= ((s->fifo_len & 0x1f) << 4); -- 2.16.2
[Qemu-devel] [PATCH for-2.12 1/2] hw/sd/bcm2835_sdhost: Add tracepoints
Add some tracepoints to the bcm2835_sdhost driver, to assist debugging. Signed-off-by: Peter Maydell --- hw/sd/bcm2835_sdhost.c | 10 ++ hw/sd/trace-events | 6 ++ 2 files changed, 16 insertions(+) diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c index f7f4e656df..79f3c5ceeb 100644 --- a/hw/sd/bcm2835_sdhost.c +++ b/hw/sd/bcm2835_sdhost.c @@ -15,6 +15,7 @@ #include "qemu/log.h" #include "sysemu/blockdev.h" #include "hw/sd/bcm2835_sdhost.h" +#include "trace.h" #define TYPE_BCM2835_SDHOST_BUS "bcm2835-sdhost-bus" #define BCM2835_SDHOST_BUS(obj) \ @@ -99,6 +100,7 @@ static void bcm2835_sdhost_update_irq(BCM2835SDHostState *s) { uint32_t irq = s->status & (SDHSTS_BUSY_IRPT | SDHSTS_BLOCK_IRPT | SDHSTS_SDIO_IRPT); +trace_bcm2835_sdhost_update_irq(irq); qemu_set_irq(s->irq, !!irq); } @@ -211,6 +213,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) s->edm &= ~0xf; s->edm |= SDEDM_FSM_DATAMODE; +trace_bcm2835_sdhost_edm_change("datacnt 0", s->edm); if (s->config & SDHCFG_DATA_IRPT_EN) { s->status |= SDHSTS_SDIO_IRPT; @@ -229,6 +232,7 @@ static void bcm2835_sdhost_fifo_run(BCM2835SDHostState *s) s->edm &= ~(0x1f << 4); s->edm |= ((s->fifo_len & 0x1f) << 4); +trace_bcm2835_sdhost_edm_change("fifo run", s->edm); } static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset, @@ -280,6 +284,8 @@ static uint64_t bcm2835_sdhost_read(void *opaque, hwaddr offset, break; } +trace_bcm2835_sdhost_read(offset, res, size); + return res; } @@ -288,6 +294,8 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset, { BCM2835SDHostState *s = (BCM2835SDHostState *)opaque; +trace_bcm2835_sdhost_write(offset, value, size); + switch (offset) { case SDCMD: s->cmd = value; @@ -314,6 +322,7 @@ static void bcm2835_sdhost_write(void *opaque, hwaddr offset, value &= ~0xf; } s->edm = value; +trace_bcm2835_sdhost_edm_change("guest register write", s->edm); break; case SDHCFG: s->config = value; @@ -390,6 +399,7 @@ static void bcm2835_sdhost_reset(DeviceState *dev) s->cmd = 0; s->cmdarg = 0; s->edm = 0xc60f; +trace_bcm2835_sdhost_edm_change("device reset", s->edm); s->config = 0; s->hbct = 0; s->hblc = 0; diff --git a/hw/sd/trace-events b/hw/sd/trace-events index 2059ace61f..bfd1d62efc 100644 --- a/hw/sd/trace-events +++ b/hw/sd/trace-events @@ -1,5 +1,11 @@ # See docs/devel/tracing.txt for syntax documentation. +# hw/sd/bcm2835_sdhost.c +bcm2835_sdhost_read(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" +bcm2835_sdhost_write(uint64_t offset, uint64_t data, unsigned size) "offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u" +bcm2835_sdhost_edm_change(const char *why, uint32_t edm) "(%s) EDM now 0x%x" +bcm2835_sdhost_update_irq(uint32_t irq) "IRQ bits 0x%x\n" + # hw/sd/core.c sdbus_command(const char *bus_name, uint8_t cmd, uint32_t arg, uint8_t crc) "@%s CMD%02d arg 0x%08x crc 0x%02x" sdbus_read(const char *bus_name, uint8_t value) "@%s value 0x%02x" -- 2.16.2
[Qemu-devel] [Bug 1756927] [NEW] ARMv7 LPAE: IFSR doesn't have the LPAE bit in case of BKPT
Public bug reported: When a user application triggers a 'bkpt' instruction while LPAE is used, the bit [9] of IFSR is not correctly set during the prefetch abort exception. You'll find attached a minimal example to reproduce the issue (just run 'make all'). The output I get is: supervisor user prefetch short-descriptor The last entry should read 'long-descriptor'. Qemu revision: 48ae1f60d8c9a770e6da64407984d84e25253c69 Ubuntu verison: 16.04 LTS Cross Compiler: gcc linaro 6.3.1-2017.02-x86_64_arm-eabi ** Affects: qemu Importance: Undecided Status: New ** Attachment added: "Minimal example with linker script and makefile" https://bugs.launchpad.net/bugs/1756927/+attachment/5084186/+files/ifsr_br.tar.gz -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1756927 Title: ARMv7 LPAE: IFSR doesn't have the LPAE bit in case of BKPT Status in QEMU: New Bug description: When a user application triggers a 'bkpt' instruction while LPAE is used, the bit [9] of IFSR is not correctly set during the prefetch abort exception. You'll find attached a minimal example to reproduce the issue (just run 'make all'). The output I get is: supervisor user prefetch short-descriptor The last entry should read 'long-descriptor'. Qemu revision: 48ae1f60d8c9a770e6da64407984d84e25253c69 Ubuntu verison: 16.04 LTS Cross Compiler: gcc linaro 6.3.1-2017.02-x86_64_arm-eabi To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1756927/+subscriptions