[Qemu-devel] question about vioscsi queue depth patch
Hi Vadim, I test the vioscsi 0001-queue-depth254.patch for virtio-win-0.1-74 with minor modifications about virtio api, because it have not updated to use the linux api. When a VM with four disks in one controller and 4kb 32depth sequential read for each disk at the same time, the tool of iometer which simulates the IO model become no response. If fit the patch into virtio-win-0.1-94, it works well. Is there some bug with the old virtio api, or is there some reason that the patch can not work normally in virtio-win-0.1-74? Best wishes, Ting Wang
Re: [Qemu-devel] question about vioscsi queue depth patch
On Tue, 2014-12-09 at 16:20 +0800, Wangting (Kathy) wrote: Hi Vadim, I test the vioscsi 0001-queue-depth254.patch for virtio-win-0.1-74 with minor modifications about virtio api, because it have not updated to use the linux api. When a VM with four disks in one controller and 4kb 32depth sequential read for each disk at the same time, the tool of iometer which simulates the IO model become no response. If fit the patch into virtio-win-0.1-94, it works well. Is there some bug with the old virtio api, or is there some reason that the patch can not work normally in virtio-win-0.1-74? There were a lot of bug-fixes / changes introduced since build 74 (November 2013). Some of them did affect SRB completion and queue depth logic. So do you see any performance improvement after applying the patch? Cheers, Vadim. Best wishes, Ting Wang
Re: [Qemu-devel] [PATCH 0/7] MIPS: IEEE 754-2008 features support
On 9 December 2014 at 01:54, Maciej W. Rozycki ma...@codesourcery.com wrote: Hi, This patch series comprises changes to QEMU, both the MIPS backend and generic SoftFloat support code, to support IEEE 754-2008 features introduced to revision 3.50 of the MIPS Architecture as follows I really really want to hold this patchset off until the softfloat relicensing has gone through, because repeating that work would be really painful... thanks -- PMM
Re: [Qemu-devel] suggestion for the QEMU advent calendar
Am 08.12.2014 um 17:54 hat Fabian Greffrath geschrieben: PS: I am a bit shocked by the fact how complicated it is to install GRUB on a disk image. And, John, even if the approach you outlined worked, I guess GRUB will find its device.map puzzled when it will get rebooted from inside an image file instead of an actual hard-drive partition the next time. As long as you don't say which version of GRUB, it's easy. Install GRUB Legacy, copy stage[12] and menu.lst to the image and do this in the GRUB shell: device (hd0) $IMAGE_PATH root (hd0,0) install /boot/grub/stage1 (hd0) /boot/grub/stage2 p /boot/grub/menu.lst Or boot from a floppy/CD with GRUB in the VM and run the same thing without the device command. Syslinux is relatively easy as well. Just avoid GRUB 2 if you want to set up disk images from the host. I've done it before, but it's ugly... Kevin
Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board
Hi, So... after playing with this thing for some time, it's become clear that MMIO traps are painfully slow on the aarch64 platform we've been working on (using KVM). So, as we don't have compatibility requirements, and we also can't play tricks like using x86 string instructions: How about a completely different, dma-style interface for fw_cfg access? One register for the (physical) target address. One register for the transfer size. One register for the fw_cfg entry. Possibly one register for the fw_cfg offset (not really needed, but avoids the need for side effects such as writing fw_cfg entry register clearing the offset). One register to kick the transfer. cheers, Gerd
Re: [Qemu-devel] Redundant VDE network
On 09/12/14 06:29, Dmitry Antipov wrote: On 12/08/2014 08:53 PM, Stefan Hajnoczi wrote: Try: qemu-system-sparc -m 256 \ -netdev vde,sock=/tmp/vde0,id=vde0 \ -device ne2k_pci,netdev=vde0 \ -netdev vde,sock=/tmp/vde1,id=vde1 \ -device ne2k_pci,netdev=vde1 \ -hda vm0.img This works just fine for qemu-system-x86, but not for SPARC with lance NIC: qemu-system-sparc -m 256 \ -netdev vde,sock=/tmp/vde0,id=vde0 -device lance,netdev=vde0 \ -netdev vde,sock=/tmp/vde1,id=vde1 -device lance,netdev=vde1 \ -hda vm0.img qemu-system-sparc: -device lance,netdev=vde0: Parameter 'driver' expects pluggable device type It looks like PCI/sysbus emulation is incomplete/broken on SPARC. Assuming that x86 is OK, what SPARC-specific stuff should I check? It's not that the emulation is broken (it's actually working as expected), it's due to the way in which the BIOS probes sbus devices. On real hardware, the device tree is generated by running bytecode within the ROM contained upon each card by probing a known offset within each slot, which is exactly how the cg3 and tcx cards are detected upon boot. However the since the lance device is on-board, it is hard-coded at a fixed address in both Sun's OBP and OpenBIOS rather than being probed via execution of an external ROM. It should be a fairly trivial exercise to tweak QEMU and OpenBIOS to add a second lance instance at a spare sbus slot address, but as you can see it would require some extra smarts to dynamically patch a compiled ROM within QEMU to insert the relevant properties from the device tree when trying to add another network card via -device. ATB, Mark.
Re: [Qemu-devel] [PATCH v2] virtio: remove useless declaration of virtio_net_init()
arei.gong...@huawei.com writes: From: Gonglei arei.gong...@huawei.com commit 1773d9ee (virtio-net: cleanup: init and exit function) removed the definition of virtio_net_init(), but didn't remove its declaration in the header. Clean that up. Cc: Markus Armbruster arm...@redhat.com Signed-off-by: Gonglei arei.gong...@huawei.com Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] Redundant VDE network
On Tue, Dec 9, 2014 at 6:29 AM, Dmitry Antipov dmanti...@yandex.ru wrote: On 12/08/2014 08:53 PM, Stefan Hajnoczi wrote: Try: qemu-system-sparc -m 256 \ -netdev vde,sock=/tmp/vde0,id=vde0 \ -device ne2k_pci,netdev=vde0 \ -netdev vde,sock=/tmp/vde1,id=vde1 \ -device ne2k_pci,netdev=vde1 \ -hda vm0.img This works just fine for qemu-system-x86, but not for SPARC with lance NIC: qemu-system-sparc -m 256 \ -netdev vde,sock=/tmp/vde0,id=vde0 -device lance,netdev=vde0 \ -netdev vde,sock=/tmp/vde1,id=vde1 -device lance,netdev=vde1 \ -hda vm0.img qemu-system-sparc: -device lance,netdev=vde0: Parameter 'driver' expects pluggable device type It looks like PCI/sysbus emulation is incomplete/broken on SPARC. Assuming that x86 is OK, what SPARC-specific stuff should I check? Is there a reason why you are using the old 32-bit SPARC machine types instead of the more modern 64-bit SPARC machine types? $ sparc-softmmu/qemu-system-sparc -M help Supported machines are: LX Sun4m platform, SPARCstation LX SPARCClassic Sun4m platform, SPARCClassic SPARCbookSun4m platform, SPARCbook SS-10Sun4m platform, SPARCstation 10 SS-20Sun4m platform, SPARCstation 20 SS-4 Sun4m platform, SPARCstation 4 SS-5 Sun4m platform, SPARCstation 5 (default) SS-600MP Sun4m platform, SPARCserver 600MP Voyager Sun4m platform, SPARCstation Voyager leon3_genericLeon-3 generic none empty machine $ sparc64-softmmu/qemu-system-sparc64 -M help Supported machines are: Niagara Sun4v platform, Niagara none empty machine sun4uSun4u platform (default) sun4vSun4v platform I guess the SS-5 default machine type you are running doesn't have a PCI bus? By the way, I have not tested the command-line because Fedora does not have a vde package. You are in niche territory with both SPARC and vde, expect to either debug things yourself or file bugs (they may not be addressed in a timely fashion). Just want to set expectations for what you will find. Stefan
[Qemu-devel] [Bug 1363641] Re: Build of v2.1.0 fails on armv7l due to undeclared __NR_select
Hello Ben, you're completely right on what regards the version on the error message. I'll fix it as soon as possible. Sorry for the trouble on that :( (and sorry for the late reply I was on vacations) Regarding the if statement, as Peter said here -- http://lists.gnu.org/archive/html/qemu-devel/2014-11/msg00960.html -- shell evaluates the OR before the AND, so that's Ok. Thanks. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1363641 Title: Build of v2.1.0 fails on armv7l due to undeclared __NR_select Status in QEMU: New Bug description: After `make clean` and `git clean -x -f -d` `git checkout v2.1.0 configure --prefix=/home/user/prefix-qemu-2.1.0 make` fails due to missing declarations CCqemu-seccomp.o qemu-seccomp.c:28:1: error: '__NR_select' undeclared here (not in a function) qemu-seccomp.c:36:1: error: '__NR_mmap' undeclared here (not in a function) qemu-seccomp.c:57:1: error: '__NR_getrlimit' undeclared here (not in a function) qemu-seccomp.c:96:1: error: '__NR_time' undeclared here (not in a function) GEN qmp-marshal.c qemu-seccomp.c:186:1: error: '__NR_alarm' undeclared here (not in a function) make: *** [qemu-seccomp.o] Error 1 Same errors for master 8b3030114a449e66c68450acaac4b66f26d91416. `configure`should not succeed for a failing build if the error occurs due to missing dependencies, if it's a bug it needs to be fixed. `config.log` for v2.1.0 and 8b303011... attached. The content is mostly compiler output which I think is unusual for `config.log`, but see for yourself. I'm building on a debian 7.6 chroot on Synology DSM 5.0. `uname -a` says `Linux diskstatation 3.2.40 #4493 SMP Thu Aug 21 21:43:02 CST 2014 armv7l GNU/Linux`. After installing some of the missing dependencies, i.e. `apt-get install liblzo2-dev libbsd-dev syslinux-common libhwloc-dev librdmacm- dev libsnappy-dev libibverbs-dev valgrind linux- headers-3.2.0-4-common` I'm getting CCmigration-rdma.o migration-rdma.c: In function 'ram_chunk_start': migration-rdma.c:523:12: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c: In function '__qemu_rdma_add_block': migration-rdma.c:556:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:557:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c: In function '__qemu_rdma_delete_block': migration-rdma.c:664:45: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:699:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c: In function 'qemu_rdma_search_ram_block': migration-rdma.c:1113:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c: In function 'qemu_rdma_register_and_get_keys': migration-rdma.c:1176:50: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c:1177:29: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c:1177:51: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c:1178:29: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c: In function 'qemu_rdma_post_send_control': migration-rdma.c:1562:36: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c: In function 'qemu_rdma_post_recv_control': migration-rdma.c:1616:37: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c: In function 'qemu_rdma_write_one': migration-rdma.c:1864:16: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] migration-rdma.c:1868:53: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:1922:52: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:1923:50: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:1977:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:1998:49: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c:2010:58: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast] migration-rdma.c: In function 'qemu_rdma_registration_handle': migration-rdma.c:3027:21: error: cast from pointer to integer of
Re: [Qemu-devel] qcow2: Can create qcow2 image format on rbd server
On Tue, Dec 9, 2014 at 3:52 AM, Josh Durgin josh.dur...@inktank.com wrote: On 12/08/2014 05:58 AM, Jun Li wrote: On Fri, 12/05 18:01, Max Reitz wrote: On 2014-12-05 at 16:32, Jun Li wrote: Currently, qemu-img can not create qcow2 image format on rbd server. Analysis the code as followings: when create qcow2 format image: qcow2_create2 bdrv_create_file(filename, opts, local_err); -- Here will create a 0 size file(e.g: file1) on rbd server. ... ret = bdrv_pwrite(bs, 0, header, cluster_size); -- So here can not write qcow2 header into file1 due to the file1's length is 0. Seems qemu_rbd_aio_writev can not write beyond EOF. ... As above analysis, there are two methods to solve the above bz as followings: 1, When create file1, just create a fixed-size file1 on rbd server(not 0 size). Should be possible by using -o preallocation=falloc or -o preallocation=full. Although -o preallocation=falloc or -o preallocation=full can create a qcow2 format image successfully, but when perform qemu-img resize file.qcow2 +500M, then use the extend 500M disk image still hit the same issue(as rbd block driver does not support growable file). Why not use 'rbd resize' and raw images instead? rbd already supports snapshots, cloning, thin provisioning, and differential backup natively, so putting qcow2 on top tends to just add overhead. In general, I don't expect many people to use qcow2 on rbd either. However, qcow2 does work on top of iSCSI, LVM, or host block devices and some users actually use this feature (even though there is LVM thin provisioning, for example). So the failure on rbd is unexpected and inconsistent. The way it works on other protocols is that the user must first create an adequately sized volume before running qemu-img create. With rbd this doesn't work because we truncate the volume to 0 bytes during create. Either we need to fix this (without losing the ability to qemu-img create -f raw rbd:... 10G) or we should have a clear error message. The simplest way would just be to detect rbd create with size 0 and print a clear error message like image formats that grow on demand are not supported on rbd. Stefan
Re: [Qemu-devel] suggestion for the QEMU advent calendar
On Tue, Dec 9, 2014 at 9:44 AM, Fabian Greffrath fab...@greffrath.com wrote: Am Dienstag, den 09.12.2014, 10:28 +0100 schrieb Kevin Wolf: Syslinux is relatively easy as well. Just avoid GRUB 2 if you want to set up disk images from the host. I've done it before, but it's ugly... Yep, I was talking about GRUB 2. However, in the meantime I have figured out what to do in order to create a minimal booting image. After all the image creation, partitioning, formatting, device mapping and finally mounting you only have to call grub-install with the right parameters (!), copy the actual kernel into the image and create a minimal grub.cfg file. By right parameters I meant that grub-install per default copies GRUB 2 in its entirety onto the image, including the whole menu system, graphical support, file system drivers and localization, etc. This will take up about 10MB, which is why I put the minimal word in quotation marks. You can, of course, manually select which modules you want to install, but this must be a perfect guess. In my simple case it worked with '--install-modules=part_msdos ext2 multiboot normal --locales=' passed to grub-install on the command line. Anyway, it's easier to just boot the multiboot kernel image and be done with it. ;) The multiboot image is fine. 10 MB for a boot loader? Just wow, at some point it's better to slap a boot sector onto the Linux kernel and be done with it, kexec already exists. Stefan
Re: [Qemu-devel] suggestion for the QEMU advent calendar
Am Dienstag, den 09.12.2014, 09:51 + schrieb Stefan Hajnoczi: The multiboot image is fine. I think so. 10 MB for a boot loader? Just wow, at some point it's better to slap a boot sector onto the Linux kernel and be done with it, kexec already exists. Well, it's a bit more than just a boot loader, but yeah: $ du -h /boot/grub/ 3,2M /boot/grub/locale 2,4M /boot/grub/i386-pc 2,3M /boot/grub/fonts 11M /boot/grub/ - Fabian
Re: [Qemu-devel] [PATCH 1/9] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled
On Mon, 8 Dec 2014 22:57:05 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:00PM +, Igor Mammedov wrote: ACPI parser in XP considers PNP0A06 devices of CPU and memory hotplug as duplicates. Adding unique _UID to CPU hotplug device fixes BSOD. Signed-off-by: Igor Mammedov imamm...@redhat.com And let's add them for memory hotplug as well? XP doesn't support it. Also, if we do stable branch release, we probably want to only do it for memory hotplug in a separate patch, right? This way users who don't enable memory hotplug are unaffected, reduces risk slightly. Memory hotplug device already has _UID, so it doesn't need patching. This patch just fixes BSOD if QEMU has been started with hotplug enabled i.e. for example -m 2G,slots=2,maxmem=4G, and prevents clashing between memory hotplug and cpu hotplug devices on XP (i.e. XP specific quirk). I've tested it with XPsp3 and all later version upto WS2012R2. --- hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index 34aab5a..268d870 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -94,6 +94,7 @@ Scope(\_SB) { Device(CPU_HOTPLUG_RESOURCE_DEVICE) { Name(_HID, EisaId(PNP0A06)) +Name(_UID, CPU hotplug resources) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN) -- 1.8.3.1
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 3/9] pc: acpi-build: cleanup AcpiPmInfo initialization
On Mon, 8 Dec 2014 23:03:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:02PM +, Igor Mammedov wrote: zero initialize AcpiPmInfo struct to reduce code bloat a little bit. Signed-off-by: Igor Mammedov imamm...@redhat.com I generally prefer explicit initialization, but it's a matter of taste. I did this since it saves some LOCs and will save even more later with patches for dynamic _CRS. --- hw/i386/acpi-build.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 1fb92e5..f5ec66a 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -161,6 +161,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm) Object *obj = NULL; QObject *o; +memset(pm, 0, sizeof(*pm)); + if (piix) { obj = piix; } @@ -173,22 +175,16 @@ static void acpi_get_pm_info(AcpiPmInfo *pm) o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL); if (o) { pm-s3_disabled = qint_get_int(qobject_to_qint(o)); -} else { -pm-s3_disabled = false; } qobject_decref(o); o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL); if (o) { pm-s4_disabled = qint_get_int(qobject_to_qint(o)); -} else { -pm-s4_disabled = false; } qobject_decref(o); o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL); if (o) { pm-s4_val = qint_get_int(qobject_to_qint(o)); -} else { -pm-s4_val = false; } qobject_decref(o); -- 1.8.3.1
[Qemu-devel] [PATCH v6 0/3] machvirt dynamic sysbus device instantiation
This patch series enables machvirt to dynamically instantiate sysbus devices from command line (using -device option). All those sysbus devices are plugged onto a platform bus. This latter device is instantiated in machvirt and takes care of the binding of children sysbus devices on a machine init done notifier. The device tree node generation for children dynamic sysbus device also happens on a subsequent notifier that must be executed after the above one. machvirt registers that notifier before the platform bus creation to make sure notifiers are executed in the right order: dt generation after actual QOM binding. Very few sysbus devices are supposed to be instantiated that way. VFIO devices belong to them. Node creation really is architecture specific. On ARM the dynamic sysbus device node creation is implemented in a new C module, hw/arm/sysbus-fdt.c and not in the machine file. Machvirt transformations and sysbus-fdt are largely inspired from Alex work. The patch series can be found at: http://git.linaro.org/people/eric.auger/qemu.git branch vfio_integ_v9_rc6_official_dynsysbus_v6 Best Regards Eric v5 - v6: Take into account Peter's comments: - dtb overload mechanism rewritten: arm_load_kernel code is moved into a machine init done notifier notify instead. arm_load_kernel only registers that notifier. As a consequence the dtb is loaded once. - v5 1-4 patch files are removed and replaced by a single patch file moving arm_load_kernel in the notifier (2). - as a consequence arm_load_kernel must be called before sysbus-fdt arm_register_platform_bus_fdt_creator. - In virt, platform_bus_params not a const anymore since its fields are initialized from vbi-memmap and vbi-irqmap. Hence create_platform_bus proto can be simplified. - In sysbus-fdt add_all_platform_bus_fdt_nodes now takes a handle to an ARMPlatformBusFdtParams. This is not a modify_dtb function anymore fdt pointer is checked in case the callback is called after the load_dtb (this latter deallocated fdt pointer). Check of fdt_filename moved in here. upgrade_dtb is removed. copyright aligned between .h and .c. v4 - v5: - in virt.c: platform_bus_params becomes static const - sysbus-fdt: change indentation in add_fdt_node_functions array init - s/load_dtb/arm_load_dtb in one boot.c comment v3 - v4: - dyn_sysbus_binding removed since binding stuff now are implemented by the platform bus device - due to a change in ARM load_dtb implementation using rom_add_blob_fixed, the dt no more is generated in a reset notifier but is generated on a machine init done notifier - the augmented device tree is not generated from scratch anymore but is added using a modify_dtb function. This required some small change in boot.c - the case where the user provides a dtb file now is handled - some cleanup in virt additions - implement a list of dyanmically instantiable devices in sysbus-fdt v2 - v3: - patch now applies on top of Alex full patchset - dyn_sysbus_devtree: add arm_prefix to emphasize the fact those functions are arm specific; arm_sysbus_device_create_devtree becomes static - load_dtb renamed into arm_load_dtb - add copyright in hw/arm/dyn_sysbus_devtree.c v1 - v2: - device node generation no more in sysbus device but in dyn_sysbus_devtree - VFIO region shrinked to 4MB and relocated in machvirt to avoid PCI shrink (dynamic vfio-mmio support might come latter) - platform_bus_base removed from PlatformDevtreeData Eric Auger (3): hw/arm/sysbus-fdt: helpers for platform bus nodes addition hw/arm/boot: arm_load_kernel implemented as a machine init done notifier hw/arm/virt: add dynamic sysbus device support hw/arm/Makefile.objs| 1 + hw/arm/boot.c | 14 +++- hw/arm/sysbus-fdt.c | 173 hw/arm/virt.c | 97 - include/hw/arm/arm.h| 28 +++ include/hw/arm/sysbus-fdt.h | 60 +++ 6 files changed, 355 insertions(+), 18 deletions(-) create mode 100644 hw/arm/sysbus-fdt.c create mode 100644 include/hw/arm/sysbus-fdt.h -- 1.8.3.2
[Qemu-devel] [PATCH v6 1/3] hw/arm/sysbus-fdt: helpers for platform bus nodes addition
This new C module will be used by ARM machine files to generate platform bus node and their dynamic sysbus device tree nodes. Dynamic sysbus device node addition is done in a machine init done notifier. arm_register_platform_bus_fdt_creator does the registration of this latter and is supposed to be called by ARM machine files that support platform bus and their dynamic sysbus. Addition of dynamic sysbus nodes is done only if the user did not provide any dtb. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Eric Auger eric.au...@linaro.org --- v5 - v6: - add_all_platform_bus_fdt_nodes is not a modify_dtb function anymore - it now takes a handle to an ARMPlatformBusFdtParams. - fdt pointer is checked in case this notifier is executed after the one that executes the load_dtb (this latter deallocates the fdt pointer) - check of fdt_filename moved in here. - upgrade_dtb is removed - copyright aligned between .h and .c v4 - v5: - change indentation in add_fdt_node_functions. Also becomes a static const. - ARMPlatformBusFdtParams.system_params becomes a pointer to a const ARMPlatformBusSystemParams - removes platform-bus.h second inclusion v3 - v4: - dyn_sysbus_devtree.c renamed into sysbus-fdt.c - use new PlatformBusDevice object - the dtb upgrade is done through modify_dtb. Before the fdt was recreated from scratch. When the user provided a dtb this latter was overwritten which was not correct. - an array contains the association between device type names and their node creation function - I must aknowledge I did not find any cleaner way to implement a FDT_BUILDER interface, as suggested by Paolo. The class method would need to be initialized somewhere and since it cannot happen in the device itself - according to Alex Peter comments -, I don't see when I shall associate the device type and its interface implementation. v2 - v3: - add arm_ prefix - arm_sysbus_device_create_devtree becomes static v1 - v2: - Code moved in an arch specific file to accomodate architecture dependent specificities. - remove platform_bus_base from PlatformDevtreeData v1: code originally written by Alex Graf in e500.c and reused for ARM [Eric Auger] --- hw/arm/Makefile.objs| 1 + hw/arm/sysbus-fdt.c | 173 include/hw/arm/sysbus-fdt.h | 60 +++ 3 files changed, 234 insertions(+) create mode 100644 hw/arm/sysbus-fdt.c create mode 100644 include/hw/arm/sysbus-fdt.h diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs index 6088e53..0cc63e1 100644 --- a/hw/arm/Makefile.objs +++ b/hw/arm/Makefile.objs @@ -3,6 +3,7 @@ obj-$(CONFIG_DIGIC) += digic_boards.o obj-y += integratorcp.o kzm.o mainstone.o musicpal.o nseries.o obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o +obj-y += sysbus-fdt.o obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o obj-$(CONFIG_DIGIC) += digic.o diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c new file mode 100644 index 000..843405b --- /dev/null +++ b/hw/arm/sysbus-fdt.c @@ -0,0 +1,173 @@ +/* + * ARM Platform Bus device tree generation helpers + * + * Copyright (c) 2014 Linaro Limited + * + * Authors: + * Alex Graf ag...@suse.de + * Eric Auger eric.au...@linaro.org + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see http://www.gnu.org/licenses/. + * + */ + +#include hw/arm/sysbus-fdt.h +#include qemu/error-report.h +#include sysemu/device_tree.h +#include hw/platform-bus.h +#include sysemu/sysemu.h + +/* + * internal struct that contains the information to create dynamic + * sysbus device node + */ +typedef struct PlatformBusFdtData { +void *fdt; /* device tree handle */ +int irq_start; /* index of the first IRQ usable by platform bus devices */ +const char *pbus_node_name; /* name of the platform bus node */ +PlatformBusDevice *pbus; +} PlatformBusFdtData; + +/* + * struct used when calling the machine init done notifier + * that constructs the fdt nodes of platform bus devices + */ +typedef struct PlatformBusFdtNotifierParams { +ARMPlatformBusFdtParams *fdt_params; +Notifier notifier; +} PlatformBusFdtNotifierParams; + +/* struct that associates a device type name and a node creation function */ +typedef struct NodeCreationPair { +const char *typename; +int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque); +}
Re: [Qemu-devel] [PATCH v5 6/6] hw/arm/virt: add dynamic sysbus device support
On 12/05/2014 05:36 PM, Peter Maydell wrote: On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote: Allows sysbus devices to be instantiated from command line by using -device option. Machvirt creates a platform bus at init. The dynamic sysbus devices are attached to this platform bus device. The platform bus device registers a machine init done notifier whose role will be to bind the dynamic sysbus devices. Indeed dynamic sysbus devices are created after machine init. machvirt also registers a notifier that will build the device tree nodes for the platform bus and its children dynamic sysbus devices. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Eric Auger eric.au...@linaro.org --- v4 - v5: - platform_bus_params becomes static const - reword comment in create_platform_bus - reword the commit message v3 - v4: - use platform bus object, instantiated in create_platform_bus - device tree generation for platform bus and children dynamic sysbus devices is no more handled at reset but in a machine_init_done_notifier (due to the change in implementaion of ARM load dtb using rom_add_blob_fixed). - device tree enhancement now takes into account the case of user provided dtb. Before the user dtb was overwritten which was wrong. However in case the dtb is provided by the user, dynamic sysbus nodes are not added there. - renaming of MACHVIRT_PLATFORM defines - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore, hence removed. - DynSysbusParams struct renamed into ARMPlatformBusSystemParams and above params removed. - separation of dt creation and QEMU binding is not mandated anymore since the device tree is not created from scratch anymore. Instead the modify_dtb function is used. - create_platform_bus registers another machine init done notifier to start VFIO IRQ handling. This latter executes after the dynamic sysbus device binding. v2 - v3: - renaming of arm_platform_bus_create_devtree and arm_load_dtb - add copyright in hw/arm/dyn_sysbus_devtree.c v1 - v2: - remove useless vfio-platform.h include file - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE - use dyn_sysbus_binding and dyn_sysbus_devtree - dynamic sysbus platform buse size shrinked to 4MB and moved between RTC and MMIO v1: Inspired from what Alex Graf did in ppc e500 https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html Conflicts: hw/arm/sysbus-fdt.c --- hw/arm/virt.c | 57 + 1 file changed, 57 insertions(+) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 314e55b..37326a9 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -42,6 +42,8 @@ #include exec/address-spaces.h #include qemu/bitops.h #include qemu/error-report.h +#include hw/arm/sysbus-fdt.h +#include hw/platform-bus.h #define NUM_VIRTIO_TRANSPORTS 32 @@ -59,6 +61,11 @@ #define GIC_FDT_IRQ_PPI_CPU_START 8 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 +#define PLATFORM_BUS_BASE 0x940 +#define PLATFORM_BUS_SIZE (4ULL * 1024 * 1024) +#define PLATFORM_BUS_FIRST_IRQ48 +#define PLATFORM_BUS_NUM_IRQS 20 + enum { VIRT_FLASH, VIRT_MEM, @@ -68,6 +75,7 @@ enum { VIRT_UART, VIRT_MMIO, VIRT_RTC, +VIRT_PLATFORM_BUS, }; typedef struct MemMapEntry { @@ -107,6 +115,7 @@ static const MemMapEntry a15memmap[] = { [VIRT_GIC_CPU] ={ 0x0801, 0x0001 }, [VIRT_UART] = { 0x0900, 0x1000 }, [VIRT_RTC] ={ 0x0901, 0x1000 }, +[VIRT_PLATFORM_BUS] = {PLATFORM_BUS_BASE , PLATFORM_BUS_SIZE}, This makes it pretty hard to read this -- you should use the raw 0x numbers here. Anywhere else that wants to know the base address etc should fish it out of the memory map at runtime, as we do with the other devices. Hi Peter ok. As a side effect platform_bus_params will not be a const anymore, previously recommended by Alex. [VIRT_MMIO] = { 0x0a00, 0x0200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ /* 0x1000 .. 0x4000 reserved for PCI */ @@ -117,6 +126,14 @@ static const int a15irqmap[] = { [VIRT_UART] = 1, [VIRT_RTC] = 2, [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ +[VIRT_PLATFORM_BUS] = PLATFORM_BUS_FIRST_IRQ, Similarly with interrupt numbers. ok Thanks Eric -- PMM
[Qemu-devel] [PATCH v6 2/3] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier
Device tree nodes for the platform bus and its children dynamic sysbus devices are added in a machine init done notifier. To load the dtb once, after those latter nodes are built and before ROM freeze, the actual arm_load_kernel existing code is moved into a notifier notify function, arm_load_kernel_notify. arm_load_kernel now only registers the corresponding notifier. Machine files that do not support platform bus stay unchanged. Machine files willing to support dynamic sysbus devices must call arm_load_kernel before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure dynamic sysbus device nodes are integrated in the dtb. Signed-off-by: Eric Auger eric.au...@linaro.org --- v6: creation of this patch file --- hw/arm/boot.c| 14 +- include/hw/arm/arm.h | 28 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/hw/arm/boot.c b/hw/arm/boot.c index 0014c34..9d8aaa0 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -476,7 +476,7 @@ static void do_cpu_reset(void *opaque) } } -void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) +static void arm_load_kernel_notify(Notifier *notifier, void *data) { CPUState *cs; int kernel_size; @@ -487,6 +487,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) hwaddr entry, kernel_load_offset; int big_endian; static const ARMInsnFixup *primary_loader; +ArmLoadKernelNotifier *n = +container_of(notifier, ArmLoadKernelNotifier, notifier); +ARMCPU *cpu = n-cpu; +struct arm_boot_info *info = +container_of(n, struct arm_boot_info, load_kernel_notifier); /* CPU objects (unlike devices) are not automatically reset on system * reset, so we must always register a handler to do so. If we're @@ -665,3 +670,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) ARM_CPU(cs)-env.boot_info = info; } } + +void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) +{ +info-load_kernel_notifier.cpu = cpu; +info-load_kernel_notifier.notifier.notify = arm_load_kernel_notify; +qemu_add_machine_init_done_notifier(info-load_kernel_notifier.notifier); +} diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h index cefc9e6..88cf80b 100644 --- a/include/hw/arm/arm.h +++ b/include/hw/arm/arm.h @@ -19,6 +19,16 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory, int flash_size, int sram_size, const char *kernel_filename, const char *cpu_model); +/* + * struct used as a parameter of the arm_load_kernel machine init + * done notifier + */ +typedef struct { +ARMCPU *cpu; /* handle to the first cpu object */ +Notifier notifier; /* actual notifier */ +} ArmLoadKernelNotifier; + + /* arm_boot.c */ struct arm_boot_info { uint64_t ram_size; @@ -61,12 +71,30 @@ struct arm_boot_info { * the user it should implement this hook. */ void (*modify_dtb)(const struct arm_boot_info *info, void *fdt); +/* machine init done notifier executing arm_load_dtb */ +ArmLoadKernelNotifier load_kernel_notifier; /* Used internally by arm_boot.c */ int is_linux; hwaddr initrd_start; hwaddr initrd_size; hwaddr entry; }; + +/** + * arm_load_kernel - Loads memory with everything needed to boot + * + * @cpu: handle to the first CPU object + * @info: handle to the boot info struct + * Registers a machine init done notifier that copies to memory + * everything needed to boot, depending on machine and user options: + * kernel image, boot loaders, initrd, dtb. Also registers the CPU + * reset handler. + * + * In case the machine file supports the platform bus device and its + * dynamically instantiable sysbus devices, this function must be called + * before sysbus-fdt arm_register_platform_bus_fdt_creator. Indeed the + * machine init done notifiers are called in registration reverse order. + */ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info); /* Multiplication factor to convert from system clock ticks to qemu timer -- 1.8.3.2
[Qemu-devel] [PATCH v6 3/3] hw/arm/virt: add dynamic sysbus device support
Allows sysbus devices to be instantiated from command line by using -device option. Machvirt creates a platform bus at init. The dynamic sysbus devices are attached to this platform bus device. The platform bus device registers a machine init done notifier whose role will be to bind the dynamic sysbus devices. Indeed dynamic sysbus devices are created after machine init. machvirt also registers a notifier that will build the device tree nodes for the platform bus and its children dynamic sysbus devices. Signed-off-by: Alexander Graf ag...@suse.de Signed-off-by: Eric Auger eric.au...@linaro.org --- v5 - v6: - Take into account Peter's comments: - platform_bus_params initialized from vbi-memmap and vbi-irqmap. As a consequence, const is removed. Also alignment in a15memmap is slightly changed. - ARMPlatformBusSystemParams handle removed from create_platform_bus prototype - arm_load_kernel has become a machine init done notifier registration. It must be called before platform_bus_create to guarantee the correct notifier execution sequence v4 - v5: - platform_bus_params becomes static const - reword comment in create_platform_bus - reword the commit message v3 - v4: - use platform bus object, instantiated in create_platform_bus - device tree generation for platform bus and children dynamic sysbus devices is no more handled at reset but in a machine_init_done_notifier (due to the change in implementaion of ARM load dtb using rom_add_blob_fixed). - device tree enhancement now takes into account the case of user provided dtb. Before the user dtb was overwritten which was wrong. However in case the dtb is provided by the user, dynamic sysbus nodes are not added there. - renaming of MACHVIRT_PLATFORM defines - MACHVIRT_PLATFORM_PAGE_SHIFT and SIZE_PAGES not needed anymore, hence removed. - DynSysbusParams struct renamed into ARMPlatformBusSystemParams and above params removed. - separation of dt creation and QEMU binding is not mandated anymore since the device tree is not created from scratch anymore. Instead the modify_dtb function is used. - create_platform_bus registers another machine init done notifier to start VFIO IRQ handling. This latter executes after the dynamic sysbus device binding. v2 - v3: - renaming of arm_platform_bus_create_devtree and arm_load_dtb - add copyright in hw/arm/dyn_sysbus_devtree.c v1 - v2: - remove useless vfio-platform.h include file - s/MACHVIRT_PLATFORM_HOLE/MACHVIRT_PLATFORM_SIZE - use dyn_sysbus_binding and dyn_sysbus_devtree - dynamic sysbus platform buse size shrinked to 4MB and moved between RTC and MMIO v1: Inspired from what Alex Graf did in ppc e500 https://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00012.html Conflicts: hw/arm/sysbus-fdt.c --- hw/arm/virt.c | 97 --- 1 file changed, 80 insertions(+), 17 deletions(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 314e55b..366083a 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -42,6 +42,8 @@ #include exec/address-spaces.h #include qemu/bitops.h #include qemu/error-report.h +#include hw/arm/sysbus-fdt.h +#include hw/platform-bus.h #define NUM_VIRTIO_TRANSPORTS 32 @@ -59,6 +61,9 @@ #define GIC_FDT_IRQ_PPI_CPU_START 8 #define GIC_FDT_IRQ_PPI_CPU_WIDTH 8 +#define PLATFORM_BUS_FIRST_IRQ48 +#define PLATFORM_BUS_NUM_IRQS 20 + enum { VIRT_FLASH, VIRT_MEM, @@ -68,8 +73,11 @@ enum { VIRT_UART, VIRT_MMIO, VIRT_RTC, +VIRT_PLATFORM_BUS, }; +static ARMPlatformBusSystemParams platform_bus_params; + typedef struct MemMapEntry { hwaddr base; hwaddr size; @@ -100,23 +108,25 @@ typedef struct VirtBoardInfo { */ static const MemMapEntry a15memmap[] = { /* Space up to 0x800 is reserved for a boot ROM */ -[VIRT_FLASH] = { 0, 0x0800 }, -[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 }, +[VIRT_FLASH] = { 0, 0x0800 }, +[VIRT_CPUPERIPHS] = { 0x0800, 0x0002 }, /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */ -[VIRT_GIC_DIST] = { 0x0800, 0x0001 }, -[VIRT_GIC_CPU] ={ 0x0801, 0x0001 }, -[VIRT_UART] = { 0x0900, 0x1000 }, -[VIRT_RTC] ={ 0x0901, 0x1000 }, -[VIRT_MMIO] = { 0x0a00, 0x0200 }, +[VIRT_GIC_DIST] = { 0x0800, 0x0001 }, +[VIRT_GIC_CPU] ={ 0x0801, 0x0001 }, +[VIRT_UART] = { 0x0900, 0x1000 }, +[VIRT_RTC] ={ 0x0901, 0x1000 }, +[VIRT_PLATFORM_BUS] = { 0x0940, 0x0400 }, +[VIRT_MMIO] = { 0x0a00, 0x0200 }, /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */ /* 0x1000 .. 0x4000 reserved for PCI */ -[VIRT_MEM] ={ 0x4000, 30ULL * 1024 * 1024 * 1024 }, +[VIRT_MEM] ={ 0x4000,
Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary
On Mon, 8 Dec 2014 23:15:32 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote: According to ACPI spec NameSeg shorter than 4 characters must be padded up to 4 characters with _ symbol. ACPI 5.0: 20.2.2 Name Objects Encoding Do it in build_append_nameseg() so that caller shouldn't know or care about it. Signed-off-by: Igor Mammedov imamm...@redhat.com To me just doing it right in callers seems just as easy, but I guess you disagree :) That means, author MUST know about padding, if he/she doesn't or forget about it that would introduce error usually resulting in BSOD. This patch allows to avoid such mistakes. --- hw/i386/acpi-build.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f5ec66a..a8b7a2b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, GArray *val) g_array_append_vals(array, val-data, val-len); } +#define ACPI_NAMESEG_LEN 4 + static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char *format, ...) char s[] = ; int len; va_list args; +const char padding = '_'; va_start(args, format); len = vsnprintf(s, sizeof s, format, args); va_end(args); -assert(len == 4); +g_assert(len = ACPI_NAMESEG_LEN); I'm not sure when is g_assert preferable to assert. What's the motivation here? + g_array_append_vals(array, s, len); +while (len != ACPI_NAMESEG_LEN) { +g_array_append_val(array, padding); +++len; +} Easier /* Pad up to 4 characters if necessary. */ g_array_append_vals(array, , 4 - len); } /* 5.4 Definition Block Encoding */ @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { op = 0x82; /* DeviceOp */ -build_append_nameseg(bus_table, S%.02X_, +build_append_nameseg(bus_table, S%.02X, bus-parent_dev-devfn); build_append_byte(bus_table, 0x08); /* NameOp */ build_append_nameseg(bus_table, _SUN); @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_int(notify, 0x1U i); build_append_byte(notify, 0x00); /* NullName */ build_append_byte(notify, 0x86); /* NotifyOp */ -build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0)); +build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0)); build_append_byte(notify, 0x69); /* Arg1Op */ /* Pack it up */ @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { build_append_byte(parent-notify_table, '^'); /* ParentPrefixChar */ build_append_byte(parent-notify_table, 0x2E); /* DualNamePrefix */ -build_append_nameseg(parent-notify_table, S%.02X_, +build_append_nameseg(parent-notify_table, S%.02X, bus-parent_dev-devfn); build_append_nameseg(parent-notify_table, PCNT); } @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker, GArray *sb_scope = build_alloc_array(); uint8_t op = 0x10; /* ScopeOp */ -build_append_nameseg(sb_scope, _SB_); +build_append_nameseg(sb_scope, _SB); /* build Processor object for each processor */ for (i = 0; i acpi_cpus; i++) { -- 1.8.3.1
Re: [Qemu-devel] [PATCH v5 1/6] hw/arm/boot: load_dtb becomes non static arm_load_dtb
On 12/05/2014 07:16 PM, Peter Maydell wrote: On 5 December 2014 at 16:38, Peter Maydell peter.mayd...@linaro.org wrote: On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote: load_dtb is renamed into arm_load_dtb and becomes non static. it will be used by machvirt for dynamic instantiation of platform devices 'virt' shouldn't be a special case -- we should always handle setting up the DTB in guest memory in the same way, whether there happens to be a vfio platform device available or not. ...this probably means that a bunch of the work currently done in arm_load_kernel() should be deferred to a 'machine init complete' hook (perhaps all of it?). Hi Peter, OK I moved the arm_load_kernel code into a machine init done notify and arm_load_kernel now only registers the notifier. For machine files willing to support platform bus, the arm_load_kernel must happen before the registration of the notifier that adds platform bus nodes and after CPU init (notifiers are executed in registration reverse order). Best Regards Eric -- PMM
Re: [Qemu-devel] [PATCH v5 5/6] hw/arm/sysbus-fdt: helpers for platform bus nodes addition
On 12/05/2014 05:40 PM, Peter Maydell wrote: On 30 November 2014 at 18:19, Eric Auger eric.au...@linaro.org wrote: --- /dev/null +++ b/hw/arm/sysbus-fdt.c @@ -0,0 +1,180 @@ +/* + * ARM Platform Bus device tree generation helpers + * + * Copyright (c) 2014 Linaro Limited + * + * Authors: + * Alex Graf ag...@suse.de + * Eric Auger eric.au...@linaro.org + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2 or later, as published by the Free Software Foundation. but diff --git a/include/hw/arm/sysbus-fdt.h b/include/hw/arm/sysbus-fdt.h new file mode 100644 index 000..d4bec85 --- /dev/null +++ b/include/hw/arm/sysbus-fdt.h @@ -0,0 +1,50 @@ +/* + * Dynamic sysbus device tree node generation API + * + * Copyright Linaro Limited, 2014 + * + * Authors: + * Alex Graf ag...@suse.de + * Eric Auger eric.au...@linaro.org + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ ...shouldn't the .c file and the .h file be under the same license? OK copyrights are now aligned Thanks Eric -- PMM
Re: [Qemu-devel] [PATCH 1/9] pc: acpi: fix WindowsXP BSOD when memory hotplug is enabled
On Tue, Dec 09, 2014 at 11:05:37AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 22:57:05 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:00PM +, Igor Mammedov wrote: ACPI parser in XP considers PNP0A06 devices of CPU and memory hotplug as duplicates. Adding unique _UID to CPU hotplug device fixes BSOD. Signed-off-by: Igor Mammedov imamm...@redhat.com And let's add them for memory hotplug as well? XP doesn't support it. Also, if we do stable branch release, we probably want to only do it for memory hotplug in a separate patch, right? This way users who don't enable memory hotplug are unaffected, reduces risk slightly. Memory hotplug device already has _UID, so it doesn't need patching. This patch just fixes BSOD if QEMU has been started with hotplug enabled i.e. for example -m 2G,slots=2,maxmem=4G, and prevents clashing between memory hotplug and cpu hotplug devices on XP (i.e. XP specific quirk). I've tested it with XPsp3 and all later version upto WS2012R2. I see, I misunderstood. Thanks, I'll apply this one. --- hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index 34aab5a..268d870 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -94,6 +94,7 @@ Scope(\_SB) { Device(CPU_HOTPLUG_RESOURCE_DEVICE) { Name(_HID, EisaId(PNP0A06)) +Name(_UID, CPU hotplug resources) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN) -- 1.8.3.1
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH] ide: Implement VPD response for ATAPI
On Thu, Dec 04, 2014 at 01:59:19PM -0500, John Snow wrote: diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index c63b7e5..d27c34b 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -622,19 +622,98 @@ static void cmd_request_sense(IDEState *s, uint8_t *buf) static void cmd_inquiry(IDEState *s, uint8_t *buf) { int max_len = buf[4]; What guarantees do we have about the size of buf in this function? We can't trust max_len because it comes from the guest. +int idx = 0; -buf[0] = 0x05; /* CD-ROM */ -buf[1] = 0x80; /* removable */ -buf[2] = 0x00; /* ISO */ -buf[3] = 0x21; /* ATAPI-2 (XXX: put ATAPI-4 ?) */ -buf[4] = 31; /* additional length */ -buf[5] = 0; /* reserved */ -buf[6] = 0; /* reserved */ -buf[7] = 0; /* reserved */ -padstr8(buf + 8, 8, QEMU); -padstr8(buf + 16, 16, QEMU DVD-ROM); -padstr8(buf + 32, 4, s-version); -ide_atapi_cmd_reply(s, 36, max_len); +/* If the EVPD (Enable Vital Product Data) bit is set in byte 1, + * we are being asked for a specific page of info indicated by byte 2. */ +if (buf[1] 0x01) { +switch (buf[2]) { +case 0x00: +/* Supported Pages */ +buf[idx++] = 0x05; /* CD-ROM */ +buf[idx++] = 0x00; /* Page Code (0x00) */ +buf[idx++] = 0x00; /* reserved */ +buf[idx++] = 0x02; /* Two pages supported: */ +buf[idx++] = 0x00; /* 0x00: Supported Pages, and: */ +buf[idx++] = 0x83; /* 0x83: Device Identification. */ +break; +case 0x83: +/* Device Identification. Each entry is optional, but the entries + * included here are modeled after libata's VPD responses. */ +buf[idx++] = 0x05; /* CD-ROM */ +buf[idx++] = 0x83; /* Page Code */ +buf[idx++] = 0x00; +buf[idx++] = 0x00; /* Length of encapsulated structure: */ + +/* Entry 1: Serial */ +if (idx + 24 max_len) { +/* Not enough room for even the first entry: */ +/* 4 byte header + 20 byte string */ +ide_atapi_cmd_error(s, ILLEGAL_REQUEST, +ASC_DATA_PHASE_ERROR); Missing return +} +buf[idx++] = 0x02; /* Ascii */ +buf[idx++] = 0x00; /* Vendor Specific */ +buf[idx++] = 0x00; +buf[idx++] = 20; /* Remaining length */ +padstr8(buf + idx, 20, s-drive_serial_str); +idx += 20; + +/* Entry 2: Drive Model and Serial */ +if (idx + 72 max_len) { +/* 4 (header) + 8 (vendor) + 60 (model serial) */ +goto out; +} I guess this time it's okay to return early when there is not enough space left due to optional fields? Do we need to set buf[3] (which is currently 0)? pgpg9v5wlC00F.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 5/9] acpi: move generic aml building helpers into dedictated file
On Mon, 8 Dec 2014 22:43:40 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:04PM +, Igor Mammedov wrote: the will be later used for composing AML primitives and all that could be reused later for ARM machines as well. Signed-off-by: Igor Mammedov imamm...@redhat.com It will be easy to move when needed. Why do it now? I'm trying to reduce already huge dynamic AML series, and moving this now is not any way worse than moving it later. Since it should be moved anyway for ARM machines to reuse. --- hw/acpi/Makefile.objs| 1 + hw/acpi/acpi_gen_utils.c | 169 +++ hw/i386/acpi-build.c | 165 +- include/hw/acpi/acpi_gen_utils.h | 23 ++ 4 files changed, 195 insertions(+), 163 deletions(-) create mode 100644 hw/acpi/acpi_gen_utils.c create mode 100644 include/hw/acpi/acpi_gen_utils.h diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs index acd2389..4237232 100644 --- a/hw/acpi/Makefile.objs +++ b/hw/acpi/Makefile.objs @@ -1,3 +1,4 @@ common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o common-obj-$(CONFIG_ACPI) += memory_hotplug.o common-obj-$(CONFIG_ACPI) += acpi_interface.o +common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c new file mode 100644 index 000..1583b35 --- /dev/null +++ b/hw/acpi/acpi_gen_utils.c @@ -0,0 +1,169 @@ +#include stdio.h +#include stdarg.h +#include assert.h +#include stdbool.h +#include hw/acpi/acpi_gen_utils.h + +GArray *build_alloc_array(void) +{ +return g_array_new(false, true /* clear */, 1); +} + +void build_free_array(GArray *array) +{ +g_array_free(array, true); +} + +void build_prepend_byte(GArray *array, uint8_t val) +{ +g_array_prepend_val(array, val); +} + +void build_append_byte(GArray *array, uint8_t val) +{ +g_array_append_val(array, val); +} + +void build_append_array(GArray *array, GArray *val) +{ +g_array_append_vals(array, val-data, val-len); +} + +#define ACPI_NAMESEG_LEN 4 + +void GCC_FMT_ATTR(2, 3) +build_append_nameseg(GArray *array, const char *format, ...) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char s[] = ; +int len; +va_list args; +const char padding = '_'; + +va_start(args, format); +len = vsnprintf(s, sizeof s, format, args); +va_end(args); + +g_assert(len = ACPI_NAMESEG_LEN); + +g_array_append_vals(array, s, len); +while (len != ACPI_NAMESEG_LEN) { +g_array_append_val(array, padding); +++len; +} +} + +/* 5.4 Definition Block Encoding */ +enum { +PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ +PACKAGE_LENGTH_2BYTE_SHIFT = 4, +PACKAGE_LENGTH_3BYTE_SHIFT = 12, +PACKAGE_LENGTH_4BYTE_SHIFT = 20, +}; + +void build_prepend_package_length(GArray *package, unsigned min_bytes) +{ +uint8_t byte; +unsigned length = package-len; +unsigned length_bytes; + +if (length + 1 (1 PACKAGE_LENGTH_1BYTE_SHIFT)) { +length_bytes = 1; +} else if (length + 2 (1 PACKAGE_LENGTH_3BYTE_SHIFT)) { +length_bytes = 2; +} else if (length + 3 (1 PACKAGE_LENGTH_4BYTE_SHIFT)) { +length_bytes = 3; +} else { +length_bytes = 4; +} + +/* Force length to at least min_bytes. + * This wastes memory but that's how bios did it. + */ +length_bytes = MAX(length_bytes, min_bytes); + +/* PkgLength is the length of the inclusive length of the data. */ +length += length_bytes; + +switch (length_bytes) { +case 1: +byte = length; +build_prepend_byte(package, byte); +return; +case 4: +byte = length PACKAGE_LENGTH_4BYTE_SHIFT; +build_prepend_byte(package, byte); +length = (1 PACKAGE_LENGTH_4BYTE_SHIFT) - 1; +/* fall through */ +case 3: +byte = length PACKAGE_LENGTH_3BYTE_SHIFT; +build_prepend_byte(package, byte); +length = (1 PACKAGE_LENGTH_3BYTE_SHIFT) - 1; +/* fall through */ +case 2: +byte = length PACKAGE_LENGTH_2BYTE_SHIFT; +build_prepend_byte(package, byte); +length = (1 PACKAGE_LENGTH_2BYTE_SHIFT) - 1; +/* fall through */ +} +/* + * Most significant two bits of byte zero indicate how many following bytes + * are in PkgLength encoding. + */ +byte = ((length_bytes - 1) PACKAGE_LENGTH_1BYTE_SHIFT) | length; +build_prepend_byte(package, byte); +} + +void build_package(GArray *package, uint8_t op, unsigned min_bytes) +{ +
Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary
On Tue, Dec 09, 2014 at 11:32:45AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 23:15:32 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote: According to ACPI spec NameSeg shorter than 4 characters must be padded up to 4 characters with _ symbol. ACPI 5.0: 20.2.2 Name Objects Encoding Do it in build_append_nameseg() so that caller shouldn't know or care about it. Signed-off-by: Igor Mammedov imamm...@redhat.com To me just doing it right in callers seems just as easy, but I guess you disagree :) That means, author MUST know about padding, if he/she doesn't or forget about it that would introduce error usually resulting in BSOD. Not really. assert will trigger on qemu start. This patch allows to avoid such mistakes. Even the most basic testing (e.g. make check) will find these mistakes. It's more a question of which API we prefer. Anyway, could you respond on g_assert vs assert please? --- hw/i386/acpi-build.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f5ec66a..a8b7a2b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, GArray *val) g_array_append_vals(array, val-data, val-len); } +#define ACPI_NAMESEG_LEN 4 + static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char *format, ...) char s[] = ; int len; va_list args; +const char padding = '_'; va_start(args, format); len = vsnprintf(s, sizeof s, format, args); va_end(args); -assert(len == 4); +g_assert(len = ACPI_NAMESEG_LEN); I'm not sure when is g_assert preferable to assert. What's the motivation here? + g_array_append_vals(array, s, len); +while (len != ACPI_NAMESEG_LEN) { +g_array_append_val(array, padding); +++len; +} Easier /* Pad up to 4 characters if necessary. */ g_array_append_vals(array, , 4 - len); } /* 5.4 Definition Block Encoding */ @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { op = 0x82; /* DeviceOp */ -build_append_nameseg(bus_table, S%.02X_, +build_append_nameseg(bus_table, S%.02X, bus-parent_dev-devfn); build_append_byte(bus_table, 0x08); /* NameOp */ build_append_nameseg(bus_table, _SUN); @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_int(notify, 0x1U i); build_append_byte(notify, 0x00); /* NullName */ build_append_byte(notify, 0x86); /* NotifyOp */ -build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0)); +build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0)); build_append_byte(notify, 0x69); /* Arg1Op */ /* Pack it up */ @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { build_append_byte(parent-notify_table, '^'); /* ParentPrefixChar */ build_append_byte(parent-notify_table, 0x2E); /* DualNamePrefix */ -build_append_nameseg(parent-notify_table, S%.02X_, +build_append_nameseg(parent-notify_table, S%.02X, bus-parent_dev-devfn); build_append_nameseg(parent-notify_table, PCNT); } @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker, GArray *sb_scope = build_alloc_array(); uint8_t op = 0x10; /* ScopeOp */ -build_append_nameseg(sb_scope, _SB_); +build_append_nameseg(sb_scope, _SB); /* build Processor object for each processor */ for (i = 0; i acpi_cpus; i++) { -- 1.8.3.1
Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper
On Mon, 8 Dec 2014 22:21:20 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote: Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding Signed-off-by: Igor Mammedov imamm...@redhat.com Well that's wasteful since most places do have simple 4 byte names. What do you mean under wasteful? How about we replace where this is needed? --- hw/acpi/acpi_gen_utils.c | 86 +++- hw/i386/acpi-build.c | 35 include/hw/acpi/acpi_gen_utils.h | 2 +- 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c index 1583b35..31e2694 100644 --- a/hw/acpi/acpi_gen_utils.c +++ b/hw/acpi/acpi_gen_utils.c @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char *format, ...) } } +static const uint8_t ACPI_DualNamePrefix = 0x2E; +static const uint8_t ACPI_MultiNamePrefix = 0x2F; + +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; +} + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: +g_array_append_val(array, ACPI_DualNamePrefix); +build_append_nameseg(array, %s, s); +build_append_nameseg(array, %s, segs[1]); +break; + +default: +g_array_append_val(array, ACPI_MultiNamePrefix); +assert(seg_count = 0xFF); /* ByteData Max */ +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 870e4a0..0f6202d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) { GArray *method = build_alloc_array(); -build_append_nameseg(method, %s, name); +build_append_namestring(method, %s, name); build_append_byte(method, arg_count); /* MethodFlags: ArgCount */ return method; @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char *name, for (i = 0; i count; i++) { GArray *target = build_alloc_array(); -build_append_nameseg(target, format, i); +build_append_namestring(target, format, i); assert(i 256); /* Fits in 1 byte */ build_append_notify_target_ifequal(method, target, i, 1); build_free_array(target); @@ -693,24 +693,24 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { op = 0x82; /* DeviceOp */ -build_append_nameseg(bus_table, S%.02X, +build_append_namestring(bus_table, S%.02X,
[Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
Booting a minimal KVM guest throws the below error on Cubietruck: kvm_init_vcpu failed: Invalid argument More context and an easy reproducer in this QEMU bug[1] for Fedora. Context quoting Rich Jones from comment #2: For some reason I thought this had been fixed upstream, but now that I've finally got my CT working again, I see that I am still carrying that patch in my custom qemu. diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5ce7350..04d69d1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(cpu-env, ARM_FEATURE_CBAR_RO); set_feature(cpu-env, ARM_FEATURE_LPAE); -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; cpu-midr = 0x412fc0f1; cpu-reset_fpsid = 0x410430f0; cpu-mvfr0 = 0x10110222; So that's the answer really, it's a qemu bug. Actually it looks as if qemu contains some code to try to get the host CPU type, but it doesn't work, or maybe we need to pass a -cpu option ... [1] https://bugzilla.redhat.com/show_bug.cgi?id=1171501 -- Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument -- /kashyap
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote: Booting a minimal KVM guest throws the below error on Cubietruck: kvm_init_vcpu failed: Invalid argument More context and an easy reproducer in this QEMU bug[1] for Fedora. Context quoting Rich Jones from comment #2: For some reason I thought this had been fixed upstream, but now that I've finally got my CT working again, I see that I am still carrying that patch in my custom qemu. diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5ce7350..04d69d1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(cpu-env, ARM_FEATURE_CBAR_RO); set_feature(cpu-env, ARM_FEATURE_LPAE); -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; cpu-midr = 0x412fc0f1; cpu-reset_fpsid = 0x410430f0; cpu-mvfr0 = 0x10110222; This is obviously a bogus patch -- this is the initfn for Cortex-A15 so telling it to be a Cortex-A7 is wrong (and would break working setups on A15 hosts). So that's the answer really, it's a qemu bug. Actually it looks as if qemu contains some code to try to get the host CPU type, but it doesn't work, or maybe we need to pass a -cpu option ... Yes, you need to pass a -cpu option. For KVM on ARM, you always need to either: * pass a -cpu option matching the host CPU * pass -cpu host The CubieTruck is a Cortex-A7, which we don't have specific support for in QEMU, so you will need -cpu host. thanks -- PMM
Re: [Qemu-devel] [PATCH 7/9] acpi: replace opencoded notify codes with named values
On Mon, 8 Dec 2014 22:54:03 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:06PM +, Igor Mammedov wrote: Signed-off-by: Igor Mammedov imamm...@redhat.com I'm not sure this makes sense for these constants. Device Check seems more readable than ACPI_DEV_CHK. If your object here is readability, please do not abbreviate. Generally ability to match spec names exactly is what made me prefer code comments to enums for one-off constants. Looking up Device Check works in any spec version, I don't have to dig up the exact one, find table by name, break my eyes trying to locate the correct line in a huge table. Just text search, and the correct line is highlighted. Why look in another spec version you might ask? Well it's definitely helpful to understand legacy guest quirks. sure, lets drop it. --- hw/i386/acpi-build.c | 4 ++-- include/hw/acpi/acpi_gen_utils.h | 6 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 0f6202d..a33d130 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -840,10 +840,10 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_namestring(method, BNUM); build_append_namestring(method, DVNT); build_append_namestring(method, PCIU); -build_append_int(method, 1); /* Device Check */ +build_append_int(method, ACPI_DEV_CHK); build_append_namestring(method, DVNT); build_append_namestring(method, PCID); -build_append_int(method, 3); /* Eject Request */ +build_append_int(method, ACPI_DEV_EJ); } /* Notify about child bus events in any case */ diff --git a/include/hw/acpi/acpi_gen_utils.h b/include/hw/acpi/acpi_gen_utils.h index fd50625..ce76dc1 100644 --- a/include/hw/acpi/acpi_gen_utils.h +++ b/include/hw/acpi/acpi_gen_utils.h @@ -5,6 +5,12 @@ #include glib.h #include qemu/compiler.h +/* ACPI 5.0: table Device Object Notification Values */ +enum { +ACPI_DEV_CHK = 1, +ACPI_DEV_EJ = 3, +}; + GArray *build_alloc_array(void); void build_free_array(GArray *array); void build_prepend_byte(GArray *array, uint8_t val); -- 1.8.3.1
Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs
Hi PMM, On 5 December 2014 at 19:41, Peter Maydell peter.mayd...@linaro.org wrote: These patches implement support for migration/save/load on AArch64 CPUs. The first one from Alex (with some mangling from me) just moves the sysreg sync code we have for 32-bit across to 64-bit. The second fills in the gaps in the main CPU vmstate struct. Notes: * currently doing 'from cold' incoming migration with -loadvm or -incoming command line arguments is broken if using the VGIC; this is a kernel bug, fixed by this patch: http://www.spinics.net/lists/arm-kernel/msg383736.html * Pranav: patch 1 here is what you need to rebase the bigendian virtio patch on so it can use a single common hook for 32 and 64 bit without hacks to work around missing register sync Thanks for the patch. I have applied this patch and called cpu_synchronize_state() to get sctlr_el1 value in env-cp15.c1_sys during virtio endianness determination while ARM64 guest was booting , but value getting read is 0. Do I need to do anything else ? Thanks, Pranav * Christoffer: patch 1 is also the one which moves reset around Alex Bennée (1): target-arm/kvm: make reg sync code common between kvm32/64 Peter Maydell (1): target-arm: Support save/load for 64 bit CPUs target-arm/kvm.c | 98 target-arm/kvm32.c | 94 ++--- target-arm/kvm64.c | 24 +++-- target-arm/kvm_arm.h | 22 target-arm/machine.c | 22 ++-- 5 files changed, 156 insertions(+), 104 deletions(-) -- 1.9.1
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On Tue, Dec 09, 2014 at 11:50:31AM +0100, Kashyap Chamarthy wrote: Booting a minimal KVM guest throws the below error on Cubietruck: kvm_init_vcpu failed: Invalid argument More context and an easy reproducer in this QEMU bug[1] for Fedora. Context quoting Rich Jones from comment #2: For some reason I thought this had been fixed upstream, but now that I've finally got my CT working again, I see that I am still carrying that patch in my custom qemu. diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5ce7350..04d69d1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(cpu-env, ARM_FEATURE_CBAR_RO); set_feature(cpu-env, ARM_FEATURE_LPAE); -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; cpu-midr = 0x412fc0f1; cpu-reset_fpsid = 0x410430f0; cpu-mvfr0 = 0x10110222; So that's the answer really, it's a qemu bug. Actually it looks as if qemu contains some code to try to get the host CPU type, but it doesn't work, or maybe we need to pass a -cpu option ... Kashyap, Can you try modifying src/launch-direct.c to see if we can pass -cpu cortex-a7 and if that fixes the problem? If that does solve the problem, the question becomes how to detect the right CPU (either cortex-a7, cortex-a15, cortex-a57, ...) This information isn't easily available to libguestfs AFAIK. I really think that qemu should just do the right thing though. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On Tue, Dec 09, 2014 at 10:53:41AM +, Peter Maydell wrote: On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote: Booting a minimal KVM guest throws the below error on Cubietruck: kvm_init_vcpu failed: Invalid argument More context and an easy reproducer in this QEMU bug[1] for Fedora. Context quoting Rich Jones from comment #2: For some reason I thought this had been fixed upstream, but now that I've finally got my CT working again, I see that I am still carrying that patch in my custom qemu. diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5ce7350..04d69d1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(cpu-env, ARM_FEATURE_CBAR_RO); set_feature(cpu-env, ARM_FEATURE_LPAE); -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; cpu-midr = 0x412fc0f1; cpu-reset_fpsid = 0x410430f0; cpu-mvfr0 = 0x10110222; This is obviously a bogus patch -- this is the initfn for Cortex-A15 so telling it to be a Cortex-A7 is wrong (and would break working setups on A15 hosts). Yes, I definitely was not proposing this as a serious patch! In my copy of qemu it has HACK in all capital letters in the summary ... So that's the answer really, it's a qemu bug. Actually it looks as if qemu contains some code to try to get the host CPU type, but it doesn't work, or maybe we need to pass a -cpu option ... Yes, you need to pass a -cpu option. For KVM on ARM, you always need to either: * pass a -cpu option matching the host CPU * pass -cpu host The CubieTruck is a Cortex-A7, which we don't have specific support for in QEMU, so you will need -cpu host. Kashyap ^^ can you try this? Should be a trivial one-liner change in src/launch-direct.c. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
[Qemu-devel] New Bee : What is the canonical path.
Hi, I am new to qemu, And i need help in understanding a part of code, Can any one tell the use of Canonical paths of the Object. Thanks,Sai Pavan
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote: I really think that qemu should just do the right thing though. Tricky, because you have to use the same CPU as the host if you want to use KVM, and that conflicts with wanting to do the same thing on all host platforms for a given set of command line options. If you have some concrete suggestions for improving things that don't break backwards-compatibility I'm happy to hear them, though. -- PMM
Re: [Qemu-devel] [PATCH 0/2] support migration/save/load on AArch64 CPUs
On 9 December 2014 at 11:07, Pranavkumar Sawargaonkar pranavku...@linaro.org wrote: Thanks for the patch. I have applied this patch and called cpu_synchronize_state() to get sctlr_el1 value in env-cp15.c1_sys during virtio endianness determination while ARM64 guest was booting , but value getting read is 0. Do I need to do anything else ? Hmm, on a 64 bit host that should work. (There's an issue with 32-bit hosts that means we won't sync registers properly which I'm working on, but it shouldn't affect 64-bit.) I suggest you debug further to see why the sync doesn't get the right SCTLR value across. -- PMM
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, 9 Dec 2014 12:34:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? It allows to do eject on bridge when it shouldn't. I don't know about consequences of it, it's better not to allow invalid action in the first place. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper
On Tue, Dec 09, 2014 at 11:39:39AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 22:21:20 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote: Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding Signed-off-by: Igor Mammedov imamm...@redhat.com Well that's wasteful since most places do have simple 4 byte names. What do you mean under wasteful? I think I misunderstood what this patch does: you mean select the correct name type based on the string used? I'll have to re-read it, sorry about the noise. How about we replace where this is needed? --- hw/acpi/acpi_gen_utils.c | 86 +++- hw/i386/acpi-build.c | 35 include/hw/acpi/acpi_gen_utils.h | 2 +- 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c index 1583b35..31e2694 100644 --- a/hw/acpi/acpi_gen_utils.c +++ b/hw/acpi/acpi_gen_utils.c @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char *format, ...) } } +static const uint8_t ACPI_DualNamePrefix = 0x2E; +static const uint8_t ACPI_MultiNamePrefix = 0x2F; + +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; +} + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: +g_array_append_val(array, ACPI_DualNamePrefix); +build_append_nameseg(array, %s, s); +build_append_nameseg(array, %s, segs[1]); +break; + +default: +g_array_append_val(array, ACPI_MultiNamePrefix); +assert(seg_count = 0xFF); /* ByteData Max */ +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 870e4a0..0f6202d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) { GArray *method = build_alloc_array(); -build_append_nameseg(method, %s, name); +build_append_namestring(method, %s, name); build_append_byte(method, arg_count); /* MethodFlags: ArgCount */ return method; @@ -565,7 +565,7 @@ build_append_notify_method(GArray *device, const char *name, for (i = 0; i count; i++) { GArray *target = build_alloc_array(); -build_append_nameseg(target, format, i); +build_append_namestring(target, format, i); assert(i 256); /* Fits in 1
[Qemu-devel] [PATCH v2 1/4] Start migrating migration code into a migration directory
From: Dr. David Alan Gilbert dgilb...@redhat.com The migration code now occupies a fair chunk of the top level .c files, it seems time to give it it's own directory. I've not touched: arch_init.c - that's mostly RAM migration but has a few random other bits savevm.c- because it's built target specific This is purely a code move; no code has changed. - it fails checkpatch because of old violations, it feels safer to keep this as purely a move and fix those at some mythical future date. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- Makefile.objs| 10 ++ migration/Makefile.objs | 10 ++ block-migration.c = migration/block-migration.c | 0 migration-exec.c = migration/migration-exec.c | 0 migration-fd.c = migration/migration-fd.c | 0 migration-rdma.c = migration/migration-rdma.c | 0 migration-tcp.c = migration/migration-tcp.c | 0 migration-unix.c = migration/migration-unix.c | 0 migration.c = migration/migration.c | 0 qemu-file-stdio.c = migration/qemu-file-stdio.c | 0 qemu-file-unix.c = migration/qemu-file-unix.c | 0 qemu-file.c = migration/qemu-file.c | 0 vmstate.c = migration/vmstate.c | 0 xbzrle.c = migration/xbzrle.c | 0 tests/Makefile | 6 +++--- 15 files changed, 15 insertions(+), 11 deletions(-) create mode 100644 migration/Makefile.objs rename block-migration.c = migration/block-migration.c (100%) rename migration-exec.c = migration/migration-exec.c (100%) rename migration-fd.c = migration/migration-fd.c (100%) rename migration-rdma.c = migration/migration-rdma.c (100%) rename migration-tcp.c = migration/migration-tcp.c (100%) rename migration-unix.c = migration/migration-unix.c (100%) rename migration.c = migration/migration.c (100%) rename qemu-file-stdio.c = migration/qemu-file-stdio.c (100%) rename qemu-file-unix.c = migration/qemu-file-unix.c (100%) rename qemu-file.c = migration/qemu-file.c (100%) rename vmstate.c = migration/vmstate.c (100%) rename xbzrle.c = migration/xbzrle.c (100%) diff --git a/Makefile.objs b/Makefile.objs index 18fd35c..abeb902 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -48,15 +48,9 @@ common-obj-$(CONFIG_POSIX) += os-posix.o common-obj-$(CONFIG_LINUX) += fsdev/ -common-obj-y += migration.o migration-tcp.o -common-obj-y += vmstate.o -common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o -common-obj-$(CONFIG_RDMA) += migration-rdma.o +common-obj-y += migration/ common-obj-y += qemu-char.o #aio.o -common-obj-y += block-migration.o -common-obj-y += page_cache.o xbzrle.o - -common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o +common-obj-y += page_cache.o common-obj-$(CONFIG_SPICE) += spice-qemu-char.o diff --git a/migration/Makefile.objs b/migration/Makefile.objs new file mode 100644 index 000..63dbe93 --- /dev/null +++ b/migration/Makefile.objs @@ -0,0 +1,10 @@ +common-obj-y += migration.o migration-tcp.o +common-obj-y += vmstate.o +common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o +common-obj-$(CONFIG_RDMA) += migration-rdma.o +common-obj-y += xbzrle.o + +common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o + +common-obj-y += block-migration.o + diff --git a/block-migration.c b/migration/block-migration.c similarity index 100% rename from block-migration.c rename to migration/block-migration.c diff --git a/migration-exec.c b/migration/migration-exec.c similarity index 100% rename from migration-exec.c rename to migration/migration-exec.c diff --git a/migration-fd.c b/migration/migration-fd.c similarity index 100% rename from migration-fd.c rename to migration/migration-fd.c diff --git a/migration-rdma.c b/migration/migration-rdma.c similarity index 100% rename from migration-rdma.c rename to migration/migration-rdma.c diff --git a/migration-tcp.c b/migration/migration-tcp.c similarity index 100% rename from migration-tcp.c rename to migration/migration-tcp.c diff --git a/migration-unix.c b/migration/migration-unix.c similarity index 100% rename from migration-unix.c rename to migration/migration-unix.c diff --git a/migration.c b/migration/migration.c similarity index 100% rename from migration.c rename to migration/migration.c diff --git a/qemu-file-stdio.c b/migration/qemu-file-stdio.c similarity index 100% rename from qemu-file-stdio.c rename to migration/qemu-file-stdio.c diff --git a/qemu-file-unix.c b/migration/qemu-file-unix.c similarity index 100% rename from qemu-file-unix.c rename to migration/qemu-file-unix.c diff --git a/qemu-file.c b/migration/qemu-file.c similarity index 100% rename from qemu-file.c rename to migration/qemu-file.c diff --git a/vmstate.c b/migration/vmstate.c similarity index 100% rename from vmstate.c rename to migration/vmstate.c diff --git a/xbzrle.c
[Qemu-devel] [PATCH v2 2/4] Remove migration- pre/post fixes off files in migration/ dir
From: Dr. David Alan Gilbert dgilb...@redhat.com The general feeling is that having migration/migration-blah is overkill. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- migration/Makefile.objs | 8 migration/{block-migration.c = block.c} | 0 migration/{migration-exec.c = exec.c} | 0 migration/{migration-fd.c = fd.c} | 0 migration/{migration-rdma.c = rdma.c} | 0 migration/{migration-tcp.c = tcp.c} | 0 migration/{migration-unix.c = unix.c} | 0 7 files changed, 4 insertions(+), 4 deletions(-) rename migration/{block-migration.c = block.c} (100%) rename migration/{migration-exec.c = exec.c} (100%) rename migration/{migration-fd.c = fd.c} (100%) rename migration/{migration-rdma.c = rdma.c} (100%) rename migration/{migration-tcp.c = tcp.c} (100%) rename migration/{migration-unix.c = unix.c} (100%) diff --git a/migration/Makefile.objs b/migration/Makefile.objs index 63dbe93..ce1e3c7 100644 --- a/migration/Makefile.objs +++ b/migration/Makefile.objs @@ -1,10 +1,10 @@ -common-obj-y += migration.o migration-tcp.o +common-obj-y += migration.o tcp.o common-obj-y += vmstate.o common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o -common-obj-$(CONFIG_RDMA) += migration-rdma.o common-obj-y += xbzrle.o -common-obj-$(CONFIG_POSIX) += migration-exec.o migration-unix.o migration-fd.o +common-obj-$(CONFIG_RDMA) += rdma.o +common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o -common-obj-y += block-migration.o +common-obj-y += block.o diff --git a/migration/block-migration.c b/migration/block.c similarity index 100% rename from migration/block-migration.c rename to migration/block.c diff --git a/migration/migration-exec.c b/migration/exec.c similarity index 100% rename from migration/migration-exec.c rename to migration/exec.c diff --git a/migration/migration-fd.c b/migration/fd.c similarity index 100% rename from migration/migration-fd.c rename to migration/fd.c diff --git a/migration/migration-rdma.c b/migration/rdma.c similarity index 100% rename from migration/migration-rdma.c rename to migration/rdma.c diff --git a/migration/migration-tcp.c b/migration/tcp.c similarity index 100% rename from migration/migration-tcp.c rename to migration/tcp.c diff --git a/migration/migration-unix.c b/migration/unix.c similarity index 100% rename from migration/migration-unix.c rename to migration/unix.c -- 2.1.0
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On Tue, Dec 09, 2014 at 11:27:02AM +, Richard W.M. Jones wrote: On Tue, Dec 09, 2014 at 10:53:41AM +, Peter Maydell wrote: On 9 December 2014 at 10:50, Kashyap Chamarthy kcham...@redhat.com wrote: Booting a minimal KVM guest throws the below error on Cubietruck: kvm_init_vcpu failed: Invalid argument More context and an easy reproducer in this QEMU bug[1] for Fedora. Context quoting Rich Jones from comment #2: For some reason I thought this had been fixed upstream, but now that I've finally got my CT working again, I see that I am still carrying that patch in my custom qemu. diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 5ce7350..04d69d1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -858,7 +858,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(cpu-env, ARM_FEATURE_CBAR_RO); set_feature(cpu-env, ARM_FEATURE_LPAE); -cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; +cpu-kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A7; cpu-midr = 0x412fc0f1; cpu-reset_fpsid = 0x410430f0; cpu-mvfr0 = 0x10110222; This is obviously a bogus patch -- this is the initfn for Cortex-A15 so telling it to be a Cortex-A7 is wrong (and would break working setups on A15 hosts). Yes, I definitely was not proposing this as a serious patch! In my copy of qemu it has HACK in all capital letters in the summary ... So that's the answer really, it's a qemu bug. Actually it looks as if qemu contains some code to try to get the host CPU type, but it doesn't work, or maybe we need to pass a -cpu option ... Yes, you need to pass a -cpu option. For KVM on ARM, you always need to either: * pass a -cpu option matching the host CPU * pass -cpu host The CubieTruck is a Cortex-A7, which we don't have specific support for in QEMU, so you will need -cpu host. Kashyap ^^ can you try this? Should be a trivial one-liner change in src/launch-direct.c. Yeah, libguestfs build in progress on Cubietruck (it's slow), will test. -- /kashyap
[Qemu-devel] [PATCH v2 0/4] More migration file cleanup
From: Dr. David Alan Gilbert dgilb...@redhat.com This is a small set of patches that shuffle migration code around, but doesn't change the behaviour: 1) Move a lot of the migration source into a separate 'migration' directory Note this moves a lot of files around, git format-patch -M -B spots the renames 2) Now they're in a migration directory, rename them to remove the migration- 3) Split the 'struct QEMUFile' out into a private header Because the QEMU Buffered file code wants to access fields, and it's about to be in a separate file from QEMUFile. 4) Split the QEMU buffered file code out in the same way as the stdio and unix/socket code has been split out. Dave v2: Also move block-migration.c Shorten file names now they're all in the migration/ directory Dr. David Alan Gilbert (4): Start migrating migration code into a migration directory Remove migration- pre/post fixes off files in migration/ dir Split struct QEMUFile out Split the QEMU buffered file code out Makefile.objs| 10 +- migration/Makefile.objs | 10 + block-migration.c = migration/block.c | 0 migration-exec.c = migration/exec.c | 0 migration-fd.c = migration/fd.c | 0 migration.c = migration/migration.c | 0 qemu-file.c = migration/qemu-file-buf.c | 511 +- migration/qemu-file-internal.h | 53 +++ qemu-file-stdio.c = migration/qemu-file-stdio.c | 0 qemu-file-unix.c = migration/qemu-file-unix.c | 0 migration/qemu-file.c| 519 +++ migration-rdma.c = migration/rdma.c | 0 migration-tcp.c = migration/tcp.c | 0 migration-unix.c = migration/unix.c | 0 vmstate.c = migration/vmstate.c | 0 xbzrle.c = migration/xbzrle.c | 0 tests/Makefile | 7 +- 17 files changed, 589 insertions(+), 521 deletions(-) create mode 100644 migration/Makefile.objs rename block-migration.c = migration/block.c (100%) rename migration-exec.c = migration/exec.c (100%) rename migration-fd.c = migration/fd.c (100%) rename migration.c = migration/migration.c (100%) rename qemu-file.c = migration/qemu-file-buf.c (51%) create mode 100644 migration/qemu-file-internal.h rename qemu-file-stdio.c = migration/qemu-file-stdio.c (100%) rename qemu-file-unix.c = migration/qemu-file-unix.c (100%) create mode 100644 migration/qemu-file.c rename migration-rdma.c = migration/rdma.c (100%) rename migration-tcp.c = migration/tcp.c (100%) rename migration-unix.c = migration/unix.c (100%) rename vmstate.c = migration/vmstate.c (100%) rename xbzrle.c = migration/xbzrle.c (100%) -- 2.1.0
[Qemu-devel] [PATCH v2 3/4] Split struct QEMUFile out
From: Dr. David Alan Gilbert dgilb...@redhat.com Now we've got multiple QEMUFile source files, some of them need access to things that were defined in qemu-file.c, so create a -internal header for them. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- migration/qemu-file-internal.h | 53 ++ migration/qemu-file.c | 23 +- 2 files changed, 54 insertions(+), 22 deletions(-) create mode 100644 migration/qemu-file-internal.h diff --git a/migration/qemu-file-internal.h b/migration/qemu-file-internal.h new file mode 100644 index 000..d95e853 --- /dev/null +++ b/migration/qemu-file-internal.h @@ -0,0 +1,53 @@ +/* + * QEMU System Emulator + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef QEMU_FILE_INTERNAL_H +#define QEMU_FILE_INTERNAL_H 1 + +#include qemu-common.h +#include qemu/iov.h + +#define IO_BUF_SIZE 32768 +#define MAX_IOV_SIZE MIN(IOV_MAX, 64) + +struct QEMUFile { +const QEMUFileOps *ops; +void *opaque; + +int64_t bytes_xfer; +int64_t xfer_limit; + +int64_t pos; /* start of buffer when writing, end of buffer +when reading */ +int buf_index; +int buf_size; /* 0 when writing */ +uint8_t buf[IO_BUF_SIZE]; + +struct iovec iov[MAX_IOV_SIZE]; +unsigned int iovcnt; + +int last_error; +}; + +#endif diff --git a/migration/qemu-file.c b/migration/qemu-file.c index f938e36..671fba9 100644 --- a/migration/qemu-file.c +++ b/migration/qemu-file.c @@ -27,30 +27,9 @@ #include block/coroutine.h #include migration/migration.h #include migration/qemu-file.h +#include migration/qemu-file-internal.h #include trace.h -#define IO_BUF_SIZE 32768 -#define MAX_IOV_SIZE MIN(IOV_MAX, 64) - -struct QEMUFile { -const QEMUFileOps *ops; -void *opaque; - -int64_t bytes_xfer; -int64_t xfer_limit; - -int64_t pos; /* start of buffer when writing, end of buffer -when reading */ -int buf_index; -int buf_size; /* 0 when writing */ -uint8_t buf[IO_BUF_SIZE]; - -struct iovec iov[MAX_IOV_SIZE]; -unsigned int iovcnt; - -int last_error; -}; - bool qemu_file_mode_is_not_valid(const char *mode) { if (mode == NULL || -- 2.1.0
[Qemu-devel] [PATCH v2 4/4] Split the QEMU buffered file code out
From: Dr. David Alan Gilbert dgilb...@redhat.com The splitting of qemu-file and addition of the buffered file landed at the same time; so now split the buffered file code out. Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com --- migration/Makefile.objs | 2 +- migration/qemu-file-buf.c | 486 ++ migration/qemu-file.c | 455 --- tests/Makefile| 3 +- 4 files changed, 489 insertions(+), 457 deletions(-) create mode 100644 migration/qemu-file-buf.c diff --git a/migration/Makefile.objs b/migration/Makefile.objs index ce1e3c7..d929e96 100644 --- a/migration/Makefile.objs +++ b/migration/Makefile.objs @@ -1,6 +1,6 @@ common-obj-y += migration.o tcp.o common-obj-y += vmstate.o -common-obj-y += qemu-file.o qemu-file-unix.o qemu-file-stdio.o +common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o common-obj-y += xbzrle.o common-obj-$(CONFIG_RDMA) += rdma.o diff --git a/migration/qemu-file-buf.c b/migration/qemu-file-buf.c new file mode 100644 index 000..d33dd44 --- /dev/null +++ b/migration/qemu-file-buf.c @@ -0,0 +1,486 @@ +/* + * QEMU System Emulator + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include qemu-common.h +#include qemu/iov.h +#include qemu/sockets.h +#include block/coroutine.h +#include migration/migration.h +#include migration/qemu-file.h +#include migration/qemu-file-internal.h +#include trace.h + +#define QSB_CHUNK_SIZE (1 10) +#define QSB_MAX_CHUNK_SIZE (16 * QSB_CHUNK_SIZE) + +/** + * Create a QEMUSizedBuffer + * This type of buffer uses scatter-gather lists internally and + * can grow to any size. Any data array in the scatter-gather list + * can hold different amount of bytes. + * + * @buffer: Optional buffer to copy into the QSB + * @len: size of initial buffer; if @buffer is given, buffer must + * hold at least len bytes + * + * Returns a pointer to a QEMUSizedBuffer or NULL on allocation failure + */ +QEMUSizedBuffer *qsb_create(const uint8_t *buffer, size_t len) +{ +QEMUSizedBuffer *qsb; +size_t alloc_len, num_chunks, i, to_copy; +size_t chunk_size = (len QSB_MAX_CHUNK_SIZE) +? QSB_MAX_CHUNK_SIZE +: QSB_CHUNK_SIZE; + +num_chunks = DIV_ROUND_UP(len ? len : QSB_CHUNK_SIZE, chunk_size); +alloc_len = num_chunks * chunk_size; + +qsb = g_try_new0(QEMUSizedBuffer, 1); +if (!qsb) { +return NULL; +} + +qsb-iov = g_try_new0(struct iovec, num_chunks); +if (!qsb-iov) { +g_free(qsb); +return NULL; +} + +qsb-n_iov = num_chunks; + +for (i = 0; i num_chunks; i++) { +qsb-iov[i].iov_base = g_try_malloc0(chunk_size); +if (!qsb-iov[i].iov_base) { +/* qsb_free is safe since g_free can cope with NULL */ +qsb_free(qsb); +return NULL; +} + +qsb-iov[i].iov_len = chunk_size; +if (buffer) { +to_copy = (len - qsb-used) chunk_size + ? chunk_size : (len - qsb-used); +memcpy(qsb-iov[i].iov_base, buffer[qsb-used], to_copy); +qsb-used += to_copy; +} +} + +qsb-size = alloc_len; + +return qsb; +} + +/** + * Free the QEMUSizedBuffer + * + * @qsb: The QEMUSizedBuffer to free + */ +void qsb_free(QEMUSizedBuffer *qsb) +{ +size_t i; + +if (!qsb) { +return; +} + +for (i = 0; i qsb-n_iov; i++) { +g_free(qsb-iov[i].iov_base); +} +g_free(qsb-iov); +g_free(qsb); +} + +/** + * Get the number of used bytes in the QEMUSizedBuffer + * + * @qsb: A QEMUSizedBuffer + * + * Returns the number of bytes currently used in this buffer + */ +size_t qsb_get_length(const QEMUSizedBuffer *qsb) +{ +return qsb-used; +} + +/** + * Set the length of the buffer; the primary usage of this
Re: [Qemu-devel] [PATCH 0/7] MIPS: IEEE 754-2008 features support
On 9 December 2014 at 12:28, Maciej W. Rozycki ma...@codesourcery.com wrote: On Tue, 9 Dec 2014, Peter Maydell wrote: I really really want to hold this patchset off until the softfloat relicensing has gone through, because repeating that work would be really painful... Understood. Do you have an ETA for this to happen? It's waiting for people to review the relicensing patchset (which includes a couple of reimplementations of functions, so you might want to review and test if you have any handy test setups.) thanks -- PMM
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: On Tue, 9 Dec 2014 12:34:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? It allows to do eject on bridge when it shouldn't. I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a bridge might well work correctly. I don't know about consequences of it, it's better not to allow invalid action in the first place. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary
On Tue, 9 Dec 2014 12:38:12 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:32:45AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 23:15:32 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:03PM +, Igor Mammedov wrote: According to ACPI spec NameSeg shorter than 4 characters must be padded up to 4 characters with _ symbol. ACPI 5.0: 20.2.2 Name Objects Encoding Do it in build_append_nameseg() so that caller shouldn't know or care about it. Signed-off-by: Igor Mammedov imamm...@redhat.com To me just doing it right in callers seems just as easy, but I guess you disagree :) That means, author MUST know about padding, if he/she doesn't or forget about it that would introduce error usually resulting in BSOD. Not really. assert will trigger on qemu start. it sure will, but a bit confusing according to ASL spec name is up to 4 charactes and only AML spec specifies that there should be padding if necessary. I'm trying to get away from constructing AML by hands and the next step for API would be ASL like one which is my eventual target in the end. This patch allows to avoid such mistakes. Even the most basic testing (e.g. make check) will find these mistakes. It's more a question of which API we prefer. Anyway, could you respond on g_assert vs assert please? Ah, I'm sorry about that, haven't noticed comments below. --- hw/i386/acpi-build.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index f5ec66a..a8b7a2b 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, GArray *val) g_array_append_vals(array, val-data, val-len); } +#define ACPI_NAMESEG_LEN 4 + static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char *format, ...) char s[] = ; int len; va_list args; +const char padding = '_'; va_start(args, format); len = vsnprintf(s, sizeof s, format, args); va_end(args); -assert(len == 4); +g_assert(len = ACPI_NAMESEG_LEN); I'm not sure when is g_assert preferable to assert. What's the motivation here? None, I'm not sure what policy on it either I can keep assert here if preferable. + g_array_append_vals(array, s, len); +while (len != ACPI_NAMESEG_LEN) { +g_array_append_val(array, padding); +++len; +} Easier /* Pad up to 4 characters if necessary. */ g_array_append_vals(array, , 4 - len); Thanks, I'll fix it up. } /* 5.4 Definition Block Encoding */ @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { op = 0x82; /* DeviceOp */ -build_append_nameseg(bus_table, S%.02X_, +build_append_nameseg(bus_table, S%.02X, bus-parent_dev-devfn); build_append_byte(bus_table, 0x08); /* NameOp */ build_append_nameseg(bus_table, _SUN); @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) build_append_int(notify, 0x1U i); build_append_byte(notify, 0x00); /* NullName */ build_append_byte(notify, 0x86); /* NotifyOp */ -build_append_nameseg(notify, S%.02X_, PCI_DEVFN(i, 0)); +build_append_nameseg(notify, S%.02X, PCI_DEVFN(i, 0)); build_append_byte(notify, 0x69); /* Arg1Op */ /* Pack it up */ @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) if (bus-parent_dev) { build_append_byte(parent-notify_table, '^'); /* ParentPrefixChar */ build_append_byte(parent-notify_table, 0x2E); /* DualNamePrefix */ -build_append_nameseg(parent-notify_table, S%.02X_, +build_append_nameseg(parent-notify_table, S%.02X, bus-parent_dev-devfn); build_append_nameseg(parent-notify_table, PCNT); } @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker, GArray *sb_scope = build_alloc_array(); uint8_t op = 0x10; /* ScopeOp */ -build_append_nameseg(sb_scope, _SB_); +build_append_nameseg(sb_scope, _SB); /* build Processor object for each processor */ for (i = 0; i acpi_cpus; i++) { -- 1.8.3.1
Re: [Qemu-devel] Redundant VDE network
On 12/09/2014 12:37 PM, Stefan Hajnoczi wrote: Is there a reason why you are using the old 32-bit SPARC machine types instead of the more modern 64-bit SPARC machine types? Unfortunately yes, this is the closest match to the real (legacy) hardware I need to use :-(. I guess the SS-5 default machine type you are running doesn't have a PCI bus? Yes. By the way, I have not tested the command-line because Fedora does not have a vde package. FYI, there is one I build myself: http://37.139.80.10/tmp/vde-2.3.2-0.587.fc20.src.rpm Dmitry
Re: [Qemu-devel] [PATCH 6/9] acpi: add build_append_namestring() helper
On Tue, 9 Dec 2014 14:02:17 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:39:39AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 22:21:20 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:05PM +, Igor Mammedov wrote: Use build_append_namestring() instead of build_append_nameseg() So user won't have to care whether name is NameSeg, NamePath or NameString. See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding Signed-off-by: Igor Mammedov imamm...@redhat.com Well that's wasteful since most places do have simple 4 byte names. What do you mean under wasteful? I think I misunderstood what this patch does: you mean select the correct name type based on the string used? Yep, it chooses appropriate type depending on input. I'll have to re-read it, sorry about the noise. How about we replace where this is needed? --- hw/acpi/acpi_gen_utils.c | 86 +++- hw/i386/acpi-build.c | 35 include/hw/acpi/acpi_gen_utils.h | 2 +- 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c index 1583b35..31e2694 100644 --- a/hw/acpi/acpi_gen_utils.c +++ b/hw/acpi/acpi_gen_utils.c @@ -31,7 +31,7 @@ void build_append_array(GArray *array, GArray *val) #define ACPI_NAMESEG_LEN 4 -void GCC_FMT_ATTR(2, 3) +static void GCC_FMT_ATTR(2, 3) build_append_nameseg(GArray *array, const char *format, ...) { /* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ @@ -53,6 +53,90 @@ build_append_nameseg(GArray *array, const char *format, ...) } } +static const uint8_t ACPI_DualNamePrefix = 0x2E; +static const uint8_t ACPI_MultiNamePrefix = 0x2F; + +static void +build_append_namestringv(GArray *array, const char *format, va_list ap) +{ +/* It would be nicer to use g_string_vprintf but it's only there in 2.22 */ +char *s; +int len; +va_list va_len; +char **segs; +char **segs_iter; +int seg_count = 0; + +va_copy(va_len, ap); +len = vsnprintf(NULL, 0, format, va_len); +va_end(va_len); +len += 1; +s = g_new(typeof(*s), len); + +len = vsnprintf(s, len, format, ap); + +segs = g_strsplit(s, ., 0); +g_free(s); + +/* count segments */ +segs_iter = segs; +while (*segs_iter) { +++segs_iter; +++seg_count; +} + +/* handle RootPath || PrefixPath */ +s = *segs; +while (*s == '\\' || *s == '^') { +g_array_append_val(array, *s); +++s; +} + +switch (seg_count) { +case 1: +if (!*s) { /* NullName */ +const uint8_t nullpath = 0; +g_array_append_val(array, nullpath); +} else { +build_append_nameseg(array, %s, s); +} +break; + +case 2: +g_array_append_val(array, ACPI_DualNamePrefix); +build_append_nameseg(array, %s, s); +build_append_nameseg(array, %s, segs[1]); +break; + +default: +g_array_append_val(array, ACPI_MultiNamePrefix); +assert(seg_count = 0xFF); /* ByteData Max */ +g_array_append_val(array, seg_count); + +/* handle the 1st segment manually due to prefix/root path */ +build_append_nameseg(array, %s, s); + +/* add the rest of segments */ +segs_iter = segs + 1; +while (*segs_iter) { +build_append_nameseg(array, %s, *segs_iter); +++segs_iter; +} +break; +} +g_strfreev(segs); +} + +void GCC_FMT_ATTR(2, 3) +build_append_namestring(GArray *array, const char *format, ...) +{ +va_list ap; + +va_start(ap, format); +build_append_namestringv(array, format, ap); +va_end(ap); +} + /* 5.4 Definition Block Encoding */ enum { PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */ diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 870e4a0..0f6202d 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -273,7 +273,7 @@ static GArray *build_alloc_method(const char *name, uint8_t arg_count) { GArray *method = build_alloc_array(); -build_append_nameseg(method, %s, name); +build_append_namestring(method, %s, name); build_append_byte(method, arg_count); /* MethodFlags: ArgCount */ return method; @@ -565,7 +565,7 @@
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, Dec 09, 2014 at 01:57:51PM +0100, Igor Mammedov wrote: On Tue, 9 Dec 2014 14:51:59 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: On Tue, 9 Dec 2014 12:34:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? It allows to do eject on bridge when it shouldn't. I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a bridge might well work correctly. If I remember correctly SHPC hotplug doesn't need ASL parts at all. Right. And then presumably bridge itself can be removed. I don't know about consequences of it, it's better not to allow invalid action in the first place. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] KVM call for agenda for 2014-12-08
Hello, Is this call still going to happen today? I haven't yet received the contact details. Thanks and best regards.
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
* Peter Maydell (peter.mayd...@linaro.org) wrote: On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote: I really think that qemu should just do the right thing though. Tricky, because you have to use the same CPU as the host if you want to use KVM That's an unusally strict requirement; is there no concept of a common subset that will work on multiple CPUs? Is that still true for v8? I can see it making it tricky for users with piles of newer/older hosts. Dave and that conflicts with wanting to do the same thing on all host platforms for a given set of command line options. If you have some concrete suggestions for improving things that don't break backwards-compatibility I'm happy to hear them, though. -- PMM -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 1/2] s390/pci: add error event support
On Fri, 5 Dec 2014 10:19:58 +0100 Frank Blaschka blasc...@linux.vnet.ibm.com wrote: From: Frank Blaschka frank.blasc...@de.ibm.com This patch adds support to generate s390 pci error events It also uses the new infrastructure for iota error handling (which confused me when I read this), so I modified the patch description to read: This patch adds support to generate s390 pci error events and makes use of it to support iota error handling. Signed-off-by: Frank Blaschka frank.blasc...@de.ibm.com --- hw/s390x/s390-pci-bus.c | 50 +--- hw/s390x/s390-pci-bus.h | 35 +++- hw/s390x/s390-pci-inst.c | 86 ++-- 3 files changed, 155 insertions(+), 16 deletions(-)
Re: [Qemu-devel] [PATCH 0/2] s390/pci: add 2 more features
On Fri, 5 Dec 2014 10:19:57 +0100 Frank Blaschka blasc...@linux.vnet.ibm.com wrote: Coni, Alex, Christian, here are 2 more s390/pci features on top of the base pci support. Thx! Frank Frank Blaschka (2): s390/pci: add error event support s390/pci: implement stpcifc instruction hw/s390x/s390-pci-bus.c | 50 ++-- hw/s390x/s390-pci-bus.h | 36 +++- hw/s390x/s390-pci-inst.c | 148 --- hw/s390x/s390-pci-inst.h | 1 + target-s390x/kvm.c | 9 ++- 5 files changed, 227 insertions(+), 17 deletions(-) Applied 1-2 (patch 1 with slightly modified description) to s390-next.
Re: [Qemu-devel] [PATCH] s390/pci: remove unnecessary cpu_synchronize_state
On Mon, 8 Dec 2014 09:18:59 +0100 Frank Blaschka blasc...@linux.vnet.ibm.com wrote: Remove all unnecessary calls to cpu_synchronize_state Signed-off-by: Frank Blaschka blasc...@linux.vnet.ibm.com --- hw/s390x/s390-pci-inst.c | 6 -- 1 file changed, 6 deletions(-) Applied to s390-next.
Re: [Qemu-devel] KVM call for agenda for 2014-12-08
Alexander Spyridakis a.spyrida...@virtualopensystems.com wrote: Hello, Is this call still going to happen today? I haven't yet received the contact details. Just sent it, it would be in 20 mins. Later, Juan.
[Qemu-devel] KVM call for 2014-12-09
Hi Just a gentle reminder that Today we have a call to continue talking about multithread. Thanks, Juan.
Re: [Qemu-devel] [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0
On Tue, Dec 02, 2014 at 02:00:22PM +0100, Cornelia Huck wrote: virtio-ccw should now have everything in place to operate virtio 1.0 devices, so let's enable revision 1. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com Looks like this will allow revision 1 for all devices, we only want this for virtio 1 devices. The following should fix it I think: Signed-off-by: Michael S. Tsirkin m...@redhat.com diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index d40e3be..f5a1d3e 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -69,9 +69,6 @@ typedef struct VirtIOCCWDeviceClass { int (*exit)(VirtioCcwDevice *dev); } VirtIOCCWDeviceClass; -/* The maximum virtio revision we support. */ -#define VIRTIO_CCW_REV_MAX 1 - /* Performance improves when virtqueue kick processing is decoupled from the * vcpu thread using ioeventfd for some devices. */ #define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1 @@ -104,6 +101,12 @@ struct VirtioCcwDevice { uint64_t ind_bit; }; +/* The maximum virtio revision we support. */ +static int virtio_ccw_rev_max(VirtioCcwDevice *dev) +{ +return dev-host_features (1ULL VIRTIO_F_VERSION_1) ? 1 : 0; +} + /* virtual css bus type */ typedef struct VirtualCssBus { BusState parent_obj; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 3826074..922b021 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -693,7 +693,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } cpu_physical_memory_read(ccw.cda, revinfo, len); if (dev-revision = 0 || -revinfo.revision VIRTIO_CCW_REV_MAX) { +revinfo.revision virtio_ccw_rev_max(dev)) { ret = -ENOSYS; break; }
Re: [Qemu-devel] Cubietruck: cannot create KVM guests: kvm_init_vcpu failed: Invalid argument
On 9 December 2014 at 12:33, Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Peter Maydell (peter.mayd...@linaro.org) wrote: On 9 December 2014 at 11:25, Richard W.M. Jones rjo...@redhat.com wrote: I really think that qemu should just do the right thing though. Tricky, because you have to use the same CPU as the host if you want to use KVM That's an unusally strict requirement; is there no concept of a common subset that will work on multiple CPUs? Is that still true for v8? I can see it making it tricky for users with piles of newer/older hosts. It's a limitation of the kernel implementation -- we don't attempt to trap things like ID registers, so what the guest sees is just the host CPU. The architecture provides the means to trap these and the KVM API allows userspace to ask the kernel to provide an emulation specifically of a particular CPU. But the kernel will currently always refuse to emulate anything other than the host CPU type it's running on. thanks -- PMM
Re: [Qemu-devel] [PATCH 9/9] pc: acpi-build: replace recursive PCI bus tree generation with loop based
On Mon, 8 Dec 2014 22:43:24 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:08PM +, Igor Mammedov wrote: it replaces PCI tree structure in SSDT with a set of scopes describing each PCI bus as a separate scope with a child devices. It makes code easier to follow and a little bit smaller. In addition it makes simplier to convert current template patching approach to completely dynamically generated SSDT without dependency on IASL, which would allow to drop template binary blobs from source tree. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 362 +++ 1 file changed, 165 insertions(+), 197 deletions(-) I like it that your patch makes code smaller and simpler, but I think we do need to generate hierarchical AML. If only reason for hierarchical AML is table size then I don't see how scopes could possibly double the table size, Compared to tree version scope adds only: ~5 bytes for package + NamePath(~[0-2] + 4 * tree depth) Which is almost nothing compared to slots description per a bridge. One needs to configure insanely deep bridge nesting for scope cost to significantly affect table size, which doesn't sound as practical thing to do anyway except of testing limitation on how many nested bridges QEMU would stomach. I think this can still be done with some modifications to the logic you have here. Basically current code does all work in build_pci_bus_end. It follows that you can do the same by simply walking the list in reverse order. I've thought about it already, but tree creation means creating temporary contexts for each scope, copying them to parent context, keeping track of contexts to build correct notifiers. It's basically the same or even worse as current code just other way around. And all that makes code quite hard to follow and maintain, that is the main reason to drop recursion in favor of simple flat scope sets. And it makes easier to switch from AML+template patching to ASL API later and hide from user direct access to table array, forcing him to work only with ASL constructs when building table. diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 97ff245..7606a73 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -641,196 +641,186 @@ static void acpi_set_pci_info(void) } } -static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state, - AcpiBuildPciBusHotplugState *parent, - bool pcihp_bridge_en) +static char *pci_bus_get_scope_name(PCIBus *bus) { -state-parent = parent; -state-device_table = build_alloc_array(); -state-notify_table = build_alloc_array(); -state-pcihp_bridge_en = pcihp_bridge_en; -} +char *name = NULL; +char *last = name; -static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state) -{ -build_free_array(state-device_table); -build_free_array(state-notify_table); -} +while (bus-parent_dev) { +last = name; +name = g_strdup_printf(%s.S%.02X_, + name ? name : , bus-parent_dev-devfn); +g_free(last); -static void *build_pci_bus_begin(PCIBus *bus, void *parent_state) -{ -AcpiBuildPciBusHotplugState *parent = parent_state; -AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child); +bus = bus-parent_dev-bus; +} -build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en); +last = name; +name = g_strdup_printf(PCI0%s, name ? name : ); +g_free(last); -return child; +return name; } -static void build_pci_bus_end(PCIBus *bus, void *bus_state) +static GQueue *get_pci_bus_list(PCIBus *bus, bool pcihp_bridge_en) { -AcpiBuildPciBusHotplugState *child = bus_state; -AcpiBuildPciBusHotplugState *parent = child-parent; -GArray *bus_table = build_alloc_array(); -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX); -DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX); -uint8_t op; -int i; -QObject *bsel; -GArray *method; -bool bus_hotplug_support = false; - -/* - * Skip bridge subtree creation if bridge hotplug is disabled - * to make acpi tables compatible with legacy machine types. - */ -if (!child-pcihp_bridge_en bus-parent_dev) { -return; -} - -if (bus-parent_dev) { -op = 0x82; /* DeviceOp */ -build_append_namestring(bus_table, S%.02X, - bus-parent_dev-devfn); -build_append_byte(bus_table, 0x08); /* NameOp */ -
[Qemu-devel] [PATCH 0/1] Update email addresses for Chrysostomos Nanakos
Remove first email address and let the one from which I am contributing. Chrysostomos Nanakos (1): MAINTAINERS: Update email addresses for Chrysostomos Nanakos MAINTAINERS |1 - 1 file changed, 1 deletion(-) -- 1.7.10.4
Re: [Qemu-devel] [PATCH v2] vt82c686: fix coverity warning about out-of-bounds write
On 09/12/2014 08:15, zhanghailiang wrote: Refactor superio_ioport_writeb to fix the out of bounds write warning. In addition, fix two typos: s/chage/change/ Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com --- - using bool instead of int for can_write as suggested by Stefan Weil - fix two typos: s/chage/change --- hw/isa/vt82c686.c | 41 +++-- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c index e0c235c..6c00f22 100644 --- a/hw/isa/vt82c686.c +++ b/hw/isa/vt82c686.c @@ -50,13 +50,13 @@ typedef struct VT82C686BState { static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data, unsigned size) { -int can_write; SuperIOConfig *superio_conf = opaque; DPRINTF(superio_ioport_writeb address 0x%x val 0x%x\n, addr, data); if (addr == 0x3f0) { superio_conf-index = data 0xff; } else { +bool can_write = true; /* 0x3f1 */ switch (superio_conf-index) { case 0x00 ... 0xdf: @@ -68,30 +68,27 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data, case 0xf7: case 0xf9 ... 0xfb: case 0xfd ... 0xff: -can_write = 0; +can_write = false; break; -default: -can_write = 1; - -if (can_write) { -switch (superio_conf-index) { -case 0xe7: -if ((data 0xff) != 0xfe) { -DPRINTF(chage uart 1 base. unsupported yet\n); -} -break; -case 0xe8: -if ((data 0xff) != 0xbe) { -DPRINTF(chage uart 2 base. unsupported yet\n); -} -break; - -default: -superio_conf-config[superio_conf-index] = data 0xff; -} +case 0xe7: +if ((data 0xff) != 0xfe) { +DPRINTF(change uart 1 base. unsupported yet\n); +can_write = false; +} +break; +case 0xe8: +if ((data 0xff) != 0xbe) { +DPRINTF(change uart 2 base. unsupported yet\n); +can_write = false; } +break; +default: +break; + +} +if (can_write == true) { Michael, can you remove == true when applying this patch? Thanks, Paolo +superio_conf-config[superio_conf-index] = data 0xff; } -superio_conf-config[superio_conf-index] = data 0xff; } }
[Qemu-devel] [PATCH 1/1] MAINTAINERS: Update email addresses for Chrysostomos Nanakos
Remove first email address and let the one from which I am contributing. Signed-off-by: Chrysostomos Nanakos ch...@include.gr --- MAINTAINERS |1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index bcb69e8..a33d8f7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1089,7 +1089,6 @@ S: Supported F: block/ssh.c ARCHIPELAGO -M: Chrysostomos Nanakos cnana...@grnet.gr M: Chrysostomos Nanakos ch...@include.gr S: Maintained F: block/archipelago.c -- 1.7.10.4
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, 9 Dec 2014 14:51:59 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: On Tue, 9 Dec 2014 12:34:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? It allows to do eject on bridge when it shouldn't. I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a bridge might well work correctly. If I remember correctly SHPC hotplug doesn't need ASL parts at all. I don't know about consequences of it, it's better not to allow invalid action in the first place. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH 2/9] pc: acpi: decribe bridge device as not hotpluggable
On Tue, 9 Dec 2014 14:51:59 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 12:45:27PM +0100, Igor Mammedov wrote: On Tue, 9 Dec 2014 12:34:02 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Dec 09, 2014 at 11:27:16AM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 21:13:25 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:01PM +, Igor Mammedov wrote: when bridge hotplug is disabled, i.e. for machine types less then 2.0, bridge device was created as hotpluggable by mistake since commit 133a2da. Fix it by just creating it as a present device. Signed-off-by: Igor Mammedov imamm...@redhat.com What exactly is the problem here? It seems that such bridge is hotpluggable, even though e.g. windows guests lacks drivers to support this. before 133a2da slot with existing at startup bridge weren't marked as hotpluggable nor described in SSDT. But after 133a2da it's described as hotpluggable slot for compat machines (2.0 and lower) which doesn't match with original behavior. Also Marcel mentioned that bridges could be hotpluggable but that they should not be hot-UNpluggable. OK so is there some guest that's confused? What's the bug? It allows to do eject on bridge when it shouldn't. I'm not sure it shouldn't. Assuming guest supports SHPC, eject on such a bridge might well work correctly. Question here is should we keep compat mode as it was for old machine types (i.e. not hot-removable), for new machines bridge is not hot-removable now or just ignore silent change made by 133a2da? I don't know about consequences of it, it's better not to allow invalid action in the first place. --- hw/i386/acpi-build.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..1fb92e5 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -913,7 +913,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) } } -if (!dc-hotpluggable || bridge_in_acpi) { +if (!dc-hotpluggable || pc-is_bridge) { clear_bit(slot, slot_hotplug_enable); } } -- 1.8.3.1
[Qemu-devel] [PATCH 2/5] tpm: Allow 32 16 bit accesses to the registers
Improve the access to the registers with 32 and 16 bit reads and writes. Also enable access to a non-base register address, such as reads of the 2nd byte of a register. Map the FIFO byte access to any byte within its 4 byte register (following specs). Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- hw/tpm/tpm_tis.c | 60 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index c0e7cd7..6170693 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -427,6 +427,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr, uint32_t val = 0x; uint8_t locty = tpm_tis_locality_from_addr(addr); uint32_t avail; +uint8_t v; if (tpm_backend_had_startup_error(s-be_driver)) { return val; @@ -476,14 +477,26 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr, break; case TPM_TIS_REG_DATA_FIFO: if (tis-active_locty == locty) { -switch (tis-loc[locty].state) { -case TPM_TIS_STATE_COMPLETION: -val = tpm_tis_data_read(s, locty); -break; -default: -val = TPM_TIS_NO_DATA_BYTE; -break; +if (size 4 - (addr 0x3)) { +/* prevent access beyond FIFO */ +size = 4 - (addr 0x3); +} +val = 0; +shift = 0; +while (size 0) { +switch (tis-loc[locty].state) { +case TPM_TIS_STATE_COMPLETION: +v = tpm_tis_data_read(s, locty); +break; +default: +v = TPM_TIS_NO_DATA_BYTE; +break; +} +val |= (v shift); +shift += 8; +size--; } +shift = 0; /* no more adjustments */ } break; case TPM_TIS_REG_DID_VID: @@ -518,11 +531,13 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr, { TPMState *s = opaque; TPMTISEmuState *tis = s-s.tis; -uint16_t off = addr 0xfff; +uint16_t off = addr 0xffc; +uint8_t shift = (addr 0x3) * 8; uint8_t locty = tpm_tis_locality_from_addr(addr); uint8_t active_locty, l; int c, set_new_locty = 1; uint16_t len; +uint32_t mask = (size == 1) ? 0xff : ((size == 2) ? 0x : ~0); DPRINTF(tpm_tis: write.%u(%08x) = %08x\n, size, (int)addr, (uint32_t)val); @@ -535,6 +550,15 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr, return; } +val = mask; + +if (shift) { +val = shift; +mask = shift; +} + +mask ^= 0x; + switch (off) { case TPM_TIS_REG_ACCESS: @@ -646,9 +670,10 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr, break; } -tis-loc[locty].inte = (val (TPM_TIS_INT_ENABLED | - TPM_TIS_INT_POLARITY_MASK | - TPM_TIS_INTERRUPTS_SUPPORTED)); +tis-loc[locty].inte = mask; +tis-loc[locty].inte |= (val (TPM_TIS_INT_ENABLED | +TPM_TIS_INT_POLARITY_MASK | +TPM_TIS_INTERRUPTS_SUPPORTED)); break; case TPM_TIS_REG_INT_VECTOR: /* hard wired -- ignore */ @@ -747,16 +772,25 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr, tis-loc[locty].state == TPM_TIS_STATE_COMPLETION) { /* drop the byte */ } else { -DPRINTF(tpm_tis: Byte to send to TPM: %02x\n, (uint8_t)val); +DPRINTF(tpm_tis: Data to send to TPM: %08x (size=%d)\n, +val, size); if (tis-loc[locty].state == TPM_TIS_STATE_READY) { tis-loc[locty].state = TPM_TIS_STATE_RECEPTION; tis-loc[locty].sts = TPM_TIS_STS_EXPECT | TPM_TIS_STS_VALID; } -if ((tis-loc[locty].sts TPM_TIS_STS_EXPECT)) { +val = shift; +if (size 4 - (addr 0x3)) { +/* prevent access beyond FIFO */ +size = 4 - (addr 0x3); +} + +while ((tis-loc[locty].sts TPM_TIS_STS_EXPECT) size 0) { if (tis-loc[locty].w_offset tis-loc[locty].w_buffer.size) { tis-loc[locty].w_buffer. buffer[tis-loc[locty].w_offset++] = (uint8_t)val; +val = 8; +size--; } else { tis-loc[locty].sts = TPM_TIS_STS_VALID; } -- 1.9.3
[Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation
The following series of patches extends the TPM TIS implementation to version 1.3. This will lead to a TIS version that supports TPM 2. For this I would post relatively small patches afterwards. Regards, Stefan Stefan Berger (5): tpm: Extend sts register to 32 bit tpm: Allow 32 16 bit accesses to the registers tpm: Support for XFIFO register tpm: Support for TIS selftest done flag tpm: Support for capability flags of TIS 1.3 hw/tpm/tpm_int.h | 1 + hw/tpm/tpm_passthrough.c | 37 ++-- hw/tpm/tpm_tis.c | 131 ++- hw/tpm/tpm_tis.h | 2 +- include/sysemu/tpm_backend.h | 2 +- 5 files changed, 139 insertions(+), 34 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH 5/5] tpm: Support for capability flags of TIS 1.3
Provide the TIS 1.3 capability flags. The interface now looks like a TIS 1.3 interface. It's fully compatible with previous TIS 1.2 and drivers written for TIS 1.2 continue to work. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- hw/tpm/tpm_tis.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index 61186c5..d0bb97f 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -14,7 +14,7 @@ * * Implementation of the TIS interface according to specs found at * http://www.trustedcomputinggroup.org. This implementation currently - * supports version 1.21, revision 1.0. + * supports version 1.3, 21 March 2013 * In the developers menu choose the PC Client section then find the TIS * specification. */ @@ -103,8 +103,15 @@ #endif +#define TPM_TIS_CAP_INTERFACE_VERSION1_3 (2 28) +#define TPM_TIS_CAP_DATA_TRANSFER_64B(3 9) +#define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 9) +#define TPM_TIS_CAP_BURST_COUNT_DYNAMIC (0 8) #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL (1 4) /* support is mandatory */ #define TPM_TIS_CAPABILITIES_SUPPORTED (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \ + TPM_TIS_CAP_BURST_COUNT_DYNAMIC | \ + TPM_TIS_CAP_DATA_TRANSFER_64B | \ + TPM_TIS_CAP_INTERFACE_VERSION1_3 | \ TPM_TIS_INTERRUPTS_SUPPORTED) #define TPM_TIS_TPM_DID 0x0001 -- 1.9.3
[Qemu-devel] [PATCH 3/5] tpm: Support for XFIFO register
Support for the XFIFO register (range) of the TIS 1.3 specification. We support a range of 64 bytes. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- hw/tpm/tpm_tis.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index 6170693..a37c7ce 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -51,6 +51,8 @@ #define TPM_TIS_REG_INTF_CAPABILITY 0x14 #define TPM_TIS_REG_STS 0x18 #define TPM_TIS_REG_DATA_FIFO 0x24 +#define TPM_TIS_REG_DATA_XFIFO0x80 +#define TPM_TIS_REG_DATA_XFIFO_END0xbc #define TPM_TIS_REG_DID_VID 0xf00 #define TPM_TIS_REG_RID 0xf04 @@ -476,6 +478,7 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr, } break; case TPM_TIS_REG_DATA_FIFO: +case TPM_TIS_REG_DATA_XFIFO ... TPM_TIS_REG_DATA_XFIFO_END: if (tis-active_locty == locty) { if (size 4 - (addr 0x3)) { /* prevent access beyond FIFO */ @@ -762,6 +765,7 @@ static void tpm_tis_mmio_write_intern(void *opaque, hwaddr addr, } break; case TPM_TIS_REG_DATA_FIFO: +case TPM_TIS_REG_DATA_XFIFO ... TPM_TIS_REG_DATA_XFIFO_END: /* data fifo */ if (tis-active_locty != locty) { break; -- 1.9.3
[Qemu-devel] [PATCH 4/5] tpm: Support for TIS selftest done flag
Extend the backend to check whether the TPM_ContinueSelfTest finished successfully and provide a flag to the TIS front-end if it successfully finished. The TIS then sets a flag in all localities in the STS register and keeps it until the next reset. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- hw/tpm/tpm_int.h | 1 + hw/tpm/tpm_passthrough.c | 37 hw/tpm/tpm_tis.c | 58 ++-- include/sysemu/tpm_backend.h | 2 +- 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h index 2f582ca..2b35fe2 100644 --- a/hw/tpm/tpm_int.h +++ b/hw/tpm/tpm_int.h @@ -62,6 +62,7 @@ struct tpm_resp_hdr { #define TPM_FAIL 9 +#define TPM_ORD_ContinueSelfTest 0x53 #define TPM_ORD_GetTicks 0xf1 #endif /* TPM_TPM_INT_H */ diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 56e9e0f..29063cf 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -112,14 +112,31 @@ static void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len) } } +static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len) +{ +struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in; + +if (in_len = sizeof(*hdr)) { +return (be32_to_cpu(hdr-ordinal) == TPM_ORD_ContinueSelfTest); +} + +return false; +} + static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, const uint8_t *in, uint32_t in_len, -uint8_t *out, uint32_t out_len) +uint8_t *out, uint32_t out_len, +bool *selftest_done) { int ret; +bool is_selftest; +const struct tpm_resp_hdr *hdr; tpm_pt-tpm_op_canceled = false; tpm_pt-tpm_executing = true; +*selftest_done = false; + +is_selftest = tpm_passthrough_is_selftest(in, in_len); ret = tpm_passthrough_unix_write(tpm_pt-tpm_fd, in, in_len); if (ret != in_len) { @@ -149,6 +166,11 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, packet from TPM\n); } +if (is_selftest (ret = sizeof(struct tpm_resp_hdr))) { +hdr = (struct tpm_resp_hdr *)out; +*selftest_done = (be32_to_cpu(hdr-errcode) == 0); +} + err_exit: if (ret 0) { tpm_write_fatal_error_response(out, out_len); @@ -160,13 +182,15 @@ err_exit: } static int tpm_passthrough_unix_transfer(TPMPassthruState *tpm_pt, - const TPMLocality *locty_data) + const TPMLocality *locty_data, + bool *selftest_done) { return tpm_passthrough_unix_tx_bufs(tpm_pt, locty_data-w_buffer.buffer, locty_data-w_offset, locty_data-r_buffer.buffer, -locty_data-r_buffer.size); +locty_data-r_buffer.size, +selftest_done); } static void tpm_passthrough_worker_thread(gpointer data, @@ -175,16 +199,19 @@ static void tpm_passthrough_worker_thread(gpointer data, TPMPassthruThreadParams *thr_parms = user_data; TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(thr_parms-tb); TPMBackendCmd cmd = (TPMBackendCmd)data; +bool selftest_done = false; DPRINTF(tpm_passthrough: processing command type %d\n, cmd); switch (cmd) { case TPM_BACKEND_CMD_PROCESS_CMD: tpm_passthrough_unix_transfer(tpm_pt, - thr_parms-tpm_state-locty_data); + thr_parms-tpm_state-locty_data, + selftest_done); thr_parms-recv_data_callback(thr_parms-tpm_state, - thr_parms-tpm_state-locty_number); + thr_parms-tpm_state-locty_number, + selftest_done); break; case TPM_BACKEND_CMD_INIT: case TPM_BACKEND_CMD_END: diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index a37c7ce..61186c5 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -64,6 +64,7 @@ #define TPM_TIS_STS_TPM_GO(1 5) #define TPM_TIS_STS_DATA_AVAILABLE(1 4) #define TPM_TIS_STS_EXPECT(1 3) +#define TPM_TIS_STS_SELFTEST_DONE (1 2) #define TPM_TIS_STS_RESPONSE_RETRY(1 1) #define TPM_TIS_BURST_COUNT_SHIFT 8 @@ -147,6 +148,24 @@ static void tpm_tis_show_buffer(const TPMSizedBuffer *sb, const char *string) } /* + * Set the given flags in the STS register by clearing the register but + * preserving the SELFTEST_DONE flag
[Qemu-devel] [PATCH 1/5] tpm: Extend sts register to 32 bit
More recent TIS specs extend the STS register to 32 bit. While we don't store the TIS interface state, yet, we can extend it without sideeffects. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- hw/tpm/tpm_tis.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h index 1a0db23..db78d51 100644 --- a/hw/tpm/tpm_tis.h +++ b/hw/tpm/tpm_tis.h @@ -41,7 +41,7 @@ typedef enum { typedef struct TPMLocality { TPMTISState state; uint8_t access; -uint8_t sts; +uint32_t sts; uint32_t inte; uint32_t ints; -- 1.9.3
Re: [Qemu-devel] [PATCH 9/9] pc: acpi-build: replace recursive PCI bus tree generation with loop based
On Tue, Dec 09, 2014 at 03:01:00PM +0100, Igor Mammedov wrote: On Mon, 8 Dec 2014 22:43:24 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Dec 08, 2014 at 04:08:08PM +, Igor Mammedov wrote: it replaces PCI tree structure in SSDT with a set of scopes describing each PCI bus as a separate scope with a child devices. It makes code easier to follow and a little bit smaller. In addition it makes simplier to convert current template patching approach to completely dynamically generated SSDT without dependency on IASL, which would allow to drop template binary blobs from source tree. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/acpi-build.c | 362 +++ 1 file changed, 165 insertions(+), 197 deletions(-) I like it that your patch makes code smaller and simpler, but I think we do need to generate hierarchical AML. If only reason for hierarchical AML is table size then I don't see how scopes could possibly double the table size, Compared to tree version scope adds only: ~5 bytes for package + NamePath(~[0-2] + 4 * tree depth) No, tree depth squared since you generate this per bridge. Which is almost nothing compared to slots description per a bridge. One needs to configure insanely deep bridge nesting for scope cost to significantly affect table size, which doesn't sound as practical thing to do anyway except of testing limitation on how many nested bridges QEMU would stomach. pci supports up to 256 nested bridges. So that's 256K which isn't the end of the world but not negligeable for a bios. I think this can still be done with some modifications to the logic you have here. Basically current code does all work in build_pci_bus_end. It follows that you can do the same by simply walking the list in reverse order. I've thought about it already, but tree creation means creating temporary contexts for each scope, copying them to parent context, keeping track of contexts to build correct notifiers. It's basically the same or even worse as current code just other way around. Well, the way I see it, the main simplification is that you can pass data from child to parent explicitly. Maybe I'm not explaining this properly, I'll try to find the time to send a patch for comparison. And all that makes code quite hard to follow and maintain, that is the main reason to drop recursion in favor of simple flat scope sets. I agree the approach of unrolling the tree, then walking the list has advantages, but if it's forcing a specific AML structure then that really means it's too limited. And it makes easier to switch from AML+template patching to ASL API later and hide from user direct access to table array, forcing him to work only with ASL constructs when building table. I don't necessarily see how these two ideas are related. diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 97ff245..7606a73 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -641,196 +641,186 @@ static void acpi_set_pci_info(void) } } -static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state, - AcpiBuildPciBusHotplugState *parent, - bool pcihp_bridge_en) +static char *pci_bus_get_scope_name(PCIBus *bus) { -state-parent = parent; -state-device_table = build_alloc_array(); -state-notify_table = build_alloc_array(); -state-pcihp_bridge_en = pcihp_bridge_en; -} +char *name = NULL; +char *last = name; -static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state) -{ -build_free_array(state-device_table); -build_free_array(state-notify_table); -} +while (bus-parent_dev) { +last = name; +name = g_strdup_printf(%s.S%.02X_, + name ? name : , bus-parent_dev-devfn); +g_free(last); -static void *build_pci_bus_begin(PCIBus *bus, void *parent_state) -{ -AcpiBuildPciBusHotplugState *parent = parent_state; -AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child); +bus = bus-parent_dev-bus; +} -build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en); +last = name; +name = g_strdup_printf(PCI0%s, name ? name : ); +g_free(last); -return child; +return name; } -static void build_pci_bus_end(PCIBus *bus, void *bus_state) +static GQueue *get_pci_bus_list(PCIBus *bus, bool pcihp_bridge_en) { -AcpiBuildPciBusHotplugState *child = bus_state; -AcpiBuildPciBusHotplugState *parent = child-parent; -GArray *bus_table = build_alloc_array(); -DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX); -
Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation
On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote: The following series of patches extends the TPM TIS implementation to version 1.3. This will lead to a TIS version that supports TPM 2. For this I would post relatively small patches afterwards. Regards, Stefan Since this is guest visible, should this be limited to new machine types, to avoid breaking migrating guests across hypervisor versions? Stefan Berger (5): tpm: Extend sts register to 32 bit tpm: Allow 32 16 bit accesses to the registers tpm: Support for XFIFO register tpm: Support for TIS selftest done flag tpm: Support for capability flags of TIS 1.3 hw/tpm/tpm_int.h | 1 + hw/tpm/tpm_passthrough.c | 37 ++-- hw/tpm/tpm_tis.c | 131 ++- hw/tpm/tpm_tis.h | 2 +- include/sysemu/tpm_backend.h | 2 +- 5 files changed, 139 insertions(+), 34 deletions(-) -- 1.9.3
Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation
On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote: On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote: The following series of patches extends the TPM TIS implementation to version 1.3. This will lead to a TIS version that supports TPM 2. For this I would post relatively small patches afterwards. Regards, Stefan Since this is guest visible, should this be limited to new machine types, to avoid breaking migrating guests across hypervisor versions? Migration is not currently possible with TPM, since TPM (passthrough) is preventing it. Stefan
Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation
On Tue, Dec 09, 2014 at 09:40:28AM -0500, Stefan Berger wrote: On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote: On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote: The following series of patches extends the TPM TIS implementation to version 1.3. This will lead to a TIS version that supports TPM 2. For this I would post relatively small patches afterwards. Regards, Stefan Since this is guest visible, should this be limited to new machine types, to avoid breaking migrating guests across hypervisor versions? Migration is not currently possible with TPM, since TPM (passthrough) is preventing it. Stefan OK, for live migration, fair enough. For off-line migration: will the following still work: - boot guest with tpm 1.3. use tpm in some way - halt, start guest on old qemu with tpm 1.2 ?
Re: [Qemu-devel] [PATCH 0/5] tpm: Extend the TPM TIS implementation
On 12/09/2014 09:45 AM, Michael S. Tsirkin wrote: On Tue, Dec 09, 2014 at 09:40:28AM -0500, Stefan Berger wrote: On 12/09/2014 09:35 AM, Michael S. Tsirkin wrote: On Tue, Dec 09, 2014 at 09:18:54AM -0500, Stefan Berger wrote: The following series of patches extends the TPM TIS implementation to version 1.3. This will lead to a TIS version that supports TPM 2. For this I would post relatively small patches afterwards. Regards, Stefan Since this is guest visible, should this be limited to new machine types, to avoid breaking migrating guests across hypervisor versions? Migration is not currently possible with TPM, since TPM (passthrough) is preventing it. Stefan OK, for live migration, fair enough. For off-line migration: will the following still work: - boot guest with tpm 1.3. use tpm in some way - halt, start guest on old qemu with tpm 1.2 ? So this is not a suspend resume operation ? Sure, it will work if the driver the OS is using works with a TPM TIS 1.3 interface and also with a TPM TIS 1.2 interface. Stefan
Re: [Qemu-devel] [PATCH v3] qdev: Avoid type assertion in qdev_build_hotpluggable_device_list()
Am 08.12.2014 um 15:07 schrieb Jun Li: Ping, why does this patch has not been merged until now? Could anyone give some explanations? Thx. I had already applied the previous version of the patch to qom-next. I'll drop it and replace it with this version then. Regards, Andreas On Wed, 11/05 15:03, Jun Li wrote: Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable device list. This patch also fixed the following issue: 1, boot qemu using cli: $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \ -device virtio-scsi-pci,id=scsi0 2, device_del scsi0 via hmp using tab key(first input device_del, then press Tab key). (qemu) device_del After step2, qemu will abort. (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list: Object 0x563a2460 is not an instance of type device Signed-off-by: Jun Li junm...@gmail.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com --- v3: According to Andreas's suggestion, do some changes. As followings: 1, change the Subject to more meaningful. 2, use two return 0 to return early avoid reindentation. And I have found in qcow2_grow_l1_table has also used two return 0. So accept Andreas's suggestion. Thanks. v2: This version just do a little changes for the commit message. As following show: In v1, 1, boot qemu using cli: virtio-scsi-pci,id=scsi0 -enable-kvm In v2, 1, boot qemu using cli: $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \ -device virtio-scsi-pci,id=scsi0 --- hw/core/qdev.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index b3d5196..7db3e13 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -927,7 +927,12 @@ void qdev_alias_all_properties(DeviceState *target, Object *source) int qdev_build_hotpluggable_device_list(Object *obj, void *opaque) { GSList **list = opaque; -DeviceState *dev = DEVICE(obj); +DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj), + device); + +if (dev == NULL) { +return 0; +} if (dev-realized object_property_get_bool(obj, hotpluggable, NULL)) { *list = g_slist_append(*list, dev); -- 1.9.3 -- SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
Re: [Qemu-devel] [PATCH] qmp: extend QMP to provide read/write access to physical memory
By evidence, I mean actual numbers for actual QEMU code. Nothing sophisticated, just use your new interface in a way you consider relevant for your own use case, then approximate this use with existing interfaces. The approximation can be very rough. For instance, showing that doing the whole job with your approach is a much faster than doing a necessary part of the job with existing commands would do. Sure, I can put together some numbers to help with this discussion. The QMP overhead could be too high. QMP is control plane, not data plane. How much data do you envisage flowing here? In theory, even the character device overhead could be too high. Measuring it shouldn't be too hard. This discussion has me thinking about whether QMP would be viable. I think I'll take a little time to explore that approach in a little more depth before proceeding here. I can report back with what I find. If you control a QMP monitor, you own the guest. Good point. So I'll explore the performance aspects and let that drive the decision. I feel code comment is find for internal interfaces, but for external interfaces, a separate interface document is more appropriate. Sounds good. Cheers, -bryan
Re: [Qemu-devel] [PATCH v2] Block migration: fix return value
On Tue, Nov 25, 2014 at 05:30:02PM -0600, grhookatw...@gmail.com wrote: From: Gary R Hook gary.h...@nimboxx.com Modify block_save_iterate() to return positive/zero/negative (success/not done/failure) return status. The computation of the blocks transferred (an int64_t) exceeds the size of an int return value. Signed-off-by: Gary R Hook gary.h...@nimboxx.com --- block-migration.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) Thanks, applied to my block-next tree: https://github.com/stefanha/qemu/commits/block-next Stefan pgpc9EA4vVOfw.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] nvme: 64kB page size fixes
On Tue, Dec 02, 2014 at 09:18:59PM +0100, Paolo Bonzini wrote: On 27/11/2014 04:39, Anton Blanchard wrote: Initialise our maximum page size capability to 64kB and increase the page_size variable from 16 to 32 bits. Signed-off-by: Anton Blanchard an...@samba.org -- diff --git a/hw/block/nvme.c b/hw/block/nvme.c index 1327658..aa1ed98 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -811,6 +811,7 @@ static int nvme_init(PCIDevice *pci_dev) NVME_CAP_SET_AMS(n-bar.cap, 1); NVME_CAP_SET_TO(n-bar.cap, 0xf); NVME_CAP_SET_CSS(n-bar.cap, 1); +NVME_CAP_SET_MPSMAX(n-bar.cap, 4); n-bar.vs = 0x00010001; n-bar.intmc = n-bar.intms = 0; diff --git a/hw/block/nvme.h b/hw/block/nvme.h index 993c511..b6ccb65 100644 --- a/hw/block/nvme.h +++ b/hw/block/nvme.h @@ -688,7 +688,7 @@ typedef struct NvmeCtrl { NvmeBar bar; BlockConfconf; -uint16_tpage_size; +uint32_tpage_size; uint16_tpage_bits; uint16_tmax_prp_ents; uint16_tcqe_size; This should probably be a property of the device instead. If you want to change the default, you need to preserve a backwards-compatible value for pre-2.3 machine types (-M pc-i440fx-2.2, -M pc-i440fx-2.1 etc.). Hi Anton, Any update on this? Paolo has a point that changing this value would make a guest-visible hardware change. Therefore older machine types, which users might rely on, should continue to show the old value. This is especially important with live migration because hardware should not change underneath the guest due to migration. Stefan pgp3F0WRrpdGt.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3] qdev: Avoid type assertion in qdev_build_hotpluggable_device_list()
Am 05.11.2014 um 08:03 schrieb Jun Li: Currently when *obj is not a TYPE_DEVICE, qemu will abort. This patch just fixed it. When *obj is not a TYPE_DEVICE, just do not add it to hotpluggable device list. This patch also fixed the following issue: 1, boot qemu using cli: $ /opt/qemu-git-arm/bin/qemu-system-x86_64 -monitor stdio -enable-kvm \ -device virtio-scsi-pci,id=scsi0 2, device_del scsi0 via hmp using tab key(first input device_del, then press Tab key). (qemu) device_del After step2, qemu will abort. (qemu) device_del hw/core/qdev.c:930:qdev_build_hotpluggable_device_list: Object 0x563a2460 is not an instance of type device Signed-off-by: Jun Li junm...@gmail.com Reviewed-by: Paolo Bonzini pbonz...@redhat.com --- v3: According to Andreas's suggestion, do some changes. As followings: 1, change the Subject to more meaningful. 2, use two return 0 to return early avoid reindentation. And I have found in qcow2_grow_l1_table has also used two return 0. So accept Andreas's suggestion. Thanks. Thanks, replaced the queued version on qom-next with a few edits and the following change: https://github.com/afaerber/qemu-cpu/commits/qom-next diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 51ab59e..ef06aa4 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -939,7 +939,7 @@ static int qdev_add_hotpluggable_device(Object *obj, void *opaque) { GSList **list = opaque; DeviceState *dev = (DeviceState *)object_dynamic_cast(OBJECT(obj), - device); + TYPE_DEVICE); if (dev == NULL) { return 0; Regards, Andreas -- SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 21284 AG Nürnberg
Re: [Qemu-devel] about tracetool
On Tue, Dec 02, 2014 at 09:46:50AM +0900, Ady Wahyudi Paundu wrote: I know that simpletrace records go to memory buffer before it flushed to file by a writer thread. My question is how to call this thread (preferably using python), so I can make the flush process periodical? Look at the source code: trace/simple.c Why do you want to do this? Stefan pgpBjnddPk6v8.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH for-2.3 0/2] block: Fix for trailing whitespace in qemu-img create and its tests
On 12/09/2014 12:38 AM, Fam Zheng wrote: This will make it easier to keep checkpatch.pl happy. Fam Zheng (2): qemu-iotests: Remove traling whitespaces in *.out block: Don't add trailing space in Formating... message Series: Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] qmp-command.hx: add missing docs for migration capabilites
On 12/08/2014 11:38 PM, zhanghailiang wrote: Signed-off-by: zhanghailiang zhang.zhanghaili...@huawei.com --- qmp-commands.hx | 6 ++ 1 file changed, 6 insertions(+) - xbzrle: XBZRLE support +- rdma-pin-all: pin all pages when use of RDMA during migration s/use of/using/ With the grammar fix, Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board
On 12/09/14 10:31, Gerd Hoffmann wrote: Hi, So... after playing with this thing for some time, it's become clear that MMIO traps are painfully slow on the aarch64 platform we've been working on (using KVM). So, as we don't have compatibility requirements, and we also can't play tricks like using x86 string instructions: How about a completely different, dma-style interface for fw_cfg access? One register for the (physical) target address. One register for the transfer size. One register for the fw_cfg entry. Possibly one register for the fw_cfg offset (not really needed, but avoids the need for side effects such as writing fw_cfg entry register clearing the offset). One register to kick the transfer. Again, this was the idea that Rich had in 2010 (see the links in the discussion thus far). It was rejected back then (which is why I didn't even try to resurrect it now), and Peter has now asked if anything has changed that would make that approach more acceptable now. Avi and Gleb not hanging around so they can't block the approach now might or might not be such a change; I don't know. But, I tend to trust their opinion, even if dates back to 2010 in this case. Anyway I'm certainly not opposed to performance, so if someone can thoroughly refute everything they said in that thread, go ahead. Thanks! Laszlo
Re: [Qemu-devel] [RFC PATCH V2 1/4] block: add accounting for merged requests
On 05.12.2014 15:58, Eric Blake wrote: On 12/05/2014 04:50 AM, Peter Lieven wrote: Signed-off-by: Peter Lieven p...@kamp.de --- block.c|2 ++ block/accounting.c |7 +++ block/qapi.c |2 ++ hmp.c |6 +- include/block/accounting.h |3 +++ qapi/block-core.json |9 - qmp-commands.hx| 22 ++ 7 files changed, 45 insertions(+), 6 deletions(-) @@ -2284,6 +2288,8 @@ Example: rd_total_times_ns:3465673657 flush_total_times_ns:49653 flush_operations:61, + rd_merged:0, + wr_merged:0 Oh my - we previously had invalid JSON in our example. Yay that this fixes it :) There is actually more invalid JSON, but I would fix that in a separate patch apart from this series. Peter
Re: [Qemu-devel] [RFC PATCH V2 4/4] virtio-blk: introduce multiread
On 05.12.2014 16:05, Eric Blake wrote: On 12/05/2014 04:50 AM, Peter Lieven wrote: this patch finally introduce multiread support to virtio-blk while s/introduce/introduces/ s/virtio-blk while/virtio-blk. While/ multiwrite support was there for a long time read support was missing. s/time/time,/ To achieve this the patch does serveral things which might need futher s/serveral/several/ s/futher/further/ explaination: s/explaination/explanation/ - the whole merge and multireq logic is moved from block.c into virtio-blk. This is move is a preparation for directly creating a coroutine out of virtio-blk. Can this move be done as a separate prerequisite patch? Mixing code motion and new features in the same patch is harder to review. The issue is that the code is not movable. I could artifically use the new code in a new patch only for write requests and add another patch to add read merging. But I doubt that it would make it easier to review but even harder. Peter
[Qemu-devel] [Bug 1400768] [NEW] Fatal error when running with '-machine isapc' on 2.1.2
Public bug reported: all I have are the traces, should hopefully be easy to reproduce. # qemu-system-i386 -machine isapc VNC server running on `::1:5900' qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44 EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014 EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0010 00cf9300 DPL=0 DS [-WA] CS =0008 00cf9b00 DPL=0 CS32 [-RA] SS =0010 00cf9300 DPL=0 DS [-WA] DS =0010 00cf9300 DPL=0 DS [-WA] FS =0010 00cf9300 DPL=0 DS [-WA] GS =0010 00cf9300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS32-busy GDT= 000f6be8 0037 IDT= 000f6c26 CR0=6011 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS= CCD= CCO=ADDB EFER= FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80 FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= Aborted # qemu-system-x86_64 -machine isapc VNC server running on `::1:5900' qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44 EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014 EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0010 00cf9300 DPL=0 DS [-WA] CS =0008 00cf9b00 DPL=0 CS32 [-RA] SS =0010 00cf9300 DPL=0 DS [-WA] DS =0010 00cf9300 DPL=0 DS [-WA] FS =0010 00cf9300 DPL=0 DS [-WA] GS =0010 00cf9300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS32-busy GDT= 000f6be8 0037 IDT= 000f6c26 CR0=6011 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS= CCD= CCO=ADDB EFER= FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80 FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= Aborted ** 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/1400768 Title: Fatal error when running with '-machine isapc' on 2.1.2 Status in QEMU: New Bug description: all I have are the traces, should hopefully be easy to reproduce. # qemu-system-i386 -machine isapc VNC server running on `::1:5900' qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44 EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014 EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0010 00cf9300 DPL=0 DS [-WA] CS =0008 00cf9b00 DPL=0 CS32 [-RA] SS =0010 00cf9300 DPL=0 DS [-WA] DS =0010 00cf9300 DPL=0 DS [-WA] FS =0010 00cf9300 DPL=0 DS [-WA] GS =0010 00cf9300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS32-busy GDT= 000f6be8 0037 IDT= 000f6c26 CR0=6011 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS= CCD= CCO=ADDB EFER= FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80 FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00=
Re: [Qemu-devel] [PATCH v2] arm: add fw_cfg to virt board
On 9 December 2014 at 15:41, Laszlo Ersek ler...@redhat.com wrote: Again, this was the idea that Rich had in 2010 (see the links in the discussion thus far). It was rejected back then (which is why I didn't even try to resurrect it now), and Peter has now asked if anything has changed that would make that approach more acceptable now. Avi and Gleb not hanging around so they can't block the approach now might or might not be such a change; I don't know. But, I tend to trust their opinion, even if dates back to 2010 in this case. Yeah, this is about my point of view. A direct-write-to-memory fw_cfg doesn't seem too unreasonable to me, but it would be nice to see a solidly argued case from its proponents to counterbalance the previous rejection. Anyway I'm certainly not opposed to performance, so if someone can thoroughly refute everything they said in that thread, go ahead. As far as I can tell the main line of argument was if you're using fw_cfg to transfer huge amounts of data you're doing something wrong. Is ARM much higher overhead than x86 for these accesses? (Added Alex on cc as one of the few people from the previous round of argument who's still active...) -- PMM
Re: [Qemu-devel] [PATCH v9 02/10] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
On Mon, Dec 01, 2014 at 03:30:08PM -0500, John Snow wrote: diff --git a/block.c b/block.c index e5c6ccf..3f27519 100644 --- a/block.c +++ b/block.c @@ -5420,6 +5420,25 @@ int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t sector } } +#define BDB_MIN_DEF_GRANULARITY 4096 +#define BDB_MAX_DEF_GRANULARITY 65536 +#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY + +uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs) Long names are unwieldy but introducing multiple abbreviations is not a good solution, it makes the code more confusing (BDB vs dbm). I would call the function bdrv_get_default_bitmap_granularity(). The constants weren't necessary since the point of this function is to capture the default value. No one else should use the constants - otherwise there is a high probability that they are reimplementing this logic. I would just leave them as literals in the code. diff --git a/blockdev.c b/blockdev.c index 5651a8e..4d30b09 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1894,6 +1894,60 @@ void qmp_block_set_io_throttle(const char *device, int64_t bps, int64_t bps_rd, aio_context_release(aio_context); } +void qmp_block_dirty_bitmap_add(const char *device, const char *name, +bool has_granularity, int64_t granularity, +Error **errp) +{ +BlockDriverState *bs; + +bs = bdrv_lookup_bs(device, NULL, errp); Markus: I think we need to support node-name here so dirty bitmaps can be applied at any node in the graph? +if (!bs) { +return; +} These new monitor commands need to acquire/release the BlockDriverState's AioContext like the other monitor commands. This ensures that BDS accesses in other threads are synchronized (stopped while we run). AioContext *aio_context; aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); ... out: aio_context_release(aio_context); pgp__INvPK1Kj.pgp Description: PGP signature
[Qemu-devel] [Bug 1400768] Re: Fatal error when running with '-machine isapc' on 2.1.2
Hello, I too have the same results. Below is an additional snippet where the call is made through valgrind. While valgrind grinds, the SDL window displays Guest has not initialized the display (yet). ==16648== Memcheck, a memory error detector ==16648== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==16648== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info ==16648== Command: qemu-system-i386 -machine isapc -fda bootdisk.img ==16648== qemu: fatal: Trying to execute code outside RAM or ROM at 0x1a0dff44 EAX=000f0f88 EBX=0010 ECX=07fc EDX=002c ESI=6f5c EDI=0800 EBP=07fc ESP=fffe0014 EIP=1a0dff44 EFL=0046 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0010 00cf9300 DPL=0 DS [-WA] CS =0008 00cf9b00 DPL=0 CS32 [-RA] SS =0010 00cf9300 DPL=0 DS [-WA] DS =0010 00cf9300 DPL=0 DS [-WA] FS =0010 00cf9300 DPL=0 DS [-WA] GS =0010 00cf9300 DPL=0 DS [-WA] LDT= 8200 DPL=0 LDT TR = 8b00 DPL=0 TSS32-busy GDT= 000f6be8 0037 IDT= 000f6c26 CR0=6011 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 CCS= CCD= CCO=ADDB EFER= FCW=037f FSW= [ST=0] FTW=00 MXCSR=1f80 FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= ==16648== ==16648== HEAP SUMMARY: ==16648== in use at exit: 36,820,878 bytes in 32,195 blocks ==16648== total heap usage: 198,107 allocs, 165,912 frees, 1,828,399,692 bytes allocated ==16648== ==16648== LEAK SUMMARY: ==16648==definitely lost: 8,462 bytes in 29 blocks ==16648==indirectly lost: 55,605 bytes in 1,550 blocks ==16648== possibly lost: 316,286 bytes in 773 blocks ==16648==still reachable: 36,304,789 bytes in 29,208 blocks ==16648== suppressed: 0 bytes in 0 blocks ==16648== Rerun with --leak-check=full to see details of leaked memory ==16648== ==16648== For counts of detected and suppressed errors, rerun with: -v ==16648== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) Killed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1400768 Title: Fatal error when running with '-machine isapc' on 2.1.2 Status in QEMU: New Bug description:
Re: [Qemu-devel] [PATCH v9 03/10] block: Introduce bdrv_dirty_bitmap_granularity()
On Mon, Dec 01, 2014 at 03:30:09PM -0500, John Snow wrote: diff --git a/include/block/block.h b/include/block/block.h index 066ded6..f180f93 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -440,6 +440,8 @@ void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs); uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs); +int64_t bdrv_dirty_bitmap_granularity(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap); This looks weird now. The default granularity has uint64_t type but the actual granularity has int64_t. Please make them consistent. pgpHmbIpEuyvW.pgp Description: PGP signature
[Qemu-devel] [PATCH V9 0/3] Virtual Machine Generation ID
Hi, Resending patches after the release of version 2.2. Please note that this patch set doesn't include the *.hex.generated files and the binary ACPI tables (make check will fail). Thanks, Gal. V9 - Add a unit test. - Rebased to version 2.2. - Removed hex.generated the binary files from patch. V8 - Add a device's description file. - GUID is stored in fw cfg file and the guest writes the physical address to the device (reduces vmexits). V7 - Move the device's description back to the static SSDT table. - The GUID is store in a hard coded physical address and not in the ACPI table itself. - ACPI notification is triggered when the GUID is changed. V6 - include the pre-compiled ASL file - remove an empty line at end of files. V5 - Move device's description to SSDT table (dynamic). V4 - Fix a typo in error message string. - Move device's description from DSDT back to SSDT table. V3 - Remove -uuid command line parameter. - Move device's description from SSDT to DSDT table. - Add new vmgenid sysbus device. Gal Hammer (3): docs: vm generation id device's description i386: Add a Virtual Machine Generation ID device tests: add a unit test for the vmgenid device. default-configs/i386-softmmu.mak | 1 + default-configs/x86_64-softmmu.mak | 1 + docs/specs/vmgenid.txt | 27 hw/acpi/core.c | 8 ++ hw/acpi/ich9.c | 8 ++ hw/acpi/piix4.c | 8 ++ hw/i386/acpi-build.c | 26 +++ hw/i386/acpi-dsdt.dsl| 4 ++- hw/i386/pc.c | 8 ++ hw/i386/q35-acpi-dsdt.dsl| 5 +++- hw/i386/ssdt-misc.dsl| 43 hw/isa/lpc_ich9.c| 1 + hw/misc/Makefile.objs| 1 + include/hw/acpi/acpi.h | 2 ++ include/hw/acpi/acpi_dev_interface.h | 4 +++ include/hw/acpi/ich9.h | 2 ++ include/hw/i386/pc.h | 3 +++ tests/Makefile | 2 ++ tests/vmgenid-test.c | 48 19 files changed, 200 insertions(+), 2 deletions(-) create mode 100644 docs/specs/vmgenid.txt create mode 100644 tests/vmgenid-test.c -- 1.9.3
[Qemu-devel] [PATCH V9 2/3] i386: Add a Virtual Machine Generation ID device
Based on Microsoft's sepecifications (paper can be dowloaded from http://go.microsoft.com/fwlink/?LinkId=260709), add a device description to the SSDT ACPI table and its implementation. The GUID is set using a global vmgenid.uuid parameter. Signed-off-by: Gal Hammer gham...@redhat.com --- default-configs/i386-softmmu.mak | 1 + default-configs/x86_64-softmmu.mak | 1 + hw/acpi/core.c | 8 +++ hw/acpi/ich9.c | 8 +++ hw/acpi/piix4.c | 8 +++ hw/i386/acpi-build.c | 26 ++ hw/i386/acpi-dsdt.dsl| 4 +++- hw/i386/pc.c | 8 +++ hw/i386/q35-acpi-dsdt.dsl| 5 - hw/i386/ssdt-misc.dsl| 43 hw/isa/lpc_ich9.c| 1 + hw/misc/Makefile.objs| 1 + include/hw/acpi/acpi.h | 2 ++ include/hw/acpi/acpi_dev_interface.h | 4 include/hw/acpi/ich9.h | 2 ++ include/hw/i386/pc.h | 3 +++ 16 files changed, 123 insertions(+), 2 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 8e08841..bd33c75 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_VMGENID=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 66557ac..006fc7c 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -45,3 +45,4 @@ CONFIG_IOAPIC=y CONFIG_ICC_BUS=y CONFIG_PVPANIC=y CONFIG_MEM_HOTPLUG=y +CONFIG_VMGENID=y diff --git a/hw/acpi/core.c b/hw/acpi/core.c index 51913d6..d4597c6 100644 --- a/hw/acpi/core.c +++ b/hw/acpi/core.c @@ -28,6 +28,8 @@ #include qapi-visit.h #include qapi-event.h +#define ACPI_VM_GENERATION_ID_CHANGED_STATUS 1 + struct acpi_table_header { uint16_t _length; /* our length, not actual part of the hdr */ /* allows easier parsing for fw_cfg clients */ @@ -683,3 +685,9 @@ void acpi_update_sci(ACPIREGS *regs, qemu_irq irq) (regs-pm1.evt.en ACPI_BITMASK_TIMER_ENABLE) !(pm1a_sts ACPI_BITMASK_TIMER_STATUS)); } + +void acpi_vm_generation_id_changed(ACPIREGS *acpi_regs, qemu_irq irq) +{ +acpi_regs-gpe.sts[0] |= ACPI_VM_GENERATION_ID_CHANGED_STATUS; +acpi_update_sci(acpi_regs, irq); +} diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index ea991a3..12a9387 100644 --- a/hw/acpi/ich9.c +++ b/hw/acpi/ich9.c @@ -307,3 +307,11 @@ void ich9_pm_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list) acpi_memory_ospm_status(s-pm.acpi_memory_hotplug, list); } + +void ich9_vm_generation_id_changed(AcpiDeviceIf *adev) +{ +ICH9LPCState *s = ICH9_LPC_DEVICE(adev); +ICH9LPCPMRegs *pm = s-pm; + +acpi_vm_generation_id_changed(pm-acpi_regs, pm-irq); +} diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 481a16c..41b6eb6 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -574,6 +574,13 @@ static void piix4_ospm_status(AcpiDeviceIf *adev, ACPIOSTInfoList ***list) acpi_memory_ospm_status(s-acpi_memory_hotplug, list); } +static void piix4_vm_generation_id_changed(AcpiDeviceIf *adev) +{ +PIIX4PMState *s = PIIX4_PM(adev); + +acpi_vm_generation_id_changed(s-ar, s-irq); +} + static Property piix4_pm_properties[] = { DEFINE_PROP_UINT32(smb_io_base, PIIX4PMState, smb_io_base, 0), DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0), @@ -611,6 +618,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) hc-plug = piix4_device_plug_cb; hc-unplug_request = piix4_device_unplug_request_cb; adevc-ospm_status = piix4_ospm_status; +adevc-vm_generation_id_changed = piix4_vm_generation_id_changed; } static const TypeInfo piix4_pm_info = { diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index b37a397..bb53182 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -251,6 +251,7 @@ static void acpi_get_pci_info(PcPciInfo *info) #define ACPI_BUILD_TABLE_FILE etc/acpi/tables #define ACPI_BUILD_RSDP_FILE etc/acpi/rsdp #define ACPI_BUILD_TPMLOG_FILE etc/tpm/log +#define ACPI_BUILD_VMGENID_FILE etc/vm-generation-id static void build_header(GArray *linker, GArray *table_data, @@ -1062,6 +1063,8 @@ build_ssdt(GArray *table_data, GArray *linker, { MachineState *machine = MACHINE(qdev_get_machine()); uint32_t nr_mem = machine-ram_slots; +uint32_t vm_gid_physical_address; +uint32_t vm_gid_offset = 0; unsigned acpi_cpus = guest_info-apic_id_limit; int ssdt_start = table_data-len; uint8_t *ssdt_ptr; @@ -1090,6 +1093,21 @@ build_ssdt(GArray *table_data, GArray *linker, ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml),
[Qemu-devel] [PATCH V9 3/3] tests: add a unit test for the vmgenid device.
Signed-off-by: Gal Hammer gham...@redhat.com --- tests/Makefile | 2 ++ tests/vmgenid-test.c | 48 2 files changed, 50 insertions(+) create mode 100644 tests/vmgenid-test.c diff --git a/tests/Makefile b/tests/Makefile index 16f0e4c..612441a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -64,6 +64,7 @@ gcov-files-check-qom-interface-y = qom/object.c check-unit-$(CONFIG_POSIX) += tests/test-vmstate$(EXESUF) check-unit-y += tests/test-qemu-opts$(EXESUF) gcov-files-test-qemu-opts-y = qom/test-qemu-opts.c +check-qtest-i386-y += tests/vmgenid-test$(EXESUF) check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh @@ -351,6 +352,7 @@ tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y) tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y) tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a +tests/vmgenid-test$(EXESUF): tests/vmgenid-test.o ifeq ($(CONFIG_POSIX),y) LIBS += -lutil diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c new file mode 100644 index 000..d9c3e29 --- /dev/null +++ b/tests/vmgenid-test.c @@ -0,0 +1,48 @@ +/* + * QTest testcase for VM Generation ID + * + * Copyright (c) 2014 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include string.h +#include libqtest.h + +static void vmgenid_test(void) +{ +static const uint8_t expected[16] = { +0x32, 0x4e, 0x6e, 0xaf, 0xd1, 0xd1, 0x4b, 0xf6, +0xbf, 0x41, 0xb9, 0xbb, 0x6c, 0x91, 0xfb, 0x87 +}; +uint8_t guid[16]; +uint32_t i; + +// Emulate the ACPI _INI method (tells the device where the physical +// memory was allocated. +writel(0xfedf, 0x0001f000); + +// Skip the ACPI ADDR method and read the GUID directly from memory. +for (i = 0; i 16; i++) { +guid[i] = readb(0x0001f000 + i); +} + +g_assert_cmpuint(sizeof(guid), ==, sizeof(expected)); +g_assert(memcmp(guid, expected, sizeof(guid)) == 0); +} + +int main(int argc, char **argv) +{ +int ret; + +g_test_init(argc, argv, NULL); +qtest_add_func(/vmgenid/vmgenid, vmgenid_test); + +qtest_start(-global vmgenid.uuid=324e6eaf-d1d1-4bf6-bf41-b9bb6c91fb87); +ret = g_test_run(); + +qtest_end(); + +return ret; +} -- 1.9.3
[Qemu-devel] [PATCH V9 1/3] docs: vm generation id device's description
Signed-off-by: Gal Hammer gham...@redhat.com --- docs/specs/vmgenid.txt | 27 +++ 1 file changed, 27 insertions(+) create mode 100644 docs/specs/vmgenid.txt diff --git a/docs/specs/vmgenid.txt b/docs/specs/vmgenid.txt new file mode 100644 index 000..9a09d11 --- /dev/null +++ b/docs/specs/vmgenid.txt @@ -0,0 +1,27 @@ +VIRTUAL MACHINE GENERATION ID += + +The VM generation ID (vmgenid) device is an emulated device which +expose a 128-bit, cryptographically random, integer value identifier. +This allows management applications (e.g. libvirt) to notify the guest +operating system when the virtual machine is executed with a different +configuration (e.g. snapshot execution or creation from a template). + +Specs is on the web at: http://go.microsoft.com/fwlink/?LinkId=260709 + +--- + +The vmgenid device is a sysbus device with the following ACPI ID: +QEMU0002. + +The device adds a vmgenid.uuid property, which can be modifed using +the -global command line argument or the QMP interface. + +The device uses a fixed memory resource: 0xfedf-0xfedf0003. The +guest is expected to write the physical address of the GUID's buffer +to that memory resource. This allows the device to modify the GUID if +requested by the management application. + +According to the specification, any change to the GUID executes an +ACPI notification. The vmgenid device triggers the GPE._E00 which +executes the ACPI Notify operation. -- 1.9.3
[Qemu-devel] [PATCH 2/4] hw/virtio-blk: add a constant for max number of merged requests
As it was not obvious (at least for me) where the 32 comes from; add a constant for it. Signed-off-by: Peter Lieven p...@kamp.de Reviewed-by: Eric Blake ebl...@redhat.com --- hw/block/virtio-blk.c |2 +- include/hw/virtio/virtio-blk.h |4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index b19b102..490f961 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -326,7 +326,7 @@ static void virtio_blk_handle_write(VirtIOBlockReq *req, MultiReqBuffer *mrb) block_acct_start(blk_get_stats(req-dev-blk), req-acct, req-qiov.size, BLOCK_ACCT_WRITE); -if (mrb-num_writes == 32) { +if (mrb-num_writes == VIRTIO_BLK_MAX_MERGE_REQS) { virtio_submit_multiwrite(req-dev-blk, mrb); } diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 3979dc4..3f2652f 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -134,8 +134,10 @@ typedef struct VirtIOBlock { struct VirtIOBlockDataPlane *dataplane; } VirtIOBlock; +#define VIRTIO_BLK_MAX_MERGE_REQS 32 + typedef struct MultiReqBuffer { -BlockRequestblkreq[32]; +BlockRequestblkreq[VIRTIO_BLK_MAX_MERGE_REQS]; unsigned intnum_writes; } MultiReqBuffer; -- 1.7.9.5
[Qemu-devel] [PATCH 4/4] virtio-blk: introduce multiread
this patch finally introduces multiread support to virtio-blk. While multiwrite support was there for a long time, read support was missing. To achieve this the patch does several things which might need further explanation: - the whole merge and multireq logic is moved from block.c into virtio-blk. This is move is a preparation for directly creating a coroutine out of virtio-blk. - requests are only merged if they are strictly sequential, and no longer sorted. This simplification decreases overhead and reduces latency. It will also merge some requests which were unmergable before. The old algorithm took up to 32 requests, sorted them and tried to merge them. The outcome was anything between 1 and 32 requests. In case of 32 requests there were 31 requests unnecessarily delayed. On the other hand let's imagine e.g. 16 unmergeable requests followed by 32 mergable requests. The latter 32 requests would have been split into two 16 byte requests. Last the simplified logic allows for a fast path if we have only a single request in the multirequest. In this case the request is sent as ordinary request without multireq callbacks. As a first benchmark I installed Ubuntu 14.04.1 on a local SSD. The number of merged requests is in the same order while the write latency is obviously decreased by several percent. cmdline: qemu-system-x86_64 -m 1024 -smp 2 -enable-kvm -cdrom ubuntu-14.04.1-server-amd64.iso \ -drive if=virtio,file=/dev/ssd/ubuntu1404,aio=native,cache=none -monitor stdio Before: virtio0: rd_bytes=151056896 wr_bytes=2683947008 rd_operations=18614 wr_operations=67979 flush_operations=15335 wr_total_time_ns=540428034217 rd_total_time_ns=0520068 flush_total_time_ns=40673685006 rd_merged=0 wr_merged=15531 After: virtio0: rd_bytes=149487104 wr_bytes=2701344768 rd_operations=18148 wr_operations=68578 flush_operations=15368 wr_total_time_ns=437030089565 rd_total_time_ns=9836288815 flush_total_time_ns=40597981121 rd_merged=690 wr_merged=14615 Some first numbers of improved read performance while booting: The Ubuntu 14.04.1 vServer from above: virtio0: rd_bytes=97545216 wr_bytes=119808 rd_operations=5071 wr_operations=26 flush_operations=2 wr_total_time_ns=8847669 rd_total_time_ns=13952575478 flush_total_time_ns=3075496 rd_merged=742 wr_merged=0 Windows 2012R2 (booted from iSCSI): virtio0: rd_bytes=176559104 wr_bytes=61859840 rd_operations=7200 wr_operations=360 flush_operations=68 wr_total_time_ns=34344992718 rd_total_time_ns=134386844669 flush_total_time_ns=18115517 rd_merged=641 wr_merged=216 Signed-off-by: Peter Lieven p...@kamp.de --- hw/block/dataplane/virtio-blk.c |6 +- hw/block/virtio-blk.c | 239 --- include/hw/virtio/virtio-blk.h | 22 ++-- trace-events|1 + 4 files changed, 162 insertions(+), 106 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..30bf2e7 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -96,9 +96,7 @@ static void handle_notify(EventNotifier *e) event_notifier_test_and_clear(s-host_notifier); blk_io_plug(s-conf-conf.blk); for (;;) { -MultiReqBuffer mrb = { -.num_writes = 0, -}; +MultiReqBuffer mrb = {}; int ret; /* Disable guest-host notifies to avoid unnecessary vmexits */ @@ -120,7 +118,7 @@ static void handle_notify(EventNotifier *e) virtio_blk_handle_request(req, mrb); } -virtio_submit_multiwrite(s-conf-conf.blk, mrb); +virtio_submit_multireq(s-conf-conf.blk, mrb); if (likely(ret == -EAGAIN)) { /* vring emptied */ /* Re-enable guest-host notifies and stop processing the vring. diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 490f961..cc0076a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -34,6 +34,8 @@ VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) req-dev = s; req-qiov.size = 0; req-next = NULL; +req-mr_next = NULL; +req-mr_qiov.nalloc = 0; return req; } @@ -84,20 +86,29 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, static void virtio_blk_rw_complete(void *opaque, int ret) { -VirtIOBlockReq *req = opaque; +VirtIOBlockReq *next = opaque; -trace_virtio_blk_rw_complete(req, ret); +while (next) { +VirtIOBlockReq *req = next; +next = req-mr_next; +trace_virtio_blk_rw_complete(req, ret); -if (ret) { -int p = virtio_ldl_p(VIRTIO_DEVICE(req-dev), req-out.type); -bool is_read = !(p VIRTIO_BLK_T_OUT); -if (virtio_blk_handle_rw_error(req, -ret, is_read)) -return; -} +if (req-mr_qiov.nalloc) { +qemu_iovec_destroy(req-mr_qiov); +} -virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); -