[Qemu-devel] [PATCH v11 8/9] hw/pc_piix: add pc-1.1
Signed-off-by: Jordan Justen jordan.l.jus...@intel.com --- hw/pc_piix.c | 12 ++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/hw/pc_piix.c b/hw/pc_piix.c index 2fc4211..fcf0153 100644 --- a/hw/pc_piix.c +++ b/hw/pc_piix.c @@ -371,8 +371,8 @@ static void pc_xen_hvm_init(ram_addr_t ram_size, } #endif -static QEMUMachine pc_machine_v1_0 = { -.name = pc-1.0, +static QEMUMachine pc_machine_v1_1 = { +.name = pc-1.1, .alias = pc, .desc = Standard PC, .init = pc_init_pci, @@ -380,6 +380,13 @@ static QEMUMachine pc_machine_v1_0 = { .is_default = 1, }; +static QEMUMachine pc_machine_v1_0 = { +.name = pc-1.0, +.desc = Standard PC, +.init = pc_init_pci, +.max_cpus = 255, +}; + static QEMUMachine pc_machine_v0_15 = { .name = pc-0.15, .desc = Standard PC, @@ -669,6 +676,7 @@ static QEMUMachine xenfv_machine = { static void pc_machine_init(void) { +qemu_register_machine(pc_machine_v1_1); qemu_register_machine(pc_machine_v1_0); qemu_register_machine(pc_machine_v0_15); qemu_register_machine(pc_machine_v0_14); -- 1.7.1
Re: [Qemu-devel] KVM call agenda for tuesday 31
On 02/21/2012 07:33 PM, Peter Maydell wrote: Short summary: * switch wp groups to bitfield rather than int array * convert sd.c to use memory_region_init_ram() to allocate the wp groups (being careful to use memory_region_set_dirty() when we touch them) * we don't need variable-length fields for sd.c any more * rest of the vmstate conversion is straightforward OK, got it. But it's not clear to me, have you decided against introducing .get_bufsize completely or just for sd.c? I still think it's a good idea and I wouldn't mind to use it in SD host controller implementation to retrieve buffer size directly from capabilities register, but I can take another approach for this of course. -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
[Qemu-devel] virtio-win-0.1-22 balloon install fails on 2008R2
Trying to install the virtio-win-0.1-22.iso balloon driver on Windows 2008R2: E:\Wlh\amd64x:\microsoft\WinDDK\7600.16385.win7_wdk.100208-1538\tools\devcon\amd64\devcon.exe install BALLOON.inf PCI\VEN_1AF4DEV_1002SUBSYS_00051AF4REV_00 Device node created. Install is complete when drivers are installed... Updating drivers for PCI\VEN_1AF4 from E:\Wlh\amd64\BALLOON.inf. devcon.exe failed. 'DEV_1002' is not recognized as an internal or external command, operable program or batch file. 'SUBSYS_00051AF4' is not recognized as an internal or external command, operable program or batch file. 'REV_00' is not recognized as an internal or external command, operable program or batch file. Any help appreciated. sean
[Qemu-devel] [PATCH v11 6/9] hw/pc_sysfw: support system flash memory with pflash
Flash can be enabled by calling pc_system_firmware_init with the system_flash_enabled parameter being non-zero. If system_flash_enabled is zero, then the older qemu rom creation method will be used. If flash is enabled and a pflash image is found, then it is used for the system firmware image. If flash is enabled and a pflash image is not initially found, then a read-only pflash device is created using the -bios filename. KVM cannot execute from a pflash region currently. Therefore, when KVM is enabled, the old rom based initialization method is used. Signed-off-by: Jordan Justen jordan.l.jus...@intel.com --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/pc_sysfw.c | 129 +++- 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 662348e..2c78175 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -24,3 +24,4 @@ CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y +CONFIG_PFLASH_CFI01=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index b445be2..233a856 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -24,3 +24,4 @@ CONFIG_SOUND=y CONFIG_HPET=y CONFIG_APPLESMC=y CONFIG_I8259=y +CONFIG_PFLASH_CFI01=y diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index ae5c4b1..972706c 100644 --- a/hw/pc_sysfw.c +++ b/hw/pc_sysfw.c @@ -26,8 +26,11 @@ #include sysbus.h #include hw.h #include pc.h +#include hw/boards.h #include loader.h #include sysemu.h +#include flash.h +#include kvm.h #define BIOS_FILENAME bios.bin @@ -36,7 +39,96 @@ typedef struct PcSysFwDevice { uint8_t rom_only; } PcSysFwDevice; -static void pc_system_rom_init(MemoryRegion *rom_memory) +static void pc_isa_bios_init(MemoryRegion *rom_memory, + MemoryRegion *flash_mem, + int ram_size) +{ +int isa_bios_size; +MemoryRegion *isa_bios; +uint64_t flash_size; +void *flash_ptr, *isa_bios_ptr; + +flash_size = memory_region_size(flash_mem); + +/* map the last 128KB of the BIOS in ISA space */ +isa_bios_size = flash_size; +if (isa_bios_size (128 * 1024)) { +isa_bios_size = 128 * 1024; +} +isa_bios = g_malloc(sizeof(*isa_bios)); +memory_region_init_ram(isa_bios, isa-bios, isa_bios_size); +vmstate_register_ram_global(isa_bios); +memory_region_add_subregion_overlap(rom_memory, +0x10 - isa_bios_size, +isa_bios, +1); + +/* copy ISA rom image from top of flash memory */ +flash_ptr = memory_region_get_ram_ptr(flash_mem); +isa_bios_ptr = memory_region_get_ram_ptr(isa_bios); +memcpy(isa_bios_ptr, + ((uint8_t*)flash_ptr) + (flash_size - isa_bios_size), + isa_bios_size); + +memory_region_set_readonly(isa_bios, true); +} + +static void pc_fw_add_pflash_drv(void) +{ +QemuOpts *opts; +QEMUMachine *machine; +char *filename; + +if (bios_name == NULL) { +bios_name = BIOS_FILENAME; +} +filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); + +opts = drive_add(IF_PFLASH, -1, filename, readonly=on); +if (opts == NULL) { + return; +} + +machine = find_default_machine(); +if (machine == NULL) { + return; +} + +drive_init(opts, machine-use_scsi); +} + +static void pc_system_flash_init(MemoryRegion *rom_memory, + DriveInfo *pflash_drv) +{ +BlockDriverState *bdrv; +int64_t size; +target_phys_addr_t phys_addr; +int sector_bits, sector_size; +pflash_t *system_flash; +MemoryRegion *flash_mem; + +bdrv = pflash_drv-bdrv; +size = bdrv_getlength(pflash_drv-bdrv); +sector_bits = 12; +sector_size = 1 sector_bits; + +if ((size % sector_size) != 0) { +fprintf(stderr, +qemu: PC system firmware (pflash) must be a multiple of 0x%x\n, +sector_size); +exit(1); +} + +phys_addr = 0x1ULL - size; +system_flash = pflash_cfi01_register(phys_addr, NULL, system.flash, size, + bdrv, sector_size, size sector_bits, + 1, 0x, 0x, 0x, 0x, 0); +flash_mem = pflash_cfi01_get_memory(system_flash); + +pc_isa_bios_init(rom_memory, flash_mem, size); +} + +static void old_pc_system_rom_init(MemoryRegion *rom_memory) { char *filename; MemoryRegion *bios, *isa_bios; @@ -93,11 +185,44 @@ static void pc_system_rom_init(MemoryRegion *rom_memory) void pc_system_firmware_init(MemoryRegion *rom_memory) { +DriveInfo *pflash_drv; PcSysFwDevice *sysfw_dev; sysfw_dev =
Re: [Qemu-devel] [PATCH 0/2] Add -uboot option
On 22 February 2012 06:58, Evgeny Voevodin e.voevo...@samsung.com wrote: These patches add -uboot option to ARM boards. To let user load u-boot, board should initialize .uboot_start in arm_boot_info struct. Added utilisation of -uboot for exynos4 and integratorcp. Nack. This kind of thing should be done by making QEMU load an ELF file which has u-boot in it (which at the moment you can do with -kernel). Loading Linux kernels is a special case in arm_boot because of the complicated boot protocol they have and because people want to load lots of different kernels. -- PMM
[Qemu-devel] [Bug 938431] Re: Reproducible crash in slirp_remque (qemu 1.0.1)
I have now reproduced the same segfault without the controlling script by running qemu on the command line and connecting to it with lftp. To reproduce the fault it appears to be necessary to attempt to connect to the guest before it is fully booted and ready to accept connections; if I let it settle for a while before attempting to connect then it doesn't crash. Even if I start hammering it as soon as it's launched I can only occasionally trigger the crash, so whatever's breaking is a short-lived state of some kind. If I make an lftp connection then immediately kill lftp, qemu receives a SIGPIPE. I'm wondering if a sigpipe at the wrong time is messing things up, but it's only the vaguest notion. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/938431 Title: Reproducible crash in slirp_remque (qemu 1.0.1) Status in QEMU: New Bug description: Heya I've been testing some automated data conversion scripts with qemu 1.0.1. They work fine with qemu-kvm 0.15.1, but on qemu 1.0.1 (from the website, built from source using gcc 4.6.1, i686 host), when the script runs qemu I see qemu crash in slirp_remque a few seconds after it's launched. This crash is consistent and reproducible. The qemu guest is SCO OpenServer 5.0.5. I'm using it for some data conversion from a legacy application. qemu is launched -display none -monitor stdio and controlled from a Python script that then connects to the VM over usermode port forwards to ftp data to/from the VM and send commands over telnet. qemu is launched fine with the following command: /usr/local/qemu/bin/qemu-system-i386 -display none -vga cirrus -M pc -no-acpi -no-hpet -monitor stdio -net user,net=10.0.2.0/24,host=10.0.2.2,dns=10.0.2.3,hostfwd=tcp:127.0.0.1:-10.0.2.1:22,hostfwd=tcp:127.0.0.1:2323-10.0.2.1:23,hostfwd=tcp:127.0.0.1:2121-10.0.2.1:21,hostfwd=tcp:127.0.0.1:2020-10.0.2.1:20 -net nic,model=pcnet -drive file=sco/sco.qcow2,format=qcow2,cache=unsafe,snapshot=on -drive file=sco/booksys.qcow2,format=qcow2,cache=unsafe,snapshot=on -snapshot qemu-log and images: $ for f in *.qcow2; do qemu-img info $f; echo; done image: booksys-blank-compressed.qcow2 file format: qcow2 virtual size: 4.0G (4294967296 bytes) disk size: 696K cluster_size: 65536 image: booksys.qcow2 file format: qcow2 virtual size: 4.0G (4294967296 bytes) disk size: 140K cluster_size: 65536 backing file: booksys-blank-compressed.qcow2 (actual path: booksys-blank-compressed.qcow2) image: sco-base-compressed.qcow2 file format: qcow2 virtual size: 512M (536870912 bytes) disk size: 142M cluster_size: 65536 image: sco.qcow2 file format: qcow2 virtual size: 512M (536870912 bytes) disk size: 140K cluster_size: 65536 backing file: sco-base-compressed.qcow2 (actual path: sco-base-compressed.qcow2) The VM guest begins booting fine, and nothing of interest appears in the monitor log: QEMU 1.0,1 monitor - type 'help' for more information (qemu) After a few seconds the controlling scripts begins trying to ftp into the guest over the user-mode port forward on port 2121, and it's at this point that qemu crashes with the following backtrace: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xb63e46e0 (LWP 25453)] 0xb768753b in slirp_remque (a=0xb90ee408) at slirp/misc.c:39 39 ((struct quehead *)(element-qh_rlink))-qh_link = element-qh_link; (gdb) bt #0 0xb768753b in slirp_remque (a=0xb90ee408) at slirp/misc.c:39 #1 0xb76854ad in if_start (slirp=0xb879beb0) at slirp/if.c:189 #2 0xb76853b3 in if_output (so=0xb8eb1380, ifm=0xb90eea60) at slirp/if.c:138 #3 0xb7686bb5 in ip_output (so=0xb8eb1380, m0=0xb90eea60) at slirp/ip_output.c:84 #4 0xb768f59c in tcp_output (tp=0xb906fd48) at slirp/tcp_output.c:456 #5 0xb7691b9b in tcp_timers (tp=0xb906fd48, timer=0) at slirp/tcp_timer.c:242 #6 0xb76918d4 in tcp_slowtimo (slirp=0xb879beb0) at slirp/tcp_timer.c:88 #7 0xb768965a in slirp_select_poll (readfds=0xbf9e3dcc, writefds=0xbf9e3e4c, xfds=0xbf9e3ecc, select_error=0) at slirp/slirp.c:433 #8 0xb763e2a0 in main_loop_wait (nonblocking=0) at main-loop.c:465 #9 0xb7633042 in main_loop () at /home/craig/build/qemu-1.0.1/vl.c:1481 #10 0xb76388a0 in main (argc=20, argv=0xbf9e42d4, envp=0xbf9e4328) at /home/craig/build/qemu-1.0.1/vl.c:3485 (gdb) frame 0 #0 0xb768753b in slirp_remque (a=0xb90ee408) at slirp/misc.c:39 39 ((struct quehead *)(element-qh_rlink))-qh_link = element-qh_link; A more detailed backtrace, as supplied by thread apply all bt full, follows at the end of this post. In case it matters, stdout is redirected to a logfile and stdin is attached to the Python script, which hasn't yet written anything to the stdin pipe. I'll happily post the script, but isn't much good without the OS image
[Qemu-devel] [PATCH v11 4/9] hw/pc: move rom init to pc_sysfw.c
Signed-off-by: Jordan Justen jordan.l.jus...@intel.com --- Makefile.target |1 + hw/pc.c | 56 +++-- hw/pc.h |3 ++ hw/pc_sysfw.c | 92 +++ 4 files changed, 101 insertions(+), 51 deletions(-) create mode 100644 hw/pc_sysfw.c diff --git a/Makefile.target b/Makefile.target index 651500e..2fc9f39 100644 --- a/Makefile.target +++ b/Makefile.target @@ -241,6 +241,7 @@ obj-i386-y += vmport.o obj-i386-y += pci-hotplug.o smbios.o wdt_ib700.o obj-i386-y += debugcon.o multiboot.o obj-i386-y += pc_piix.o +obj-i386-y += pc_sysfw.o obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o diff --git a/hw/pc.c b/hw/pc.c index 5746129..b9f4bc7 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -60,10 +60,6 @@ #define DPRINTF(fmt, ...) #endif -#define BIOS_FILENAME bios.bin - -#define PC_MAX_BIOS_SIZE (4 * 1024 * 1024) - /* Leave a chunk of memory at the top of RAM for the BIOS ACPI tables. */ #define ACPI_DATA_SIZE 0x1 #define BIOS_CFG_IOPORT 0x510 @@ -990,11 +986,9 @@ void pc_memory_init(MemoryRegion *system_memory, MemoryRegion *rom_memory, MemoryRegion **ram_memory) { -char *filename; -int ret, linux_boot, i; -MemoryRegion *ram, *bios, *isa_bios, *option_rom_mr; +int linux_boot, i; +MemoryRegion *ram, *option_rom_mr; MemoryRegion *ram_below_4g, *ram_above_4g; -int bios_size, isa_bios_size; void *fw_cfg; linux_boot = (kernel_filename != NULL); @@ -1020,44 +1014,9 @@ void pc_memory_init(MemoryRegion *system_memory, ram_above_4g); } -/* BIOS load */ -if (bios_name == NULL) -bios_name = BIOS_FILENAME; -filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name); -if (filename) { -bios_size = get_image_size(filename); -} else { -bios_size = -1; -} -if (bios_size = 0 || -(bios_size % 65536) != 0) { -goto bios_error; -} -bios = g_malloc(sizeof(*bios)); -memory_region_init_ram(bios, pc.bios, bios_size); -vmstate_register_ram_global(bios); -memory_region_set_readonly(bios, true); -ret = rom_add_file_fixed(bios_name, (uint32_t)(-bios_size), -1); -if (ret != 0) { -bios_error: -fprintf(stderr, qemu: could not load PC BIOS '%s'\n, bios_name); -exit(1); -} -if (filename) { -g_free(filename); -} -/* map the last 128KB of the BIOS in ISA space */ -isa_bios_size = bios_size; -if (isa_bios_size (128 * 1024)) -isa_bios_size = 128 * 1024; -isa_bios = g_malloc(sizeof(*isa_bios)); -memory_region_init_alias(isa_bios, isa-bios, bios, - bios_size - isa_bios_size, isa_bios_size); -memory_region_add_subregion_overlap(rom_memory, -0x10 - isa_bios_size, -isa_bios, -1); -memory_region_set_readonly(isa_bios, true); + +/* Initialize PC system firmware */ +pc_system_firmware_init(rom_memory); option_rom_mr = g_malloc(sizeof(*option_rom_mr)); memory_region_init_ram(option_rom_mr, pc.rom, PC_ROM_SIZE); @@ -1067,11 +1026,6 @@ void pc_memory_init(MemoryRegion *system_memory, option_rom_mr, 1); -/* map all the bios at the top of memory */ -memory_region_add_subregion(rom_memory, -(uint32_t)(-bios_size), -bios); - fw_cfg = bochs_bios_init(); rom_set_fw(fw_cfg); diff --git a/hw/pc.h b/hw/pc.h index 1b47bbd..71f7df3 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -216,6 +216,9 @@ static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo *nd) return true; } +/* pc_sysfw.c */ +void pc_system_firmware_init(MemoryRegion *rom_memory); + /* e820 types */ #define E820_RAM1 #define E820_RESERVED 2 diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c new file mode 100644 index 000..b0bb7b8 --- /dev/null +++ b/hw/pc_sysfw.c @@ -0,0 +1,92 @@ +/* + * QEMU PC System Firmware + * + * Copyright (c) 2003-2004 Fabrice Bellard + * Copyright (c) 2011-2012 Intel Corporation + * + * 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
[Qemu-devel] [PATCH v11 2/9] pflash_cfi01/02: support read-only pflash devices
Signed-off-by: Jordan Justen jordan.l.jus...@intel.com --- hw/pflash_cfi01.c | 44 +++- hw/pflash_cfi02.c | 83 2 files changed, 75 insertions(+), 52 deletions(-) diff --git a/hw/pflash_cfi01.c b/hw/pflash_cfi01.c index ee0c3ba..b03f623 100644 --- a/hw/pflash_cfi01.c +++ b/hw/pflash_cfi01.c @@ -283,8 +283,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, TARGET_FMT_plx \n, __func__, offset, pfl-sector_len); -memset(p + offset, 0xff, pfl-sector_len); -pflash_update(pfl, offset, pfl-sector_len); +if (!pfl-ro) { +memset(p + offset, 0xff, pfl-sector_len); +pflash_update(pfl, offset, pfl-sector_len); +} else { +pfl-status |= 0x20; /* Block erase error */ +} pfl-status |= 0x80; /* Ready! */ break; case 0x50: /* Clear status bits */ @@ -323,8 +327,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, case 0x10: /* Single Byte Program */ case 0x40: /* Single Byte Program */ DPRINTF(%s: Single Byte Program\n, __func__); -pflash_data_write(pfl, offset, value, width, be); -pflash_update(pfl, offset, width); +if (!pfl-ro) { +pflash_data_write(pfl, offset, value, width, be); +pflash_update(pfl, offset, width); +} else { +pfl-status |= 0x10; /* Programming error */ +} pfl-status |= 0x80; /* Ready! */ pfl-wcycle = 0; break; @@ -372,7 +380,11 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, case 2: switch (pfl-cmd) { case 0xe8: /* Block write */ -pflash_data_write(pfl, offset, value, width, be); +if (!pfl-ro) { +pflash_data_write(pfl, offset, value, width, be); +} else { +pfl-status |= 0x10; /* Programming error */ +} pfl-status |= 0x80; @@ -382,8 +394,12 @@ static void pflash_write(pflash_t *pfl, target_phys_addr_t offset, DPRINTF(%s: block write finished\n, __func__); pfl-wcycle++; -/* Flush the entire write buffer onto backing storage. */ -pflash_update(pfl, offset mask, pfl-writeblock_size); +if (!pfl-ro) { +/* Flush the entire write buffer onto backing storage. */ +pflash_update(pfl, offset mask, pfl-writeblock_size); +} else { +pfl-status |= 0x10; /* Programming error */ +} } pfl-counter--; @@ -607,13 +623,13 @@ pflash_t *pflash_cfi01_register(target_phys_addr_t base, } bdrv_attach_dev_nofail(pfl-bs, pfl); } -#if 0 /* XXX: there should be a bit to set up read-only, - * the same way the hardware does (with WP pin). - */ -pfl-ro = 1; -#else -pfl-ro = 0; -#endif + +if (pfl-bs) { +pfl-ro = bdrv_is_read_only(pfl-bs); +} else { +pfl-ro = 0; +} + pfl-timer = qemu_new_timer_ns(vm_clock, pflash_timer, pfl); pfl-base = base; pfl-sector_len = sector_len; diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c index 2ca0fd4..3e2002e 100644 --- a/hw/pflash_cfi02.c +++ b/hw/pflash_cfi02.c @@ -330,35 +330,37 @@ static void pflash_write (pflash_t *pfl, target_phys_addr_t offset, DPRINTF(%s: write data offset TARGET_FMT_plx %08x %d\n, __func__, offset, value, width); p = pfl-storage; -switch (width) { -case 1: -p[offset] = value; -pflash_update(pfl, offset, 1); -break; -case 2: -if (be) { -p[offset] = value 8; -p[offset + 1] = value; -} else { +if (!pfl-ro) { +switch (width) { +case 1: p[offset] = value; -p[offset + 1] = value 8; +pflash_update(pfl, offset, 1); +break; +case 2: +if (be) { +p[offset] = value 8; +p[offset + 1] = value; +} else { +p[offset] = value; +p[offset + 1] = value 8; +} +pflash_update(pfl, offset, 2); +break; +case 4: +if (be) { +p[offset] = value 24; +p[offset + 1] = value 16; +p[offset + 2] = value 8; +p[offset + 3] = value; +
[Qemu-devel] [PATCH v11 5/9] hw/pc_sysfw: enable pc-sysfw as a qdev
Setup a pc-sysfw device type. It contains a single property of 'rom_only' which is defaulted to enabled. Signed-off-by: Jordan Justen jordan.l.jus...@intel.com --- hw/pc_sysfw.c | 37 + 1 files changed, 37 insertions(+), 0 deletions(-) diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c index b0bb7b8..ae5c4b1 100644 --- a/hw/pc_sysfw.c +++ b/hw/pc_sysfw.c @@ -23,12 +23,19 @@ * THE SOFTWARE. */ +#include sysbus.h +#include hw.h #include pc.h #include loader.h #include sysemu.h #define BIOS_FILENAME bios.bin +typedef struct PcSysFwDevice { +SysBusDevice busdev; +uint8_t rom_only; +} PcSysFwDevice; + static void pc_system_rom_init(MemoryRegion *rom_memory) { char *filename; @@ -86,7 +93,37 @@ static void pc_system_rom_init(MemoryRegion *rom_memory) void pc_system_firmware_init(MemoryRegion *rom_memory) { +PcSysFwDevice *sysfw_dev; + +sysfw_dev = (PcSysFwDevice*) qdev_create(NULL, pc-sysfw); + pc_system_rom_init(rom_memory); } +static Property pcsysfw_properties[] = { +DEFINE_PROP_UINT8(rom_only, PcSysFwDevice, rom_only, 1), +DEFINE_PROP_END_OF_LIST(), +}; + +static void pcsysfw_class_init (ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS (klass); + +dc-desc = PC System Firmware; +dc-props = pcsysfw_properties; +} + +static TypeInfo pcsysfw_info = { +.name = pc-sysfw, +.parent= TYPE_SYS_BUS_DEVICE, +.instance_size = sizeof (PcSysFwDevice), +.class_init= pcsysfw_class_init, +}; + +static void pcsysfw_register (void) +{ +type_register_static (pcsysfw_info); +} + +type_init (pcsysfw_register); -- 1.7.1
[Qemu-devel] [PATCH v11 0/9] PC system flash support
Enable flash emulation in a PC system using pflash_cfi01. v11: * Convert pc-sysfw to qdev - Add rom_only property * Remove KVM flash support to remove the need for using bdrv_read during machine initialization. KVM should now continue to use the same 'rom' initialization sequence that it uses today. v10: * Rebase to HEAD * decouple vmstate from memory API as in c5705a7 * Break changes into smaller pieces v9: * Add pc-1.1 * pc-1.0 uses previous rom firmware init code path v8: * Cleanup two chunks of debug code (printf messages) * Fix comment in pc.h (pcflash.c = pc_sysfw.c) v7: * Do not add system firmware to qemu roms * If kvm is enabled, copy pflash drive contents into a read-only ram region, since kvm cannot currently execute code from a pflash device. * Rename pcflash.c to pc_sysfw.c v6: * Rebase for memory API * pflash_cfi01: Set error in status register when a write to erase is attempted in read-only mode. * Add system firmware to qemu roms v5: * Enable pflash read-only mode * Enable -drive with if=pflash to define system firmware image v4: * Rebase v3: * Fix code style issues * Add additional comments v2: * Convert debug printf to DPRINTF Jordan Justen (9): blockdev: allow read-only pflash devices pflash_cfi01/02: support read-only pflash devices vl: make find_default_machine externally visible hw/pc: move rom init to pc_sysfw.c hw/pc_sysfw: enable pc-sysfw as a qdev hw/pc_sysfw: support system flash memory with pflash hw/pc_piix: remove is_default for pc-0.15 hw/pc_piix: add pc-1.1 pc_piix/pc_sysfw: enable flash by default Makefile.target|1 + blockdev.c |3 +- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/boards.h|1 + hw/pc.c| 56 +--- hw/pc.h|3 + hw/pc_piix.c | 62 +- hw/pc_sysfw.c | 254 hw/pflash_cfi01.c | 44 +-- hw/pflash_cfi02.c | 83 +++-- vl.c |2 +- 12 files changed, 403 insertions(+), 108 deletions(-) create mode 100644 hw/pc_sysfw.c
Re: [Qemu-devel] [PATCH 0/2] Add -uboot option
On 22.02.2012 12:12, Peter Maydell wrote: On 22 February 2012 06:58, Evgeny Voevodine.voevo...@samsung.com wrote: These patches add -uboot option to ARM boards. To let user load u-boot, board should initialize .uboot_start in arm_boot_info struct. Added utilisation of -uboot for exynos4 and integratorcp. Nack. This kind of thing should be done by making QEMU load an ELF file which has u-boot in it (which at the moment you can do with -kernel). Loading Linux kernels is a special case in arm_boot because of the complicated boot protocol they have and because people want to load lots of different kernels. -- PMM As I understood, you mean that -kernel is for people who want to quickly compile and try different kernel. As for me, -uboot is the same ) No need to generate a file, where u-boot and kernel are placed. Just use -uboot ./u-boot -kernel ./uImage, change kernel, or change u-boot, or both, as you wish. -- Kind regards, Evgeny Voevodin, Leading Software Engineer, ASWG, Moscow RD center, Samsung Electronics e-mail: e.voevo...@samsung.com
Re: [Qemu-devel] [PATCH 0/2] Add -uboot option
On 22 February 2012 08:29, Evgeny Voevodin e.voevo...@samsung.com wrote: On 22.02.2012 12:12, Peter Maydell wrote: Nack. This kind of thing should be done by making QEMU load an ELF file which has u-boot in it (which at the moment you can do with -kernel). Loading Linux kernels is a special case in arm_boot because of the complicated boot protocol they have and because people want to load lots of different kernels. As I understood, you mean that -kernel is for people who want to quickly compile and try different kernel. As for me, -uboot is the same ) No need to generate a file, where u-boot and kernel are placed. Just use -uboot ./u-boot -kernel ./uImage, change kernel, or change u-boot, or both, as you wish. If you're using -kernel you don't need to provide a u-boot because qemu is being the bootloader. If you're making qemu execute a bootloader than you should just put the kernel where u-boot would load it normally. We really can't add random options to qemu to support loading every single thing you might possibly want to load, which is why I'm drawing a line in the sand here. -- PMM
Re: [Qemu-devel] Help me about the FDC
On Wed, Feb 22, 2012 at 6:16 AM, Zhi Hui Li zhihu...@linux.vnet.ibm.com wrote: 1: explain the difference between : type_register_static(isa_fdc_info); type_register_static(sysbus_fdc_info); type_register_static(sun4m_fdc_info); The floppy disk controller is used by several different targets, including x86 PC, SPARC, and others. Look at the TypeInfo declarations for isa_fdc_info and the others to see how they differ. For example the sun4m (SPARC) fdc sets up an memory-mapped I/O (MMIO) region and an interrupt. The isa (PC) fdc sets up programmed I/O (PIO) and an interrupt. Basically, both sun4m and PCs have similar fdc hardware but the hardware registers are mapped to different places. This is why there are different setup functions to make sure the MMIO, PIO, and/or interrupts are set up for a specific target. 2: explain the struct of FDCtrl; FDCtrl is the main struct for an emulated floppy disk controller. In order to understand it you need to know how the floppy disk controller behaves, what hardware registers it has, etc. You need to read the floppy disk controler hardware datasheet first before looking at QEMU device emulation code - otherwise you will not know what QEMU is trying to emulate :). This leads to your next question... 3: or give me some introduce of FDC. Start here: http://en.wikipedia.org/wiki/Floppy_disk_controller http://wiki.osdev.org/Floppy_Disk_Controller Then look at the datasheet: http://wiki.qemu.org/images/f/f0/29047403.pdf Learn how the device is supposed to operate first, then the QEMU device emulation code will make sense. I suggest getting an overview of how the device works without learning all the hardware register details. Then read the datasheet for details on those parts of the device that you will need to modify - there's probably too much information to learn everything about the device, just focus on what you need. Stefan
[Qemu-devel] Thank you very much!
Re: [Qemu-devel] [PATCH v2] Fix dependency issue introduced by commit 7b93fadf3a38d1ed65ea5536a52efc2772c6e3b8
Am 22.02.2012 08:27, schrieb Paolo Bonzini: On 02/22/2012 03:07 AM, 陳韋任 wrote: -HELPERS-$(CONFIG_LINUX) = qemu-bridge-helper$(EXESUF) +HELPERS-$(CONFIG_LINUX) : config-host.h qemu-bridge-helper$(EXESUF) This is not declaring the dependency, it is declaring a target. The rule should be like qemu-bridge-helper.o: config-host.h ...which Peter has already done: http://patchwork.ozlabs.org/patch/142306/ Please coordinate with him. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH] qxl: fix spice+sdl no cursor regression
regression introduced by 075360945860ad9bdd491921954b383bf762b0e5, Reported-by: Fabiano Fidêncio fabi...@fidencio.org Signed-off-by: Alon Levy al...@redhat.com --- Thanks for the report, this fixes the problem for me, the problem with reverting the commit you proposed is that it would reintroduce dropping of the global mutex in spice code. hw/qxl.c |2 ++ ui/spice-display.c | 23 ++- ui/spice-display.h |1 + 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 87ad49a..6f8351c 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1545,6 +1545,8 @@ static void display_refresh(struct DisplayState *ds) { if (qxl0-mode == QXL_MODE_VGA) { qemu_spice_display_refresh(qxl0-ssd); +} else { +qemu_spice_cursor_refresh_unlocked(qxl0-ssd); } } diff --git a/ui/spice-display.c b/ui/spice-display.c index 6c302a3..c6e61d8 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -317,16 +317,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd) ssd-notify++; } -void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd) { -dprint(3, %s:\n, __FUNCTION__); -vga_hw_update(); - -qemu_mutex_lock(ssd-lock); -if (ssd-update == NULL) { -ssd-update = qemu_spice_create_update(ssd); -ssd-notify++; -} if (ssd-cursor) { ssd-ds-cursor_define(ssd-cursor); cursor_put(ssd-cursor); @@ -337,6 +329,19 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) ssd-mouse_x = -1; ssd-mouse_y = -1; } +} + +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +{ +dprint(3, %s:\n, __func__); +vga_hw_update(); + +qemu_mutex_lock(ssd-lock); +if (ssd-update == NULL) { +ssd-update = qemu_spice_create_update(ssd); +ssd-notify++; +} +qemu_spice_cursor_refresh_unlocked(ssd); qemu_mutex_unlock(ssd-lock); if (ssd-notify) { diff --git a/ui/spice-display.h b/ui/spice-display.h index 5e52df9..a23bfc8 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -97,6 +97,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd, int x, int y, int w, int h); void qemu_spice_display_resize(SimpleSpiceDisplay *ssd); void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd); +void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd); void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, qxl_async_io async); -- 1.7.9.1
[Qemu-devel] [RFC] qom: Sorted class enumeration
Hello, For listing registered CPU classes I needed a way to sort classes in a custom (i.e., non-hashtable) order. I found it easiest to sort the classes using the existing foreach infrastructure, on the go via GLib's binary tree. Patch is still missing documentation, but do you think this is the right direction, Anthony? I've been wondering if it might make sense to replace the current filtering mechanism (abstract and type) with another callback function or whether that would be overkill - currently the only other filtering I needed to do was to ignore the host CPU class, which can be done by simple if in the callback. Regards, Andreas Andreas Färber (1): qom: Introduce object_class_foreach_ordered() include/qemu/object.h |6 ++ qom/object.c | 43 +++ 2 files changed, 49 insertions(+), 0 deletions(-) -- 1.7.7
[Qemu-devel] [PATCH RFC] qom: Introduce object_class_foreach_ordered()
This functions allows to walk the classes in a custom order rather than in hashtable order. Signed-off-by: Andreas Färber afaer...@suse.de Cc: Anthony Liguori anth...@codemonkey.ws Cc: Peter Maydell peter.mayd...@linaro.org --- include/qemu/object.h |6 ++ qom/object.c | 43 +++ 2 files changed, 49 insertions(+), 0 deletions(-) diff --git a/include/qemu/object.h b/include/qemu/object.h index 5179c0c..ab3597a 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -560,6 +560,12 @@ ObjectClass *object_class_by_name(const char *typename); void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), const char *implements_type, bool include_abstract, void *opaque); + +void object_class_foreach_ordered(void (*fn)(ObjectClass *klass, void *opaque), + const char *implements_type, + bool include_abstract, void *opaque, + GCompareFunc compfn); + /** * object_ref: * @obj: the object diff --git a/qom/object.c b/qom/object.c index 2f05ca8..dbfd6a4 100644 --- a/qom/object.c +++ b/qom/object.c @@ -570,6 +570,49 @@ void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), g_hash_table_foreach(type_table_get(), object_class_foreach_tramp, data); } +typedef struct OCFSData { +void (*fn)(ObjectClass *klass, void *opaque); +void *opaque; +GTree *classes; +} OCFSData; + +static void object_class_foreach_ordered_sort(ObjectClass *klass, + void *opaque) +{ +OCFSData *data = opaque; + +g_tree_insert(data-classes, klass, NULL); +} + +static gboolean object_class_foreach_ordered_tramp(gpointer key, + gpointer value, + gpointer data) +{ +ObjectClass *klass = key; +OCFSData *d = data; + +d-fn(klass, d-opaque); + +return FALSE; +} + +void object_class_foreach_ordered(void (*fn)(ObjectClass *klass, void *opaque), + const char *implements_type, + bool include_abstract, void *opaque, + GCompareFunc compfn) +{ +OCFSData data = { +.fn = fn, +.opaque = opaque, +}; + +data.classes = g_tree_new(compfn); +object_class_foreach(object_class_foreach_ordered_sort, + implements_type, include_abstract, data); +g_tree_foreach(data.classes, object_class_foreach_ordered_tramp, data); +g_tree_destroy(data.classes); +} + void object_ref(Object *obj) { obj-ref++; -- 1.7.7
[Qemu-devel] [PATCH v2] qxl: fix spice+sdl no cursor regression
regression introduced by 075360945860ad9bdd491921954b383bf762b0e5, v2: lock around qemu_spice_cursor_refresh_unlocked Reported-by: Fabiano Fidêncio fabi...@fidencio.org Signed-off-by: Alon Levy al...@redhat.com --- hw/qxl.c |4 ui/spice-display.c | 23 ++- ui/spice-display.h |1 + 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 87ad49a..db66d7f 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1545,6 +1545,10 @@ static void display_refresh(struct DisplayState *ds) { if (qxl0-mode == QXL_MODE_VGA) { qemu_spice_display_refresh(qxl0-ssd); +} else { +qemu_mutex_lock(qxl0-ssd.lock); +qemu_spice_cursor_refresh_unlocked(qxl0-ssd); +qemu_mutex_unlock(qxl0-ssd.lock); } } diff --git a/ui/spice-display.c b/ui/spice-display.c index 6c302a3..c6e61d8 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -317,16 +317,8 @@ void qemu_spice_display_resize(SimpleSpiceDisplay *ssd) ssd-notify++; } -void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd) { -dprint(3, %s:\n, __FUNCTION__); -vga_hw_update(); - -qemu_mutex_lock(ssd-lock); -if (ssd-update == NULL) { -ssd-update = qemu_spice_create_update(ssd); -ssd-notify++; -} if (ssd-cursor) { ssd-ds-cursor_define(ssd-cursor); cursor_put(ssd-cursor); @@ -337,6 +329,19 @@ void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) ssd-mouse_x = -1; ssd-mouse_y = -1; } +} + +void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd) +{ +dprint(3, %s:\n, __func__); +vga_hw_update(); + +qemu_mutex_lock(ssd-lock); +if (ssd-update == NULL) { +ssd-update = qemu_spice_create_update(ssd); +ssd-notify++; +} +qemu_spice_cursor_refresh_unlocked(ssd); qemu_mutex_unlock(ssd-lock); if (ssd-notify) { diff --git a/ui/spice-display.h b/ui/spice-display.h index 5e52df9..a23bfc8 100644 --- a/ui/spice-display.h +++ b/ui/spice-display.h @@ -97,6 +97,7 @@ void qemu_spice_display_update(SimpleSpiceDisplay *ssd, int x, int y, int w, int h); void qemu_spice_display_resize(SimpleSpiceDisplay *ssd); void qemu_spice_display_refresh(SimpleSpiceDisplay *ssd); +void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd); void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot, qxl_async_io async); -- 1.7.9.1
Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
Am 22.02.2012 02:11, schrieb David Gibson: On Tue, Feb 21, 2012 at 10:09:01AM +0100, Kevin Wolf wrote: Am 21.02.2012 09:35, schrieb Paolo Bonzini: On 02/20/2012 11:50 AM, Alexander Graf wrote: DMAAIOCB *dbs = qemu_aio_get(dma_aio_pool, bs, cb, opaque); -trace_dma_bdrv_io(dbs, bs, sector_num, to_dev); +trace_dma_bdrv_io(dbs, bs, sector_num, dir); Was the trace wrong before or is it now? I don't see its definition changed anywhere. Not sure what you mean. :) trace-events: dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev) dbs=%p bs=%p sector_num=% PRId64 to_dev=%d Ah, damnit. Didn't find that file. I'll resubmit with trace-events updated too. to_dev is declared bool here, and it should also be renamed to dir (the unfortunate thing about DMADirection is that it swaps 0 and 1 compared to bool to_dev... We need to check carefully that all occurrences have been caught.) Yeah. And even more unfortunate that afaict there's no way to make gcc warn on enum-bool conversions. Still there are only about 4 callers of dma_bdrv_io() - nearly everything uses the dma_bdrv_{read,write}() wrappers - so I'm confident I got them all. Re all occurrences: megasas might be affected by such a change, too (patch on the list). Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
On 02/22/2012 10:54 AM, Andreas Färber wrote: Am 22.02.2012 02:11, schrieb David Gibson: On Tue, Feb 21, 2012 at 10:09:01AM +0100, Kevin Wolf wrote: Am 21.02.2012 09:35, schrieb Paolo Bonzini: On 02/20/2012 11:50 AM, Alexander Graf wrote: DMAAIOCB *dbs = qemu_aio_get(dma_aio_pool, bs, cb, opaque); -trace_dma_bdrv_io(dbs, bs, sector_num, to_dev); +trace_dma_bdrv_io(dbs, bs, sector_num, dir); Was the trace wrong before or is it now? I don't see its definition changed anywhere. Not sure what you mean. :) trace-events: dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev) dbs=%p bs=%p sector_num=% PRId64 to_dev=%d Ah, damnit. Didn't find that file. I'll resubmit with trace-events updated too. to_dev is declared bool here, and it should also be renamed to dir (the unfortunate thing about DMADirection is that it swaps 0 and 1 compared to bool to_dev... We need to check carefully that all occurrences have been caught.) Yeah. And even more unfortunate that afaict there's no way to make gcc warn on enum-bool conversions. Still there are only about 4 callers of dma_bdrv_io() - nearly everything uses the dma_bdrv_{read,write}() wrappers - so I'm confident I got them all. Re all occurrences: megasas might be affected by such a change, too (patch on the list). Nope. megasas is using the scsi interface for reading/writing, so it should be insulated against this kind of change. Cheers, Hannes -- Dr. Hannes Reinecke zSeries Storage h...@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
[Qemu-devel] [PATCH 2/5] hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage
Convert variables descr, src and dest from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/pxa2xx_dma.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/pxa2xx_dma.c b/hw/pxa2xx_dma.c index 8ced0dd..0310154 100644 --- a/hw/pxa2xx_dma.c +++ b/hw/pxa2xx_dma.c @@ -18,9 +18,9 @@ #define PXA2XX_DMA_NUM_REQUESTS 75 typedef struct { -target_phys_addr_t descr; -target_phys_addr_t src; -target_phys_addr_t dest; +uint32_t descr; +uint32_t src; +uint32_t dest; uint32_t cmd; uint32_t state; int request; @@ -512,9 +512,9 @@ static VMStateDescription vmstate_pxa2xx_dma_chan = { .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField[]) { -VMSTATE_UINTTL(descr, PXA2xxDMAChannel), -VMSTATE_UINTTL(src, PXA2xxDMAChannel), -VMSTATE_UINTTL(dest, PXA2xxDMAChannel), +VMSTATE_UINT32(descr, PXA2xxDMAChannel), +VMSTATE_UINT32(src, PXA2xxDMAChannel), +VMSTATE_UINT32(dest, PXA2xxDMAChannel), VMSTATE_UINT32(cmd, PXA2xxDMAChannel), VMSTATE_UINT32(state, PXA2xxDMAChannel), VMSTATE_INT32(request, PXA2xxDMAChannel), -- 1.7.4.1
[Qemu-devel] [PATCH 0/5] VMState cleanups
This patchset cleans up and optimizes vmstate implementation. Patch 1 is a trivial bug fixing. Patches 2 and 3 replaces target_phys_addr_t in pxa implementation to uint32_t. Patch 4 moves VMSTATE_UINTTL from hw.h to vmstate.h. Explicit dependency on NEED_CPU_H is droped, I failed to understand why it was presented at all. Patch 5 introduces mechanism to save variable-size buffers. This already received plenty of discusion, but I still have not understand what was the final decision about it. Igor Mitsyanko (5): target-alpha/machine.c: use VMSTATE_UINT64* instead of VMSTATE_UINTTL* hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage vmstate: refactor and move VMSTATE_UINTTL* macro vmstate: introduce get_bufsize entry in VMStateField hw/exynos4210_uart.c | 10 - hw/g364fb.c|7 +- hw/hw.h| 19 hw/m48t59.c|7 +- hw/mac_nvram.c |8 ++- hw/onenand.c |7 +- hw/pxa2xx_dma.c| 12 +- hw/pxa2xx_lcd.c| 12 +- savevm.c | 39 +- target-alpha/machine.c | 34 +++--- vmstate.h | 54 --- 11 files changed, 115 insertions(+), 94 deletions(-) -- 1.7.4.1
[Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/pxa2xx_lcd.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index 9495226..4b712bb 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -19,15 +19,15 @@ #include framebuffer.h struct DMAChannel { -target_phys_addr_t branch; +uint32_t branch; uint8_t up; uint8_t palette[1024]; uint8_t pbuffer[1024]; void (*redraw)(PXA2xxLCDState *s, target_phys_addr_t addr, int *miny, int *maxy); -target_phys_addr_t descriptor; -target_phys_addr_t source; +uint32_t descriptor; +uint32_t source; uint32_t id; uint32_t command; }; @@ -934,11 +934,11 @@ static const VMStateDescription vmstate_dma_channel = { .minimum_version_id = 0, .minimum_version_id_old = 0, .fields = (VMStateField[]) { -VMSTATE_UINTTL(branch, struct DMAChannel), +VMSTATE_UINT32(branch, struct DMAChannel), VMSTATE_UINT8(up, struct DMAChannel), VMSTATE_BUFFER(pbuffer, struct DMAChannel), -VMSTATE_UINTTL(descriptor, struct DMAChannel), -VMSTATE_UINTTL(source, struct DMAChannel), +VMSTATE_UINT32(descriptor, struct DMAChannel), +VMSTATE_UINT32(source, struct DMAChannel), VMSTATE_UINT32(id, struct DMAChannel), VMSTATE_UINT32(command, struct DMAChannel), VMSTATE_END_OF_LIST() -- 1.7.4.1
[Qemu-devel] [PATCH 5/5] vmstate: introduce get_bufsize entry in VMStateField
New get_bufsize field in VMStateField is supposed to help us easily add save/restore support of dynamically allocated buffers in device's states. There are some cases when information about size of dynamically allocated buffer is already presented in specific device's state structure, but in such a form that can not be used with existing VMStateField interface. Currently, we either can get size from another variable in device's state as it is with VMSTATE_VBUFFER_* macros, or we can also multiply value kept in a variable by a constant with VMSTATE_BUFFER_MULTIPLY macro. If we need to perform any other action, we're forced to add additional variable with size information to device state structure with the only intention to use it in VMStateDescription. This approach is not very pretty. Adding extra flags to VMStateFlags enum for every other possible operation with size field seems redundant, and still it would't cover cases when we need to perform a set of operations to get size value. With get_bufsize callback we can calculate size of dynamic array in whichever way we need. We don't need .size_offset field anymore, so we can remove it from VMState Field structure to compensate for extra memory consuption because of get_bufsize addition. Macros VMSTATE_VBUFFER* are modified to use new callback instead of .size_offset. Macro VMSTATE_BUFFER_MULTIPLY and VMFlag VMS_MULTIPLY are removed completely as they are now redundant. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/exynos4210_uart.c | 10 +- hw/g364fb.c |7 ++- hw/m48t59.c |7 ++- hw/mac_nvram.c |8 +++- hw/onenand.c |7 ++- savevm.c | 18 -- vmstate.h| 43 --- 7 files changed, 54 insertions(+), 46 deletions(-) diff --git a/hw/exynos4210_uart.c b/hw/exynos4210_uart.c index 73a9c18..cbd12e8 100644 --- a/hw/exynos4210_uart.c +++ b/hw/exynos4210_uart.c @@ -554,6 +554,13 @@ static void exynos4210_uart_reset(DeviceState *dev) PRINT_DEBUG(UART%d: Rx FIFO size: %d\n, s-channel, s-rx.size); } +static size_t exynos4210_uart_get_datasize(void *opaque, int id) +{ +Exynos4210UartFIFO *s = (Exynos4210UartFIFO *)opaque; + +return s-size; +} + static const VMStateDescription vmstate_exynos4210_uart_fifo = { .name = exynos4210.uart.fifo, .version_id = 1, @@ -562,7 +569,8 @@ static const VMStateDescription vmstate_exynos4210_uart_fifo = { .fields = (VMStateField[]) { VMSTATE_UINT32(sp, Exynos4210UartFIFO), VMSTATE_UINT32(rp, Exynos4210UartFIFO), -VMSTATE_VBUFFER_UINT32(data, Exynos4210UartFIFO, 1, NULL, 0, size), +VMSTATE_VBUFFER(data, Exynos4210UartFIFO, 1, NULL, 0, +exynos4210_uart_get_datasize), VMSTATE_END_OF_LIST() } }; diff --git a/hw/g364fb.c b/hw/g364fb.c index 9c63bdd..bf9aed5 100644 --- a/hw/g364fb.c +++ b/hw/g364fb.c @@ -491,6 +491,11 @@ static int g364fb_post_load(void *opaque, int version_id) return 0; } +static size_t g364fb_get_vramsize(void *opaque, int version_id) +{ +return ((G364State *)opaque)-vram_size; +} + static const VMStateDescription vmstate_g364fb = { .name = g364fb, .version_id = 1, @@ -498,7 +503,7 @@ static const VMStateDescription vmstate_g364fb = { .minimum_version_id_old = 1, .post_load = g364fb_post_load, .fields = (VMStateField[]) { -VMSTATE_VBUFFER_UINT32(vram, G364State, 1, NULL, 0, vram_size), +VMSTATE_VBUFFER(vram, G364State, 1, NULL, 0, g364fb_get_vramsize), VMSTATE_BUFFER_UNSAFE(color_palette, G364State, 0, 256 * 3), VMSTATE_BUFFER_UNSAFE(cursor_palette, G364State, 0, 9), VMSTATE_UINT16_ARRAY(cursor, G364State, 512), diff --git a/hw/m48t59.c b/hw/m48t59.c index 60bbb00..34355c3 100644 --- a/hw/m48t59.c +++ b/hw/m48t59.c @@ -582,6 +582,11 @@ static const MemoryRegionOps nvram_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +static size_t m48t59_get_bufsize(void *opaque, int version_id) +{ +return ((M48t59State *)opaque)-size; +} + static const VMStateDescription vmstate_m48t59 = { .name = m48t59, .version_id = 1, @@ -590,7 +595,7 @@ static const VMStateDescription vmstate_m48t59 = { .fields = (VMStateField[]) { VMSTATE_UINT8(lock, M48t59State), VMSTATE_UINT16(addr, M48t59State), -VMSTATE_VBUFFER_UINT32(buffer, M48t59State, 0, NULL, 0, size), +VMSTATE_VBUFFER(buffer, M48t59State, 0, NULL, 0, m48t59_get_bufsize), VMSTATE_END_OF_LIST() } }; diff --git a/hw/mac_nvram.c b/hw/mac_nvram.c index ed0a2b7..220e745 100644 --- a/hw/mac_nvram.c +++ b/hw/mac_nvram.c @@ -100,13 +100,19 @@ static const MemoryRegionOps macio_nvram_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; +static size_t macio_nvram_get_datasize(void *opaque, int version_id) +{ +return ((MacIONVRAMState *)opaque)-size; +} + static
[Qemu-devel] [PATCH 4/5] vmstate: refactor and move VMSTATE_UINTTL* macro
Instead of defining VMSTATE_UINTTL* based on TARGET_LONG_BITS value, we can use qemu_put_betls/qemu_get_betls functions. These two functions depend on TARGET_LONG_BITS as well, and this new approach for VMSTATE_UINTTL* will result in the same thing as before (will call qemu_get_be32s/qemu_put_be32s for 32-bit target or qemu_put_be64s/qemu_get_be64s for 64-bit target). Move VMSTATE_UINTTL* definitions to vmstate.h where they belong. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/hw.h | 19 --- savevm.c | 21 + vmstate.h | 11 +++ 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/hw/hw.h b/hw/hw.h index e5cb9bf..fb66156 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -46,23 +46,4 @@ typedef int QEMUBootSetHandler(void *opaque, const char *boot_devices); void qemu_register_boot_set(QEMUBootSetHandler *func, void *opaque); int qemu_boot_set(const char *boot_devices); -#ifdef NEED_CPU_H -#if TARGET_LONG_BITS == 64 -#define VMSTATE_UINTTL_V(_f, _s, _v) \ -VMSTATE_UINT64_V(_f, _s, _v) -#define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ -VMSTATE_UINT64_ARRAY_V(_f, _s, _n, _v) -#else -#define VMSTATE_UINTTL_V(_f, _s, _v) \ -VMSTATE_UINT32_V(_f, _s, _v) -#define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ -VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v) -#endif -#define VMSTATE_UINTTL(_f, _s)\ -VMSTATE_UINTTL_V(_f, _s, 0) -#define VMSTATE_UINTTL_ARRAY(_f, _s, _n) \ -VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, 0) - -#endif - #endif diff --git a/savevm.c b/savevm.c index 80be1ff..af33157 100644 --- a/savevm.c +++ b/savevm.c @@ -1081,6 +1081,27 @@ const VMStateInfo vmstate_info_uint16_equal = { .put = put_uint16, }; +/* target unsigned long */ + +static int get_target_ul(QEMUFile *f, void *pv, size_t size) +{ +target_ulong *v = pv; +qemu_get_betls(f, v); +return 0; +} + +static void put_target_ul(QEMUFile *f, void *pv, size_t size) +{ +target_ulong *v = pv; +qemu_put_betls(f, v); +} + +const VMStateInfo vmstate_info_target_ul = { +.name = target ulong, +.get = get_target_ul, +.put = put_target_ul, +}; + /* timers */ static int get_timer(QEMUFile *f, void *pv, size_t size) diff --git a/vmstate.h b/vmstate.h index 9d3c49c..218c303 100644 --- a/vmstate.h +++ b/vmstate.h @@ -130,6 +130,7 @@ extern const VMStateInfo vmstate_info_uint8; extern const VMStateInfo vmstate_info_uint16; extern const VMStateInfo vmstate_info_uint32; extern const VMStateInfo vmstate_info_uint64; +extern const VMStateInfo vmstate_info_target_ul; extern const VMStateInfo vmstate_info_timer; extern const VMStateInfo vmstate_info_buffer; @@ -446,6 +447,8 @@ extern const VMStateInfo vmstate_info_unused_buffer; VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint32, uint32_t) #define VMSTATE_UINT64_V(_f, _s, _v) \ VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint64, uint64_t) +#define VMSTATE_UINTTL_V(_f, _s, _v) \ +VMSTATE_SINGLE(_f, _s, _v, vmstate_info_target_ul, target_ulong) #define VMSTATE_BOOL(_f, _s) \ VMSTATE_BOOL_V(_f, _s, 0) @@ -467,6 +470,8 @@ extern const VMStateInfo vmstate_info_unused_buffer; VMSTATE_UINT32_V(_f, _s, 0) #define VMSTATE_UINT64(_f, _s)\ VMSTATE_UINT64_V(_f, _s, 0) +#define VMSTATE_UINTTL(_f, _s)\ +VMSTATE_UINTTL_V(_f, _s, 0) #define VMSTATE_UINT8_EQUAL(_f, _s) \ VMSTATE_SINGLE(_f, _s, 0, vmstate_info_uint8_equal, uint8_t) @@ -558,6 +563,12 @@ extern const VMStateInfo vmstate_info_unused_buffer; #define VMSTATE_INT64_ARRAY(_f, _s, _n) \ VMSTATE_INT64_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ +VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_target_ul, target_ulong) + +#define VMSTATE_UINTTL_ARRAY(_f, _s, _n) \ +VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, 0) + #define VMSTATE_BUFFER_V(_f, _s, _v) \ VMSTATE_STATIC_BUFFER(_f, _s, _v, NULL, 0, sizeof(typeof_field(_s, _f))) -- 1.7.4.1
[Qemu-devel] [PATCH 1/5] target-alpha/machine.c: use VMSTATE_UINT64* instead of VMSTATE_UINTTL*
Do not use VMSTATE_UINTTL* macro for variables that are actually defined as uint64_t in CPUAlphaState. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- target-alpha/machine.c | 34 +- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/target-alpha/machine.c b/target-alpha/machine.c index 76d70d9..f1eef3d 100644 --- a/target-alpha/machine.c +++ b/target-alpha/machine.c @@ -21,8 +21,8 @@ static const VMStateInfo vmstate_fpcr = { }; static VMStateField vmstate_cpu_fields[] = { -VMSTATE_UINTTL_ARRAY(ir, CPUState, 31), -VMSTATE_UINTTL_ARRAY(fir, CPUState, 31), +VMSTATE_UINT64_ARRAY(ir, CPUState, 31), +VMSTATE_UINT64_ARRAY(fir, CPUState, 31), /* Save the architecture value of the fpcr, not the internally expanded version. Since this architecture value does not exist in memory to be stored, this requires a but of hoop @@ -37,10 +37,10 @@ static VMStateField vmstate_cpu_fields[] = { .flags = VMS_SINGLE, .offset = 0 }, -VMSTATE_UINTTL(pc, CPUState), -VMSTATE_UINTTL(unique, CPUState), -VMSTATE_UINTTL(lock_addr, CPUState), -VMSTATE_UINTTL(lock_value, CPUState), +VMSTATE_UINT64(pc, CPUState), +VMSTATE_UINT64(unique, CPUState), +VMSTATE_UINT64(lock_addr, CPUState), +VMSTATE_UINT64(lock_value, CPUState), /* Note that lock_st_addr is not saved; it is a temporary used during the execution of the st[lq]_c insns. */ @@ -51,19 +51,19 @@ static VMStateField vmstate_cpu_fields[] = { VMSTATE_UINT32(pcc_ofs, CPUState), -VMSTATE_UINTTL(trap_arg0, CPUState), -VMSTATE_UINTTL(trap_arg1, CPUState), -VMSTATE_UINTTL(trap_arg2, CPUState), +VMSTATE_UINT64(trap_arg0, CPUState), +VMSTATE_UINT64(trap_arg1, CPUState), +VMSTATE_UINT64(trap_arg2, CPUState), -VMSTATE_UINTTL(exc_addr, CPUState), -VMSTATE_UINTTL(palbr, CPUState), -VMSTATE_UINTTL(ptbr, CPUState), -VMSTATE_UINTTL(vptptr, CPUState), -VMSTATE_UINTTL(sysval, CPUState), -VMSTATE_UINTTL(usp, CPUState), +VMSTATE_UINT64(exc_addr, CPUState), +VMSTATE_UINT64(palbr, CPUState), +VMSTATE_UINT64(ptbr, CPUState), +VMSTATE_UINT64(vptptr, CPUState), +VMSTATE_UINT64(sysval, CPUState), +VMSTATE_UINT64(usp, CPUState), -VMSTATE_UINTTL_ARRAY(shadow, CPUState, 8), -VMSTATE_UINTTL_ARRAY(scratch, CPUState, 24), +VMSTATE_UINT64_ARRAY(shadow, CPUState, 8), +VMSTATE_UINT64_ARRAY(scratch, CPUState, 24), VMSTATE_END_OF_LIST() }; -- 1.7.4.1
[Qemu-devel] [Bug 938552] [NEW] ENH: Inherit ptys, useful output from -serial pty
Public bug reported: When controlling a qemu instance from another program, it'd be very useful to be able to have qemu inherit pseudo-tty file descriptors so they could just be specified on the command line. It's possible to allocate a pty pair in the master program before forking and exec'ing qemu and have qemu use that pty, but it's a bit painful. The master program must call ptsname(...) on the fd of the slave side and insert the path to the pty device node into qemu's command line. This doesn't work well in many scripting languages which lack a ptsname() call; a Linux-specific hack like readlink() of /proc/self/fd/[slave-fd] is necessary. If qemu accepted file descriptors for serial I/O this would all be a lot more flexible, and it wouldn't be limited to ptys either. Just accept a new format for -serial like -serial fd:7 and have the parent program not set that FD to close-on-exec. None of this would be as necessary if qemu's -serial pty option was fully functional. Unfortunately, it doesn't provide any information to associate the created PTY(s) with their qemu devices, so it's hard to know which serial port is which, which the monitor device is, etc. See, eg: $ qemu -serial pty -serial pty -monitor pty char device redirected to /dev/pts/6 char device redirected to /dev/pts/7 char device redirected to /dev/pts/8 ... which is which? Are they allocated in the order they're specified on the command line? Nope, because /dev/pts/6 is the monitor in this case. With more than one device using pty a lot of guesswork is involved. If you're using -monitor stdio you can issue an info chardev and parse that to find out what everything else is connected to, but this shouldn't really be necessary. Ideally the device names would be printed when a port is redirected to a pty, eg: $ qemu -serial pty -serial pty -monitor pty char device compat_monitor0 redirected to /dev/pts/6 char device serial0 redirected to /dev/pts/7 char device serial1 redirected to /dev/pts/8 ** 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/938552 Title: ENH: Inherit ptys, useful output from -serial pty Status in QEMU: New Bug description: When controlling a qemu instance from another program, it'd be very useful to be able to have qemu inherit pseudo-tty file descriptors so they could just be specified on the command line. It's possible to allocate a pty pair in the master program before forking and exec'ing qemu and have qemu use that pty, but it's a bit painful. The master program must call ptsname(...) on the fd of the slave side and insert the path to the pty device node into qemu's command line. This doesn't work well in many scripting languages which lack a ptsname() call; a Linux-specific hack like readlink() of /proc/self/fd/[slave-fd] is necessary. If qemu accepted file descriptors for serial I/O this would all be a lot more flexible, and it wouldn't be limited to ptys either. Just accept a new format for -serial like -serial fd:7 and have the parent program not set that FD to close-on-exec. None of this would be as necessary if qemu's -serial pty option was fully functional. Unfortunately, it doesn't provide any information to associate the created PTY(s) with their qemu devices, so it's hard to know which serial port is which, which the monitor device is, etc. See, eg: $ qemu -serial pty -serial pty -monitor pty char device redirected to /dev/pts/6 char device redirected to /dev/pts/7 char device redirected to /dev/pts/8 ... which is which? Are they allocated in the order they're specified on the command line? Nope, because /dev/pts/6 is the monitor in this case. With more than one device using pty a lot of guesswork is involved. If you're using -monitor stdio you can issue an info chardev and parse that to find out what everything else is connected to, but this shouldn't really be necessary. Ideally the device names would be printed when a port is redirected to a pty, eg: $ qemu -serial pty -serial pty -monitor pty char device compat_monitor0 redirected to /dev/pts/6 char device serial0 redirected to /dev/pts/7 char device serial1 redirected to /dev/pts/8 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/938552/+subscriptions
Re: [Qemu-devel] [PATCH 2/5] hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage
On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert variables descr, src and dest from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/pxa2xx_dma.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/pxa2xx_dma.c b/hw/pxa2xx_dma.c index 8ced0dd..0310154 100644 --- a/hw/pxa2xx_dma.c +++ b/hw/pxa2xx_dma.c @@ -18,9 +18,9 @@ #define PXA2XX_DMA_NUM_REQUESTS 75 typedef struct { - target_phys_addr_t descr; - target_phys_addr_t src; - target_phys_addr_t dest; + uint32_t descr; + uint32_t src; + uint32_t dest; uint32_t cmd; uint32_t state; int request; @@ -512,9 +512,9 @@ static VMStateDescription vmstate_pxa2xx_dma_chan = { .minimum_version_id = 1, .minimum_version_id_old = 1, .fields = (VMStateField[]) { - VMSTATE_UINTTL(descr, PXA2xxDMAChannel), - VMSTATE_UINTTL(src, PXA2xxDMAChannel), - VMSTATE_UINTTL(dest, PXA2xxDMAChannel), + VMSTATE_UINT32(descr, PXA2xxDMAChannel), + VMSTATE_UINT32(src, PXA2xxDMAChannel), + VMSTATE_UINT32(dest, PXA2xxDMAChannel), VMSTATE_UINT32(cmd, PXA2xxDMAChannel), VMSTATE_UINT32(state, PXA2xxDMAChannel), VMSTATE_INT32(request, PXA2xxDMAChannel), -- Reviewed-by: Peter Maydell peter.mayd...@linaro.org These are clearly intending to model 32 bit registers so we shouldn't be tying them to target_phys_addr_t (which would probably break things if/when we ever move to target_phys_addr_t being 64 bits for all things). -- PMM
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- hw/pxa2xx_lcd.c | 12 ++-- 1 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index 9495226..4b712bb 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -19,15 +19,15 @@ #include framebuffer.h struct DMAChannel { - target_phys_addr_t branch; + uint32_t branch; uint8_t up; uint8_t palette[1024]; uint8_t pbuffer[1024]; void (*redraw)(PXA2xxLCDState *s, target_phys_addr_t addr, int *miny, int *maxy); - target_phys_addr_t descriptor; - target_phys_addr_t source; + uint32_t descriptor; + uint32_t source; uint32_t id; uint32_t command; }; @@ -934,11 +934,11 @@ static const VMStateDescription vmstate_dma_channel = { .minimum_version_id = 0, .minimum_version_id_old = 0, .fields = (VMStateField[]) { - VMSTATE_UINTTL(branch, struct DMAChannel), + VMSTATE_UINT32(branch, struct DMAChannel), VMSTATE_UINT8(up, struct DMAChannel), VMSTATE_BUFFER(pbuffer, struct DMAChannel), - VMSTATE_UINTTL(descriptor, struct DMAChannel), - VMSTATE_UINTTL(source, struct DMAChannel), + VMSTATE_UINT32(descriptor, struct DMAChannel), + VMSTATE_UINT32(source, struct DMAChannel), VMSTATE_UINT32(id, struct DMAChannel), VMSTATE_UINT32(command, struct DMAChannel), VMSTATE_END_OF_LIST() -- 1.7.4.1 Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
Re: [Qemu-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save
On 02/21/12 22:39, Alon Levy wrote: Using vga-screen_dump results in a number of calls to ppm_save, instead of a single one. Can you investigate why? Lacking time to test all the possible users of vga-screen_dump, avoid the redundant calls by doing the vga_hw_update+ ppm_save in qxl_hw_screen_dump. I'd prefer to fix the root cause instead of adding workarounds. cheers, Gerd
Re: [Qemu-devel] [RFC v4 5/9] qxl: require spice = 0.8.2
On 02/21/12 22:39, Alon Levy wrote: drop all ifdefs on SPICE_INTERFACE_QXL_MINOR = 1 as a result, 0.8.2 has SPICE_INTERFACE_QXL_MINOR == 1. Nice. I think there are a few more SPICE_SERVER_VERSION #ifdefs which can be dropped too. Also SPICE_INTERFACE_CORE_MINOR ... cheers, Gerd
Re: [Qemu-devel] [RFC v4 6/9] qxl: remove flipped
Hi, It's not obvious to me how the non-flipped case (qxl_stride 0) is handled now. Have you tested this with both windows+linux guests? cheers, Gerd
Re: [Qemu-devel] [PATCH v2 0/4] RTC: New logic to emulate RTC
On 02/21/2012 01:00 AM, Zhang, Yang Z wrote: Thanks, this looks much better! I'll run it through some tests. We also should try to keep migration working from older versions using the load_old callback. Sure, I missed it. Will add it to the version 3. Any other comments? Here they are. 0) My alarm tests failed quite badly. :( I attach a patch for kvm-unit-tests (repository at git://git.kernel.org/pub/scm/virt/kvm/kvm-unit-tests.git). The tests can be compiled simply with make and run with qemu-kvm -kernel /path/to/x86/rtc.flat -serial stdio -display none. Upstream QEMU fails some of the tests. My branch rtc-cleanup at git://github.com/bonzini/qemu.git passes them. The tests should take 30-40 seconds to run. Currently they hang very early because UF is not set. I attempted to fix that, but ran into other problems. UIP seems not to be really in sync with the update interrupt, because the 500 ms update tests pass when testing UIP, but not when testing UF. (Another reason why I wanted to have the 500 ms rule: it improves reliability of tests!) 1) Alarm must also be handled like update. That is, the timer must be enabled when AF=0 rather than when AIE=1. Again, this is not enough to fix the problems. 2) Instead of using gettimeofday, you should use qemu_get_clock_ns(rtc_clock). If host_clock == rtc_clock it is the same, see get_clock_realtime() in qemu-timer.h. It also has the advantage that you can do all math in nanoseconds and remove the get_ticks_per_sec() / USEC_PER_SEC factors. For example gettimeofday(tv_now, NULL); host_usec = tv_now.tv_sec * USEC_PER_SEC + tv_now.tv_usec; should be simply: host_nsec = qemu_get_clock_ns(rtc_clock); 3) Please move the very complicated alarm logic to a separate function. Ideally, the timer update functions should be as simple as this: static void update_ended_timer_update(RTCState *s) { if ((s-cmos_data[RTC_REG_C] REG_C_UF) == 0 !(s-cmos_data[RTC_REG_B] REG_B_SET)) { qemu_mod_timer(s-update_timer, rtc_update_time(s)); } else { qemu_del_timer(s-update_timer); } } /* handle alarm timer */ static void alarm_timer_update(RTCState *s) { uint64_t next_update_time; uint64_t expire_time; if ((s-cmos_data[RTC_REG_C] REG_C_AF) == 0 !(s-cmos_data[RTC_REG_B] REG_B_SET)) { next_update_time = rtc_update_time(s); expire_time = (rtc_alarm_sec(s) - 1) * NSEC_PER_SEC + next_update_time; qemu_mod_timer(s-alarm_timer, expire_time); } else { qemu_del_timer(s-alarm_timer); } } 4) Related to this, my GCC reports a uninitialized variable at the += 1 here: } else if (cur_min == alarm_min) { if ((s-cmos_data[RTC_SECONDS_ALARM] 0xc0) == 0xc0) { next_alarm_sec += 1; } else if (cur_sec alarm_sec) { next_alarm_sec = alarm_sec - cur_sec; } else { min = alarm_min + MIN_PER_HOUR - cur_min; next_alarm_sec = alarm_sec + min * SEC_PER_MIN - cur_sec; } I think it should be an = 1 instead? 5) As a further cleanup, perhaps you can create hours_from_bcd and hours_to_bcd functions to abstract the 12/24 setting. 6) Setting the clock after 500 ms happens not on every set, but only when moving out of divider reset (register A bits 5-7 moving from 110 or 111 to 010). As far as I can read, SET prevents the registers from changing value, but keeps the internal sub-second counters running. Paolo From f8d5e45941562cd8259523bd225c81a9dd841308 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini pbonz...@redhat.com Date: Tue, 22 Nov 2011 18:53:42 +0100 Subject: [PATCH] add rtc emulation tests --- config-x86-common.mak |4 +- x86/rtc.c | 294 + 2 files changed, 297 insertions(+), 1 deletions(-) create mode 100644 x86/rtc.c diff --git a/config-x86-common.mak b/config-x86-common.mak index a093b8d..348c02a 100644 --- a/config-x86-common.mak +++ b/config-x86-common.mak @@ -34,7 +34,7 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \ $(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \ $(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \ $(TEST_DIR)/kvmclock_test.flat $(TEST_DIR)/eventinj.flat \ - $(TEST_DIR)/s3.flat + $(TEST_DIR)/s3.flat $(TEST_DIR)/rtc.flat ifdef API tests-common += api/api-sample @@ -77,6 +77,8 @@ $(TEST_DIR)/idt_test.elf: $(cstart.o) $(TEST_DIR)/idt_test.o $(TEST_DIR)/xsave.elf: $(cstart.o) $(TEST_DIR)/xsave.o +$(TEST_DIR)/rtc.elf: $(cstart.o) $(TEST_DIR)/rtc.o + $(TEST_DIR)/rmap_chain.elf: $(cstart.o) $(TEST_DIR)/rmap_chain.o $(TEST_DIR)/svm.elf: $(cstart.o) diff --git a/x86/rtc.c b/x86/rtc.c new file mode 100644 index 000..03cf4dc --- /dev/null +++ b/x86/rtc.c @@ -0,0 +1,294 @@ +#include io.h
Re: [Qemu-devel] [PATCH 1/5] target-alpha/machine.c: use VMSTATE_UINT64* instead of VMSTATE_UINTTL*
Looks plausible but cc'ing the Alpha maintainer... -- PMM On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Do not use VMSTATE_UINTTL* macro for variables that are actually defined as uint64_t in CPUAlphaState. Signed-off-by: Igor Mitsyanko i.mitsya...@samsung.com --- target-alpha/machine.c | 34 +- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/target-alpha/machine.c b/target-alpha/machine.c index 76d70d9..f1eef3d 100644 --- a/target-alpha/machine.c +++ b/target-alpha/machine.c @@ -21,8 +21,8 @@ static const VMStateInfo vmstate_fpcr = { }; static VMStateField vmstate_cpu_fields[] = { - VMSTATE_UINTTL_ARRAY(ir, CPUState, 31), - VMSTATE_UINTTL_ARRAY(fir, CPUState, 31), + VMSTATE_UINT64_ARRAY(ir, CPUState, 31), + VMSTATE_UINT64_ARRAY(fir, CPUState, 31), /* Save the architecture value of the fpcr, not the internally expanded version. Since this architecture value does not exist in memory to be stored, this requires a but of hoop @@ -37,10 +37,10 @@ static VMStateField vmstate_cpu_fields[] = { .flags = VMS_SINGLE, .offset = 0 }, - VMSTATE_UINTTL(pc, CPUState), - VMSTATE_UINTTL(unique, CPUState), - VMSTATE_UINTTL(lock_addr, CPUState), - VMSTATE_UINTTL(lock_value, CPUState), + VMSTATE_UINT64(pc, CPUState), + VMSTATE_UINT64(unique, CPUState), + VMSTATE_UINT64(lock_addr, CPUState), + VMSTATE_UINT64(lock_value, CPUState), /* Note that lock_st_addr is not saved; it is a temporary used during the execution of the st[lq]_c insns. */ @@ -51,19 +51,19 @@ static VMStateField vmstate_cpu_fields[] = { VMSTATE_UINT32(pcc_ofs, CPUState), - VMSTATE_UINTTL(trap_arg0, CPUState), - VMSTATE_UINTTL(trap_arg1, CPUState), - VMSTATE_UINTTL(trap_arg2, CPUState), + VMSTATE_UINT64(trap_arg0, CPUState), + VMSTATE_UINT64(trap_arg1, CPUState), + VMSTATE_UINT64(trap_arg2, CPUState), - VMSTATE_UINTTL(exc_addr, CPUState), - VMSTATE_UINTTL(palbr, CPUState), - VMSTATE_UINTTL(ptbr, CPUState), - VMSTATE_UINTTL(vptptr, CPUState), - VMSTATE_UINTTL(sysval, CPUState), - VMSTATE_UINTTL(usp, CPUState), + VMSTATE_UINT64(exc_addr, CPUState), + VMSTATE_UINT64(palbr, CPUState), + VMSTATE_UINT64(ptbr, CPUState), + VMSTATE_UINT64(vptptr, CPUState), + VMSTATE_UINT64(sysval, CPUState), + VMSTATE_UINT64(usp, CPUState), - VMSTATE_UINTTL_ARRAY(shadow, CPUState, 8), - VMSTATE_UINTTL_ARRAY(scratch, CPUState, 24), + VMSTATE_UINT64_ARRAY(shadow, CPUState, 8), + VMSTATE_UINT64_ARRAY(scratch, CPUState, 24), VMSTATE_END_OF_LIST() }; -- 1.7.4.1 -- 12345678901234567890123456789012345678901234567890123456789012345678901234567890 1 2 3 4 5 6 7 8
Re: [Qemu-devel] [RFC v4 7/9] qxl: introduce QXLCookie
On 02/21/12 22:39, Alon Levy wrote: Will be used in the next patch. Looks good, and is more readable without all the #ifdefs. static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl) { -spice_qxl_flush_surfaces_async(qxl-ssd.qxl, 0); + spice_qxl_flush_surfaces_async(qxl-ssd.qxl, +(uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO, + QXL_IO_FLUSH_SURFACES_ASYNC)); minor whitespace issue here ;) cheers, Gerd
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: This patchset cleans up and optimizes vmstate implementation. Patch 1 is a trivial bug fixing. Patches 2 and 3 replaces target_phys_addr_t in pxa implementation to uint32_t. Patch 4 moves VMSTATE_UINTTL from hw.h to vmstate.h. Explicit dependency on NEED_CPU_H is droped, I failed to understand why it was presented at all. So if we apply patches 1-3 (which all look plausible) then the only remaining user of VMSTATE_UINTTL is target-i386/machine.c as far as I can see. This leaves me wondering if we shouldn't just put it actually in target-i386/machine.c as a convenience macro for that specific CPU to avoid having to have more #ifdef TARGET_X86_64s. (I note that the machine.c code is already pretty inconsistent, eg lstar and cstar are defined as target_ulong and saved with VMSTATE_UINT64.) Basically VMSTATE_UINTTL seems like a bit of a dangerous thing to leave lying around as there aren't really very many use cases for it... -- PMM
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Cheers
Re: [Qemu-devel] [RFC v4 8/9] qxl: make qxl_render_update async
Hi, +qxl-render_update_redraw_area.left = 0; +qxl-render_update_redraw_area.right = +qxl-guest_primary.surface.width; +qxl-render_update_redraw_area.top= 0; +qxl-render_update_redraw_area.bottom = +qxl-guest_primary.surface.height; Are there cases where render_update_redraw_area != full screen? +qemu_mutex_lock(qxl-ssd.lock); +if (qxl-render_update_redraw) { +/* don't bother copying them over since we will ignore them */ +qxl-num_dirty_rects += num_updated_rects; +dprint(qxl, 1, %s: scheduling update_area_bh, #dirty %d\n, + __func__, qxl-num_dirty_rects); +qemu_bh_schedule(qxl-update_area_bh); +qemu_mutex_unlock(qxl-ssd.lock); +return; +} +if (qxl-num_dirty_rects + num_updated_rects QXL_NUM_DIRTY_RECTS) { +/* + * overflow - merge all remaining rects. Hoping this is not + * common so doesn't need to be optimized + */ +} Another easy way out is to simply set qxl-render_update_redraw = 1. cheers, Gerd
Re: [Qemu-devel] [RFC v4 9/9] qxl-render: call ppm_save on bh
On 02/21/12 22:39, Alon Levy wrote: This changes the behavior of the monitor command. After the previous patch, there is no longer an option of deadlock with virt-manager, but ppm_save is called too early, before the update has completed. With this patch it is called at the correct moment, but that means there is a race between the monitor command completing and the screendump file being saved. The only solution is to use an asynchronous monitor command. For a previous round of this see: http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02810.html Since that's contentious, I'm suggesting we do something that is almost correct and doesn't hang, instead of correct and hangs. The screendump user can inotify on the directory and the file if need be. For casual monitor usage there is no difference. I still think we should defer that and figure how to fix that properly, either using (internally) async monitor commands via qapi, or using an event. +typedef struct QXLPPMSaveBHData { +PCIQXLDevice *qxl; +QXLCookie *cookie; +} QXLPPMSaveBHData; Is there a need for a separate struct? I think you can just stick the filename into the QXLCookie struct, then write out screen shots in the update area bottom half, no? cheers, Gerd
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 11:36, andrzej zaborowski bal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Yuck. -- PMM
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
On 02/22/2012 03:26 PM, Peter Maydell wrote: On 22 February 2012 10:15, Igor Mitsyankoi.mitsya...@samsung.com wrote: This patchset cleans up and optimizes vmstate implementation. Patch 1 is a trivial bug fixing. Patches 2 and 3 replaces target_phys_addr_t in pxa implementation to uint32_t. Patch 4 moves VMSTATE_UINTTL from hw.h to vmstate.h. Explicit dependency on NEED_CPU_H is droped, I failed to understand why it was presented at all. So if we apply patches 1-3 (which all look plausible) then the only remaining user of VMSTATE_UINTTL is target-i386/machine.c as far as I can see. This leaves me wondering if we shouldn't just put it actually in target-i386/machine.c as a convenience macro for that specific CPU to avoid having to have more #ifdef TARGET_X86_64s. (I note that the machine.c code is already pretty inconsistent, eg lstar and cstar are defined as target_ulong and saved with VMSTATE_UINT64.) Basically VMSTATE_UINTTL seems like a bit of a dangerous thing to leave lying around as there aren't really very many use cases for it... I personally don't really like all these hack VMSTATE macros spread all around QEMU code. I would prefer to have all convenient VMSTATE_*s in one place and eventually replace all file-specific hack macro with standard ones. Not sure that it's worth an effort though. If we're going to move UINTTL* to target-i386/machine.c, then they probably should be implemented in the same way as they are implemented in hw.h now. But without NEED_CPU_H. -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Patch 4 moves VMSTATE_UINTTL from hw.h to vmstate.h. Explicit dependency on NEED_CPU_H is droped, I failed to understand why it was presented at all. You need #ifdef NEED_CPU_H because in generic source files (where NEED_CPU_H is not defined) there is no TARGET_LONG_BITS or target_ulong because you don't know how wide the target CPU virtual addresses are. Only in the source files which are compiled for a specific CPU virtual address size is NEED_CPU_H defined. There's no point in defining these macros in contexts where they can't possibly be used. (This dependence on the CPU vaddr width is why I like the idea of just sticking them in target-i386...) -- PMM
Re: [Qemu-devel] [PATCH v5 12/12] suspend: add qmp events
Hi, Isn't this something where it is easier to omit first and add later once we have a use case, than to add up front only to find that no one cares? Yes. I'll drop it. cheers, Gerd
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 13:00, Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 11:36, andrzej zaborowski bal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. I'm not really discussing that, my question is unrelated to migration/savevm because the patch touches parts that shouldn't be concerned with migration. If a particular function (like migration) needs the type converted to something then that's why C has type conversions. A type conversion that compiles to no code is still a type conversion. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Yuck. Or a general 32-bit memory address type, but as I said uint32_t conveys less information. Do you disagree? Cheers
Re: [Qemu-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save
On Wed, Feb 22, 2012 at 12:10:08PM +0100, Gerd Hoffmann wrote: On 02/21/12 22:39, Alon Levy wrote: Using vga-screen_dump results in a number of calls to ppm_save, instead of a single one. Can you investigate why? oh, I know why. vga_screen_dump implementation: screen_dump_filename = filename; vga_invalidate_display(s); - vga_hw_update(); screen_dump_filename = NULL; And the only user of screen_dump_filename is: static void vga_save_dpy_update(DisplayState *ds, int x, int y, int w, int h) { if (screen_dump_filename) { ppm_save(screen_dump_filename, ds-surface); } } vga_save_dpy_update is called on any dpy_update, registered after the first vga_screen_dump call. Since there are a number of callers to dpy_update: vga_draw_text calls it potentially once every line, vga_draw_graphic the same. vga_draw_blank calls it once. Lacking time to test all the possible users of vga-screen_dump, avoid the redundant calls by doing the vga_hw_update+ ppm_save in qxl_hw_screen_dump. I'd prefer to fix the root cause instead of adding workarounds. This seems to work, only tested with -vga qxl and in vga mode: diff --git a/hw/vga.c b/hw/vga.c index c1029db..51f20c1 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -163,8 +163,6 @@ static uint16_t expand2[256]; static uint8_t expand4to8[16]; static void vga_screen_dump(void *opaque, const char *filename); -static const char *screen_dump_filename; -static DisplayChangeListener *screen_dump_dcl; static void vga_update_memory_access(VGACommonState *s) { @@ -2364,22 +2362,6 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory) // /* vga screen dump */ -static void vga_save_dpy_update(DisplayState *ds, -int x, int y, int w, int h) -{ -if (screen_dump_filename) { -ppm_save(screen_dump_filename, ds-surface); -} -} - -static void vga_save_dpy_resize(DisplayState *s) -{ -} - -static void vga_save_dpy_refresh(DisplayState *s) -{ -} - int ppm_save(const char *filename, struct DisplaySurface *ds) { FILE *f; @@ -2389,10 +2371,12 @@ int ppm_save(const char *filename, struct DisplaySurface *ds) uint8_t r, g, b; int ret; char *linebuf, *pbuf; +static int calls; f = fopen(filename, wb); if (!f) return -1; +fprintf(stderr, ppm_save %d\n, ++calls); fprintf(f, P6\n%d %d\n%d\n, ds-width, ds-height, 255); linebuf = g_malloc(ds-width * 3); @@ -2423,29 +2407,13 @@ int ppm_save(const char *filename, struct DisplaySurface *ds) return 0; } -static DisplayChangeListener* vga_screen_dump_init(DisplayState *ds) -{ -DisplayChangeListener *dcl; - -dcl = g_malloc0(sizeof(DisplayChangeListener)); -dcl-dpy_update = vga_save_dpy_update; -dcl-dpy_resize = vga_save_dpy_resize; -dcl-dpy_refresh = vga_save_dpy_refresh; -register_displaychangelistener(ds, dcl); -return dcl; -} - /* save the vga display in a PPM image even if no display is available */ static void vga_screen_dump(void *opaque, const char *filename) { VGACommonState *s = opaque; -if (!screen_dump_dcl) -screen_dump_dcl = vga_screen_dump_init(s-ds); - -screen_dump_filename = filename; vga_invalidate_display(s); vga_hw_update(); -screen_dump_filename = NULL; +ppm_save(filename, s-ds-surface); } cheers, Gerd
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 02/22/2012 03:36 PM, andrzej zaborowski wrote: On 22 February 2012 11:15, Igor Mitsyankoi.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's obviously more informative, but I thought it's main purpose is to be used with code that could be executed for a different targets (with different address bus width). It's a safe hack, but I don't see the rationale. I don't consider this a hack, we are trying to emulate real hardware, and pxa lcd and dma controllers are intended to work with 32-bit bus. We should not have a possibility to use them with 64-bit targets. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Introducing new type doesn't look pretty to me, maybe just rename variables to source_addr, dest_addr e.t.c? -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
Re: [Qemu-devel] [RFC v4 6/9] qxl: remove flipped
On Wed, Feb 22, 2012 at 12:18:50PM +0100, Gerd Hoffmann wrote: Hi, It's not obvious to me how the non-flipped case (qxl_stride 0) is handled now. Have you tested this with both windows+linux guests? It isn't handled. The simplest way is just to if on the stride and do a single memcpy instead of individual line memcpy. This of course means we are doing a redundant copy, since using our own DisplayAllocator or just the existing deallocate + our own allocate of ds-surface-data removes one copy. Since the main use case is screendump, I don't think it's a big deal. How does that sound? cheers, Gerd
Re: [Qemu-devel] [RFC v4 8/9] qxl: make qxl_render_update async
On Wed, Feb 22, 2012 at 12:41:01PM +0100, Gerd Hoffmann wrote: Hi, +qxl-render_update_redraw_area.left = 0; +qxl-render_update_redraw_area.right = +qxl-guest_primary.surface.width; +qxl-render_update_redraw_area.top= 0; +qxl-render_update_redraw_area.bottom = +qxl-guest_primary.surface.height; Are there cases where render_update_redraw_area != full screen? I don't think so. I'll drop the area after making sure. +qemu_mutex_lock(qxl-ssd.lock); +if (qxl-render_update_redraw) { +/* don't bother copying them over since we will ignore them */ +qxl-num_dirty_rects += num_updated_rects; +dprint(qxl, 1, %s: scheduling update_area_bh, #dirty %d\n, + __func__, qxl-num_dirty_rects); +qemu_bh_schedule(qxl-update_area_bh); +qemu_mutex_unlock(qxl-ssd.lock); +return; +} +if (qxl-num_dirty_rects + num_updated_rects QXL_NUM_DIRTY_RECTS) { +/* + * overflow - merge all remaining rects. Hoping this is not + * common so doesn't need to be optimized + */ +} Another easy way out is to simply set qxl-render_update_redraw = 1. Yes, I agree, wasn't sure what is better. It would avoid a lot of code. I'll do it. cheers, Gerd
Re: [Qemu-devel] [RFC v4 9/9] qxl-render: call ppm_save on bh
On Wed, Feb 22, 2012 at 12:46:28PM +0100, Gerd Hoffmann wrote: On 02/21/12 22:39, Alon Levy wrote: This changes the behavior of the monitor command. After the previous patch, there is no longer an option of deadlock with virt-manager, but ppm_save is called too early, before the update has completed. With this patch it is called at the correct moment, but that means there is a race between the monitor command completing and the screendump file being saved. The only solution is to use an asynchronous monitor command. For a previous round of this see: http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02810.html Since that's contentious, I'm suggesting we do something that is almost correct and doesn't hang, instead of correct and hangs. The screendump user can inotify on the directory and the file if need be. For casual monitor usage there is no difference. I still think we should defer that and figure how to fix that properly, either using (internally) async monitor commands via qapi, or using an event. +typedef struct QXLPPMSaveBHData { +PCIQXLDevice *qxl; +QXLCookie *cookie; +} QXLPPMSaveBHData; Is there a need for a separate struct? I think you can just stick the filename into the QXLCookie struct, then write out screen shots in the update area bottom half, no? You don't get the cookie in the update_area bottom half. It's solemnly for update_area_complete. It's not easy to associate a cookie with it, since it may be called by the server without previous provocation from qemu. I'll take another look. cheers, Gerd
Re: [Qemu-devel] [RFC v4 9/9] qxl-render: call ppm_save on bh
On Wed, Feb 22, 2012 at 12:46:28PM +0100, Gerd Hoffmann wrote: On 02/21/12 22:39, Alon Levy wrote: This changes the behavior of the monitor command. After the previous patch, there is no longer an option of deadlock with virt-manager, but ppm_save is called too early, before the update has completed. With this patch it is called at the correct moment, but that means there is a race between the monitor command completing and the screendump file being saved. The only solution is to use an asynchronous monitor command. For a previous round of this see: http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02810.html Since that's contentious, I'm suggesting we do something that is almost correct and doesn't hang, instead of correct and hangs. The screendump user can inotify on the directory and the file if need be. For casual monitor usage there is no difference. I still think we should defer that and figure how to fix that properly, either using (internally) async monitor commands via qapi, or using an event. I'm looking at QAPI. But in general I think it's better to have a correct image then a timely image. Without this patch you get an old image, unless you do what autotest does and request one every few seconds, then you won't really notice the difference (it would be one frame offset, kinda). +typedef struct QXLPPMSaveBHData { +PCIQXLDevice *qxl; +QXLCookie *cookie; +} QXLPPMSaveBHData; Is there a need for a separate struct? I think you can just stick the filename into the QXLCookie struct, then write out screen shots in the update area bottom half, no? cheers, Gerd
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 12:13, andrzej zaborowski balr...@gmail.com wrote: On 22 February 2012 13:00, Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 11:36, andrzej zaborowski bal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. I'm not really discussing that, my question is unrelated to migration/savevm because the patch touches parts that shouldn't be concerned with migration. If a particular function (like migration) needs the type converted to something then that's why C has type conversions. A type conversion that compiles to no code is still a type conversion. Well, target_phys_addr_t is the wrong type here because it's really at least as large as the widest address type in the system (cf proposals to make it 64 bits), so using it for a register that must be exactly 32 bits wide is wrong. So we need to change it to something, and customarily what we use for I am modelling a physical register which is 32 bits wide is uint32_t. Introducing extra device-specific typedefs to try to label the semantics of device registers seems a bit unnecessary to me. So we need to change the type, and we also need to change the VMSTATE macro for reasons explained earlier, and we need to make sure the macro and the struct agree about the type they are dealing with, so it's simplest to do it all in one patch. If you're complaining that the VMSTATE macros don't provide a way for you to say do this cast then I agree that that is a slightly irritating omission but that's the infrastructure we have... -- PMM
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 13:26, Mitsyanko Igor i.mitsya...@samsung.com wrote: On 02/22/2012 03:36 PM, andrzej zaborowski wrote: Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's obviously more informative, but I thought it's main purpose is to be used with code that could be executed for a different targets (with different address bus width). In some case for sure, but I believe not in most cases. It's a safe hack, but I don't see the rationale. I don't consider this a hack, we are trying to emulate real hardware, and pxa lcd and dma controllers are intended to work with 32-bit bus. We should not have a possibility to use them with 64-bit targets. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Introducing new type doesn't look pretty to me, Why? maybe just rename variables to source_addr, dest_addr e.t.c? Wouldn't it be analogous to changing pointer typed variables to void * and adding the actual type in their names? The result is that at language level they'll all be the same type even though they are not. (or changing le32 and be32 to uint32 in Linux) Cheers
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
Am 22.02.2012 12:26, schrieb Peter Maydell: So if we apply patches 1-3 (which all look plausible) then the only remaining user of VMSTATE_UINTTL is target-i386/machine.c as far as I can see. This leaves me wondering if we shouldn't just put it actually in target-i386/machine.c as a convenience macro for that specific CPU to avoid having to have more #ifdef TARGET_X86_64s. Nack. I don't see the connection between target_ulong and TARGET_X86_64. Just because that becomes the only user does not mean target_ulong is an x86-specific concept. (I note that the machine.c code is already pretty inconsistent, eg lstar and cstar are defined as target_ulong and saved with VMSTATE_UINT64.) Same for TCGv. We also have quite a few mixes of int and (U)INT32. Andreas Basically VMSTATE_UINTTL seems like a bit of a dangerous thing to leave lying around as there aren't really very many use cases for it... -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
On 22 February 2012 12:49, Andreas Färber afaer...@suse.de wrote: Am 22.02.2012 12:26, schrieb Peter Maydell: So if we apply patches 1-3 (which all look plausible) then the only remaining user of VMSTATE_UINTTL is target-i386/machine.c as far as I can see. This leaves me wondering if we shouldn't just put it actually in target-i386/machine.c as a convenience macro for that specific CPU to avoid having to have more #ifdef TARGET_X86_64s. Nack. I don't see the connection between target_ulong and TARGET_X86_64. Just because that becomes the only user does not mean target_ulong is an x86-specific concept. Yeah, I guess. I would like the macro to be protected so you can only get at it if you're target-*/ though... -- PMM
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 22 February 2012 13:48, Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 12:13, andrzej zaborowski balr...@gmail.com wrote: On 22 February 2012 13:00, Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 11:36, andrzej zaborowski bal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. I'm not really discussing that, my question is unrelated to migration/savevm because the patch touches parts that shouldn't be concerned with migration. If a particular function (like migration) needs the type converted to something then that's why C has type conversions. A type conversion that compiles to no code is still a type conversion. Well, target_phys_addr_t is the wrong type here because it's really at least as large as the widest address type in the system (cf proposals to make it 64 bits), so using it for a register that must be exactly 32 bits wide is wrong. So we need to change it to something, and customarily what we use for I am modelling a physical register which is 32 bits wide is uint32_t. Introducing extra device-specific typedefs to try to label the semantics of device registers seems a bit unnecessary to me. If we treat the struct as a representation of the register values rather than state of the emulated device then I guess you're right. The reason it rings an alarm is that the change is not an improvement (other than for migration, but again the change is in code that is not related to savevm) Cheers
Re: [Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event
On Mon, 20 Feb 2012 09:28:15 +0100 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: The event name changed, which caused the subject to change too, hope this won't cause confusion. v3 o Rename the event to DEVICE_TRAY_MOVED o Rename the 'ejected' event data to 'tray-open' o Only call bdrv_eject() if the tray state changed o Drop ide_tray_state_post_load() Reviewed-by: Markus Armbruster arm...@redhat.com Kevin, I'd like to get your ack before applying it to the qmp branch.
Re: [Qemu-devel] [PATCH 6/6] qmp: add balloon-get-memory-stats event
On Fri, 17 Feb 2012 15:51:33 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: On Fri, Feb 17, 2012 at 03:16:22PM -0200, Luiz Capitulino wrote: On Fri, 17 Feb 2012 10:55:41 -0600 Anthony Liguori aligu...@us.ibm.com wrote: On 02/08/2012 02:30 PM, Luiz Capitulino wrote: This commit adds a QMP API for the guest provided memory statistics (long disabled by commit 07b0403dfc2b2ac179ae5b48105096cc2d03375a). The approach taken by the original commit (625a5befc2e3200b396594f002218d235e375da5) was to extend the query-balloon command. It introduced a severe bug though: query-balloon would hang if the guest didn't respond. The approach taken by this commit is asynchronous and thus avoids any QMP hangs. First, a client has to issue the balloon-get-memory-stats command. That command gets the process started by only sending a request to the guest, it doesn't block. When the memory stats are made available by the guest, they are returned to the client as an QMP event. Signed-off-by: Luiz Capitulinolcapitul...@redhat.com Do we need this to be stable in 1.1? Well, this is disabled for a long time already and libvirt needs it, so I'd say asap, but isn't it possible to implement this with current QOM? We can do this pretty nicely through QOM. We can have a polling property in the virtio-balloon driver, that when set, will enable the virtio-balloon device to poll the guest for statistics. We can also have properties for each of the memory statistics and a timestamp for when the last update was. I think this is a friendlier approach for clients, and a cleaner approach from a QEMU perspective. I agree it's friendlier, but is it a good idea to keep polling the guest for something that may never be needed by a mngt app (real question)? Probably not, but then again you'd only need like 1-second granularity. I've talked with Anthony by irc about the implementation details of this and it will be possible to enable/disable the polling, so this is not an issue anymore. Also, I think we can do away with the polling once async QMP is in place, so we wouldn't be stuck with it necessarilly. This is what this series does :) I don't think it's necessary to wait for async support, we're accepting ad-hoc async mechanisms for other commands and could use one here too if needed. We could allow the mngt app to do the polling by adding a query-balloon-stats command (instead of balloon-get-memory-stats event). This command could return the latest available stats if any (with a timestamp) and query the guest for new stats. The downside there is you could read some really stale data that way, to the point where any app that really cared would likely throw out the first result. Having stale data will be possible with any timer based polling...
Re: [Qemu-devel] [RFC] Next gen kvm api
On Sat, 2012-02-04 at 11:08 +0900, Takuya Yoshikawa wrote: The latter needs a fundamental change: I heard (from Avi) that we can change mmu_lock to mutex_lock if mmu_notifier becomes preemptible. So I was planning to restart this work when Peter's mm: Preemptibility http://lkml.org/lkml/2011/4/1/141 gets finished. That got merged a while ago: # git describe --contains d16dfc550f5326a4000f3322582a7c05dec91d7a --match v* v3.0-rc1~275 While I still need to get back to unifying mmu_gather across architectures the whole thing is currently preemptible.
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
On Tue, 21 Feb 2012 19:40:16 +0200 Alon Levy al...@redhat.com wrote: On Tue, Feb 21, 2012 at 09:15:45AM -0700, Eric Blake wrote: On 02/21/2012 01:19 AM, Alon Levy wrote: (2) Async monitor command. Keeps interface and works nicely. A bunch of QAPI bits tickled into master meanwhile, so we could look at this again. Luiz? What is the status here? The qapi infra is already in place for sometime now, but it doesn't support async commands yet. However, we're accepting a combination of command + async event on completion for commands that have to be async. We'll eventually have a good interface for async support, but it's not likely we'll have it for 1.1 (possible, but unlikely). I think item 2 above is a good way to go, considering it will add a new command, of course. Luiz said that this interface is going to be dropped, so we don't want to introduce another user to it now. Please don't :)
Re: [Qemu-devel] [PATCH 5/5] qmp: add DEVICE_TRAY_MOVED event
Am 17.02.2012 20:21, schrieb Luiz Capitulino: It's emitted whenever the tray is moved by the guest or by HMP/QMP commands. Signed-off-by: Luiz Capitulino lcapitul...@redhat.com --- QMP/qmp-events.txt | 18 ++ block.c| 24 monitor.c |3 +++ monitor.h |1 + 4 files changed, 46 insertions(+), 0 deletions(-) diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt index 06cb404..9286af5 100644 --- a/QMP/qmp-events.txt +++ b/QMP/qmp-events.txt @@ -26,6 +26,24 @@ Example: Note: If action is stop, a STOP event will eventually follow the BLOCK_IO_ERROR event. +DEVICE_TRAY_MOVED +- + +It's emitted whenever the tray of a removable device is moved by the guest +or by HMP/QMP commands. + +Data: + +- device: device name (json-string) For me, a device name is something related to qdev. 'device' is a misnomer consistently used in all QMP commands so far and we can't fix it any more, but at least the documentation should clarify what is meant (that's for a follow-up patch). Kevin
Re: [Qemu-devel] [PATCH v3 0/5]: QMP: add DEVICE_TRAY_MOVED event
Am 22.02.2012 13:53, schrieb Luiz Capitulino: On Mon, 20 Feb 2012 09:28:15 +0100 Markus Armbruster arm...@redhat.com wrote: Luiz Capitulino lcapitul...@redhat.com writes: The event name changed, which caused the subject to change too, hope this won't cause confusion. v3 o Rename the event to DEVICE_TRAY_MOVED o Rename the 'ejected' event data to 'tray-open' o Only call bdrv_eject() if the tray state changed o Drop ide_tray_state_post_load() Reviewed-by: Markus Armbruster arm...@redhat.com Kevin, I'd like to get your ack before applying it to the qmp branch. Acked-by: Kevin Wolf kw...@redhat.com
Re: [Qemu-devel] [PATCH v5 12/12] suspend: add qmp events
On Wed, 22 Feb 2012 13:08:16 +0100 Gerd Hoffmann kra...@redhat.com wrote: Hi, Isn't this something where it is easier to omit first and add later once we have a use case, than to add up front only to find that no one cares? Yes. I'll drop it. Yes, I think it's better to drop it for now and add it back if really needed, when we'll know better how mngt apps are going to use this.
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
On Wed, Feb 22, 2012 at 11:17:17AM -0200, Luiz Capitulino wrote: On Tue, 21 Feb 2012 19:40:16 +0200 Alon Levy al...@redhat.com wrote: On Tue, Feb 21, 2012 at 09:15:45AM -0700, Eric Blake wrote: On 02/21/2012 01:19 AM, Alon Levy wrote: (2) Async monitor command. Keeps interface and works nicely. A bunch of QAPI bits tickled into master meanwhile, so we could look at this again. Luiz? What is the status here? The qapi infra is already in place for sometime now, but it doesn't support async commands yet. However, we're accepting a combination of command + async event on completion for commands that have to be async. We'll eventually have a good interface for async support, but it's not likely we'll have it for 1.1 (possible, but unlikely). I think item 2 above is a good way to go, considering it will add a new command, of course. Ok, so that sounds good: I'll add an event, and later libvirt/autotest can use it. But in that case I'll need to introduce a connection between the command and the event, some id. That id will have to be generated by the command issuer, so a new command with event id + complete event? Luiz said that this interface is going to be dropped, so we don't want to introduce another user to it now. Please don't :)
Re: [Qemu-devel] [RFC] qom: Sorted class enumeration
On 02/22/2012 03:44 AM, Andreas Färber wrote: Hello, For listing registered CPU classes I needed a way to sort classes in a custom (i.e., non-hashtable) order. I found it easiest to sort the classes using the existing foreach infrastructure, on the go via GLib's binary tree. Patch is still missing documentation, but do you think this is the right direction, Anthony? I think it might be better to have an object_class_search() function that acts like object_class_foreach() but returns a GSList. You can then call g_list_sort() on the resulting data structure. I've been wondering if it might make sense to replace the current filtering mechanism (abstract and type) with another callback function or whether that would be overkill - currently the only other filtering I needed to do was to ignore the host CPU class, which can be done by simple if in the callback. I'd prefer not to do callbacks because this interface gets exposed via QMP and callbacks can't be modeled via QMP. Regards, Anthony Liguori Regards, Andreas Andreas Färber (1): qom: Introduce object_class_foreach_ordered() include/qemu/object.h |6 ++ qom/object.c | 43 +++ 2 files changed, 49 insertions(+), 0 deletions(-)
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 02/22/2012 04:48 PM, andrzej zaborowski wrote: On 22 February 2012 13:26, Mitsyanko Igori.mitsya...@samsung.com wrote: On 02/22/2012 03:36 PM, andrzej zaborowski wrote: Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's obviously more informative, but I thought it's main purpose is to be used with code that could be executed for a different targets (with different address bus width). In some case for sure, but I believe not in most cases. It's a safe hack, but I don't see the rationale. I don't consider this a hack, we are trying to emulate real hardware, and pxa lcd and dma controllers are intended to work with 32-bit bus. We should not have a possibility to use them with 64-bit targets. If it's because VMSTATE_UINT32 requires that specific type than a less ugly hack would be to make a pxa specific memory address type. Introducing new type doesn't look pretty to me, Why? Peter already answered, this fields should be exactly 32-bit wide (hardware is implemented this way) and we already have a type that is exactly 32-bit wide. Implementing each device without any assumptions about other parts of emulated system seems like a right approach to me. Doing something like typedef uint32_t pxalcd_phys_addr_t is fun, but then we could end up introducing typedefs like this for every device in hw/. Also, currently we can't save custom types in VMSTATE. maybe just rename variables to source_addr, dest_addr e.t.c? Wouldn't it be analogous to changing pointer typed variables to void * and adding the actual type in their names? The result is that at language level they'll all be the same type even though they are not. (or changing le32 and be32 to uint32 in Linux) Yes, but you got to admit they would be more informative for human :) -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
On 02/22/2012 04:56 PM, andrzej zaborowski wrote: On 22 February 2012 13:48, Peter Maydellpeter.mayd...@linaro.org wrote: On 22 February 2012 12:13, andrzej zaborowskibalr...@gmail.com wrote: On 22 February 2012 13:00, Peter Maydellpeter.mayd...@linaro.org wrote: On 22 February 2012 11:36, andrzej zaborowskibal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyankoi.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. I'm not really discussing that, my question is unrelated to migration/savevm because the patch touches parts that shouldn't be concerned with migration. If a particular function (like migration) needs the type converted to something then that's why C has type conversions. A type conversion that compiles to no code is still a type conversion. Well, target_phys_addr_t is the wrong type here because it's really at least as large as the widest address type in the system (cf proposals to make it 64 bits), so using it for a register that must be exactly 32 bits wide is wrong. So we need to change it to something, and customarily what we use for I am modelling a physical register which is 32 bits wide is uint32_t. Introducing extra device-specific typedefs to try to label the semantics of device registers seems a bit unnecessary to me. If we treat the struct as a representation of the register values rather than state of the emulated device then I guess you're right. The reason it rings an alarm is that the change is not an improvement (other than for migration, but again the change is in code that is not related to savevm) Cheers It's an improvement in a way that it fixes a (style) bug in code, VMSTATE_UINTTL* macro are not intended for target_phys_addr_t. -- Mitsyanko Igor ASWG, Moscow RD center, Samsung Electronics email: i.mitsya...@samsung.com
Re: [Qemu-devel] [PATCH 2/5] hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage
Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert variables descr, src and dest from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). This is an incompatible change, we need to bump the version. Looking at pxa2xx_dma_descriptor_fetch() it looks like your change is enough: uint32_t desc[4]; s-chan[ch].descr = desc[DDADR]; s-chan[ch].src = desc[DSADR]; s-chan[ch].dest = desc[DTADR]; s-chan[ch].cmd = desc[DCMD]; i.e. we always asign from a 32bit register. In general change is not valid. As I don't know if this device can appear as 64bit hardware, I don't know if the change is valid and let it to the ARM gurus. Later, Juan.
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
On Wed, 22 Feb 2012 14:22:50 +0100 Alon Levy al...@redhat.com wrote: On Wed, Feb 22, 2012 at 11:17:17AM -0200, Luiz Capitulino wrote: On Tue, 21 Feb 2012 19:40:16 +0200 Alon Levy al...@redhat.com wrote: On Tue, Feb 21, 2012 at 09:15:45AM -0700, Eric Blake wrote: On 02/21/2012 01:19 AM, Alon Levy wrote: (2) Async monitor command. Keeps interface and works nicely. A bunch of QAPI bits tickled into master meanwhile, so we could look at this again. Luiz? What is the status here? The qapi infra is already in place for sometime now, but it doesn't support async commands yet. However, we're accepting a combination of command + async event on completion for commands that have to be async. We'll eventually have a good interface for async support, but it's not likely we'll have it for 1.1 (possible, but unlikely). I think item 2 above is a good way to go, considering it will add a new command, of course. Ok, so that sounds good: I'll add an event, and later libvirt/autotest can use it. But in that case I'll need to introduce a connection between the command and the event, some id. That id will have to be generated by the command issuer, so a new command with event id + complete event? That's a good question. The only events we have today which are actually a response to an asynchronous command are the block streaming API ones and they don't include an id. Honestly, for this particular case, I'm not 100% sure that having an id is _required_, as I don't expect a client to submit multiple screendump calls in parallel and we don't officially support multiple QMP clients either. Also, having the screendump filename in the event will serve as a form of identifier too. Btw, are you planning to add the event to the already existing screendump command? Adding a new command that doesn't use the monitor async API and is truly asynchronous wouldn't better?
Re: [Qemu-devel] [PATCH 1/5] target-alpha/machine.c: use VMSTATE_UINT64* instead of VMSTATE_UINTTL*
Igor Mitsyanko i.mitsya...@samsung.com wrote: Do not use VMSTATE_UINTTL* macro for variables that are actually defined as uint64_t in CPUAlphaState. Old code is wrong (TM). I think that your changes are ok.
Re: [Qemu-devel] [PATCH 2/5] hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage
On 22 February 2012 13:47, Juan Quintela quint...@redhat.com wrote: Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert variables descr, src and dest from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). This is an incompatible change, we need to bump the version. Why? For the cases where this device is used, target_phys_addr_t is always 32 bits so we aren't changing anything, surely? -- PMM
Re: [Qemu-devel] [PATCH 3/5] hw/pxa2xx_lcd.c: drop VMSTATE_UINTTL usage
andrzej zaborowski balr...@gmail.com wrote: On 22 February 2012 13:00, Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 11:36, andrzej zaborowski bal...@zabor.org wrote: On 22 February 2012 11:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert three variables in DMAChannel state from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; Why's uint32_t more correct though? The purpose of using a named type across qemu is to mark fields as memory addresses (similar to size_t being used for sizes, etc.), uint32_t conveys less information -- only the size. It's a safe hack, but I don't see the rationale. Because we might change target_phys_addr_t to 64 bits globally some day (it's certainly been mooted) and that shouldn't suddenly change the register width and certainly shouldn't change the migration state. Basically VMSTATE_UINTTL in hw/ is always a bug, because its behaviour depends on the size of target_ulong, which is a property of the CPU, which is a completely separate device. I'm not really discussing that, my question is unrelated to migration/savevm because the patch touches parts that shouldn't be concerned with migration. If a particular function (like migration) needs the type converted to something then that's why C has type conversions. A type conversion that compiles to no code is still a type conversion. For migration, UINTTL is _always_ wrong, we need to handle it that way for backward compatibility. #if TARGET_LONG_BITS == 64 #define VMSTATE_UINTTL_V(_f, _s, _v) \ VMSTATE_UINT64_V(_f, _s, _v) #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ VMSTATE_UINT64_ARRAY_V(_f, _s, _n, _v) #else #define VMSTATE_UINTTL_V(_f, _s, _v) \ VMSTATE_UINT32_V(_f, _s, _v) #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v) #endif this was done for CPU's, not devices. If we use this, we can't access a device state without knowing the long-iness (don't you like the word) of the cpu that it is running. IMHO, something that always sent 64bit, and on reception if target_long is 32bit and value don't fit - just break migration makes much more sense. But that can only be done for new code :-( On the other hand, I understand andrzej, we are missig a target_phys_addr32_t or whatever we want to call it, that is for devices that _only_ support 32bit addressing. That is something that is valuable to document, independently of how migration is done. Later, Juan.
Re: [Qemu-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save
And the only user of screen_dump_filename is: static void vga_save_dpy_update(DisplayState *ds, int x, int y, int w, int h) { if (screen_dump_filename) { ppm_save(screen_dump_filename, ds-surface); upstream/master this here: screen_dump_filename = NULL; } } This seems to work, only tested with -vga qxl and in vga mode: The corner case where this fails is when console switching is needed, i.e. switch to monitor console via ctrl-alt-2, then type the screenshot command there ...
Re: [Qemu-devel] [PATCH 4/5] vmstate: refactor and move VMSTATE_UINTTL* macro
Igor Mitsyanko i.mitsya...@samsung.com wrote: Instead of defining VMSTATE_UINTTL* based on TARGET_LONG_BITS value, we can use qemu_put_betls/qemu_get_betls functions. These two functions depend on TARGET_LONG_BITS as well, and this new approach for VMSTATE_UINTTL* will result in the same thing as before (will call qemu_get_be32s/qemu_put_be32s for 32-bit target or qemu_put_be64s/qemu_get_be64s for 64-bit target). Move VMSTATE_UINTTL* definitions to vmstate.h where they belong. The idea was to removed the other functions. Notice that the cases are equivalent. i.e. this just bring us a new type that we have to represent, maintain. The other makes use to use a define, take your poison. Later, Juan.
Re: [Qemu-devel] [PATCH 0/5] VMState cleanups
Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 10:15, Igor Mitsyanko i.mitsya...@samsung.com wrote: This patchset cleans up and optimizes vmstate implementation. Patch 1 is a trivial bug fixing. Patches 2 and 3 replaces target_phys_addr_t in pxa implementation to uint32_t. Patch 4 moves VMSTATE_UINTTL from hw.h to vmstate.h. Explicit dependency on NEED_CPU_H is droped, I failed to understand why it was presented at all. So if we apply patches 1-3 (which all look plausible) then the only remaining user of VMSTATE_UINTTL is target-i386/machine.c as far as I can see. This leaves me wondering if we shouldn't just put it actually in target-i386/machine.c as a convenience macro for that specific CPU to avoid having to have more #ifdef TARGET_X86_64s. (I note that the machine.c code is already pretty inconsistent, eg lstar and cstar are defined as target_ulong and saved with VMSTATE_UINT64.) With my cpu-vmstate patches, all 32/64 bit cpus use it. ppc, sparc and mips use it. Move it to a place that is only used for cpus makes sense, though. Basically VMSTATE_UINTTL seems like a bit of a dangerous thing to leave lying around as there aren't really very many use cases for it... -- PMM
Re: [Qemu-devel] [PATCH 2/5] hw/pxa2xx_dma.c: drop VMSTATE_UINTTL usage
Peter Maydell peter.mayd...@linaro.org wrote: On 22 February 2012 13:47, Juan Quintela quint...@redhat.com wrote: Igor Mitsyanko i.mitsya...@samsung.com wrote: Convert variables descr, src and dest from type target_phys_addr_t to uint32_t, use VMSTATE_UINT32 instead of VMSTATE_UINTTL for these variables. We can do it safely because: 1) pxa2xx has 32-bit physical address; 2) rest of the code in this file treats these variables as uint32_t; 3) we shouldn't have used VMSTATE_UINTTL in the first place because this macro is for target_ulong type (which can be different from target_phys_addr_t). This is an incompatible change, we need to bump the version. Why? For the cases where this device is used, target_phys_addr_t is always 32 bits so we aren't changing anything, surely? You are right, I had jsut forgot that this device is only used in ARM. I stand corrected. Later, Juan.
Re: [Qemu-devel] [RFC v4 6/9] qxl: remove flipped
On 02/22/12 13:28, Alon Levy wrote: On Wed, Feb 22, 2012 at 12:18:50PM +0100, Gerd Hoffmann wrote: Hi, It's not obvious to me how the non-flipped case (qxl_stride 0) is handled now. Have you tested this with both windows+linux guests? It isn't handled. The simplest way is just to if on the stride and do a single memcpy instead of individual line memcpy. Single memcpy works only for full scanlines. qxl_flip can be extended to handle both cases (and should probably also renamed then). This of course means we are doing a redundant copy, No. You can wrap the qxl_flip call into ... if (is_shared_buffer()) { ... }. ... to skip the copy if it isn't needed. since using our own DisplayAllocator or just the existing deallocate + our own allocate of ds-surface-data removes one copy. I would just do if (qxl_stride 0) { qemu_free_displaysurface qemu_create_displaysurface_from } else { qemu_resize_displaysurface } cheers, Gerd
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
Hi, Honestly, for this particular case, I'm not 100% sure that having an id is _required_, as I don't expect a client to submit multiple screendump calls in parallel and we don't officially support multiple QMP clients either. Also, having the screendump filename in the event will serve as a form of identifier too. That is exactly my thinking, echo the filename written in the event. Btw, are you planning to add the event to the already existing screendump command? Adding a new command that doesn't use the monitor async API and is truly asynchronous wouldn't better? Good question. I'd tend to just let the existing command send trigger an event. But libvirt needs some way to figure whenever it should wait for an event ... cheers, Gerd
Re: [Qemu-devel] [RFC v4 6/9] qxl: remove flipped
On Wed, Feb 22, 2012 at 03:09:09PM +0100, Gerd Hoffmann wrote: On 02/22/12 13:28, Alon Levy wrote: On Wed, Feb 22, 2012 at 12:18:50PM +0100, Gerd Hoffmann wrote: Hi, It's not obvious to me how the non-flipped case (qxl_stride 0) is handled now. Have you tested this with both windows+linux guests? It isn't handled. The simplest way is just to if on the stride and do a single memcpy instead of individual line memcpy. Single memcpy works only for full scanlines. qxl_flip can be extended to handle both cases (and should probably also renamed then). This of course means we are doing a redundant copy, No. You can wrap the qxl_flip call into ... if (is_shared_buffer()) { ... }. ... to skip the copy if it isn't needed. since using our own DisplayAllocator or just the existing deallocate + our own allocate of ds-surface-data removes one copy. I would just do if (qxl_stride 0) { qemu_free_displaysurface qemu_create_displaysurface_from } else { qemu_resize_displaysurface } hmm. Yes, I know we can do that since it already does it right now. I guess with the console_select call gone it should be ok. cheers, Gerd
Re: [Qemu-devel] [Spice-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save
On Wed, Feb 22, 2012 at 02:58:10PM +0100, Gerd Hoffmann wrote: And the only user of screen_dump_filename is: static void vga_save_dpy_update(DisplayState *ds, int x, int y, int w, int h) { if (screen_dump_filename) { ppm_save(screen_dump_filename, ds-surface); upstream/master this here: screen_dump_filename = NULL; That's wrong, you'll get the screenshot after the first update, who's to say it is fully rendered? } } This seems to work, only tested with -vga qxl and in vga mode: The corner case where this fails is when console switching is needed, i.e. switch to monitor console via ctrl-alt-2, then type the screenshot command there ... Are you talking about sdl console or linux guest console? ctrl-alt-2 or ctrl-alt-F2? I can try both. ___ Spice-devel mailing list spice-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
On Wed, Feb 22, 2012 at 11:49:27AM -0200, Luiz Capitulino wrote: On Wed, 22 Feb 2012 14:22:50 +0100 Alon Levy al...@redhat.com wrote: On Wed, Feb 22, 2012 at 11:17:17AM -0200, Luiz Capitulino wrote: On Tue, 21 Feb 2012 19:40:16 +0200 Alon Levy al...@redhat.com wrote: On Tue, Feb 21, 2012 at 09:15:45AM -0700, Eric Blake wrote: On 02/21/2012 01:19 AM, Alon Levy wrote: (2) Async monitor command. Keeps interface and works nicely. A bunch of QAPI bits tickled into master meanwhile, so we could look at this again. Luiz? What is the status here? The qapi infra is already in place for sometime now, but it doesn't support async commands yet. However, we're accepting a combination of command + async event on completion for commands that have to be async. We'll eventually have a good interface for async support, but it's not likely we'll have it for 1.1 (possible, but unlikely). I think item 2 above is a good way to go, considering it will add a new command, of course. Ok, so that sounds good: I'll add an event, and later libvirt/autotest can use it. But in that case I'll need to introduce a connection between the command and the event, some id. That id will have to be generated by the command issuer, so a new command with event id + complete event? That's a good question. The only events we have today which are actually a response to an asynchronous command are the block streaming API ones and they don't include an id. Honestly, for this particular case, I'm not 100% sure that having an id is _required_, as I don't expect a client to submit multiple screendump calls in parallel and we don't officially support multiple QMP clients either. Also, having the screendump filename in the event will serve as a form of identifier too. Btw, are you planning to add the event to the already existing screendump command? Adding a new command that doesn't use the monitor async API and is truly asynchronous wouldn't better? I was thinking to add a new command since I'll want to add the id, and if I'm already adding a new command I'll put in a display number too.
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
On Wed, Feb 22, 2012 at 03:22:11PM +0100, Gerd Hoffmann wrote: Hi, Honestly, for this particular case, I'm not 100% sure that having an id is _required_, as I don't expect a client to submit multiple screendump calls in parallel and we don't officially support multiple QMP clients either. Also, having the screendump filename in the event will serve as a form of identifier too. That is exactly my thinking, echo the filename written in the event. Btw, are you planning to add the event to the already existing screendump command? Adding a new command that doesn't use the monitor async API and is truly asynchronous wouldn't better? Good question. I'd tend to just let the existing command send trigger an event. But libvirt needs some way to figure whenever it should wait for an event ... Right, that's the second reason I think a new command is needed. Additionally a new command can be implemented only by qxl and not by anything else (although I guess that would be a NACK?) cheers, Gerd
[Qemu-devel] [PATCH v4 02/18] dma-helpers: add dma_buf_read and dma_buf_write
These helpers do a full transfer from an in-memory buffer to target memory, with support for scatter/gather lists. It will be used to store the reply of an emulated command into a QEMUSGList provided by the adapter. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- dma-helpers.c | 31 +++ dma.h |3 +++ 2 files changed, 34 insertions(+), 0 deletions(-) diff --git a/dma-helpers.c b/dma-helpers.c index f08cdb5..df0a421 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -204,3 +204,34 @@ BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs, { return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true); } + + +static uint64_t dma_buf_rw(uint8_t *ptr, int32_t len, QEMUSGList *sg, bool to_dev) +{ +uint64_t resid; +int sg_cur_index; + +resid = sg-size; +sg_cur_index = 0; +len = MIN(len, resid); +while (len 0) { +ScatterGatherEntry entry = sg-sg[sg_cur_index++]; +int32_t xfer = MIN(len, entry.len); +cpu_physical_memory_rw(entry.base, ptr, xfer, !to_dev); +ptr += xfer; +len -= xfer; +resid -= xfer; +} + +return resid; +} + +uint64_t dma_buf_read(uint8_t *ptr, int32_t len, QEMUSGList *sg) +{ +return dma_buf_rw(ptr, len, sg, 0); +} + +uint64_t dma_buf_write(uint8_t *ptr, int32_t len, QEMUSGList *sg) +{ +return dma_buf_rw(ptr, len, sg, 1); +} diff --git a/dma.h b/dma.h index d50019b..346ac4f 100644 --- a/dma.h +++ b/dma.h @@ -58,4 +58,7 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs, BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs, QEMUSGList *sg, uint64_t sector, BlockDriverCompletionFunc *cb, void *opaque); +uint64_t dma_buf_read(uint8_t *ptr, int32_t len, QEMUSGList *sg); +uint64_t dma_buf_write(uint8_t *ptr, int32_t len, QEMUSGList *sg); + #endif -- 1.7.7.6
Re: [Qemu-devel] Merging qemu-iotests into qemu.git?
On 02/20/2012 10:42 AM, Kevin Wolf wrote: Am 16.02.2012 21:53, schrieb Christoph Hellwig: On Thu, Feb 16, 2012 at 04:01:58PM +0100, Kevin Wolf wrote: Hi Christoph, I just talked to Stefan about our testing, both regarding the block layer and qemu in general, and we came to the conclusion that it would probably make sense to merge qemu-iotests into qemu.git. The immediate benefit would be that we could include some short-running tests into 'make check'. Long-term we would profit from being in the same tests/ directory as all future tests for other qemu subsystems, so we could probably share quite some of the framework code. (We were initially talking about image streaming tests, which need a real VM instead of just qemu-img/io and some monitor interaction - it's easy to imagine similar cases) What do you think? I'm fine with doing that. Anthony, how would we merge this best? I can rewrite the history of qemu-iotests to move everything into a tests/qemu-iotests/ directory and rewrite the subject lines of all commits to include qemu-iotests (actually I have just tried it out locally, so this part is done). We could then just pull from this temporary repository and keep all of the existing git history. I would send a pull request then without reposting all the patches in the history of qemu-iotests as some of them are pretty big. Would that work for you? Should I add a Signed-off-by to each patch for rewriting the history or would it be okay with Christoph's existing SoB? Just Christoph's SoB is fine as long as every commit has a SoB. I think rewriting the history plus a pull would be a good approach. Regards, Anthony Liguori Kevin
[Qemu-devel] [PATCH v4 05/18] scsi: pass residual amount to command_complete
With the upcoming sglist support, HBAs will not see any transfer_data call and will not have a way to detect short transfers. So pass the residual amount of data upon command completion. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/esp.c |3 ++- hw/lsi53c895a.c |2 +- hw/scsi-bus.c| 12 hw/scsi.h|3 ++- hw/spapr_vscsi.c |2 +- hw/usb-msd.c |2 +- 6 files changed, 15 insertions(+), 9 deletions(-) diff --git a/hw/esp.c b/hw/esp.c index 2dda8e3..8d73e56 100644 --- a/hw/esp.c +++ b/hw/esp.c @@ -390,7 +390,8 @@ static void esp_do_dma(ESPState *s) esp_dma_done(s); } -static void esp_command_complete(SCSIRequest *req, uint32_t status) +static void esp_command_complete(SCSIRequest *req, uint32_t status, + size_t resid) { ESPState *s = DO_UPCAST(ESPState, busdev.qdev, req-bus-qbus.parent); diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c index 0acd1d0..edc09b7 100644 --- a/hw/lsi53c895a.c +++ b/hw/lsi53c895a.c @@ -699,7 +699,7 @@ static int lsi_queue_req(LSIState *s, SCSIRequest *req, uint32_t len) } /* Callback to indicate that the SCSI layer has completed a command. */ -static void lsi_command_complete(SCSIRequest *req, uint32_t status) +static void lsi_command_complete(SCSIRequest *req, uint32_t status, size_t resid) { LSIState *s = DO_UPCAST(LSIState, dev.qdev, req-bus-qbus.parent); int out; diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index b3e97ce..eb97c87 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -533,6 +533,8 @@ SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, } req-cmd = cmd; +req-resid = req-cmd.xfer; + switch (buf[0]) { case INQUIRY: trace_scsi_inquiry(d-id, lun, tag, cmd.buf[1], cmd.buf[2]); @@ -1275,10 +1277,12 @@ void scsi_req_data(SCSIRequest *req, int len) { if (req-io_canceled) { trace_scsi_req_data_canceled(req-dev-id, req-lun, req-tag, len); -} else { -trace_scsi_req_data(req-dev-id, req-lun, req-tag, len); -req-bus-info-transfer_data(req, len); +return; } +trace_scsi_req_data(req-dev-id, req-lun, req-tag, len); +assert(req-cmd.mode != SCSI_XFER_NONE); +req-resid -= len; +req-bus-info-transfer_data(req, len); } void scsi_req_print(SCSIRequest *req) @@ -1337,7 +1341,7 @@ void scsi_req_complete(SCSIRequest *req, int status) scsi_req_ref(req); scsi_req_dequeue(req); -req-bus-info-complete(req, req-status); +req-bus-info-complete(req, req-status, req-resid); scsi_req_unref(req); } diff --git a/hw/scsi.h b/hw/scsi.h index dc72b6f..8722ef9 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -46,6 +46,7 @@ struct SCSIRequest { uint32_t tag; uint32_t lun; uint32_t status; +size_tresid; SCSICommand cmd; BlockDriverAIOCB *aiocb; uint8_t sense[SCSI_SENSE_BUF_SIZE]; @@ -112,7 +113,7 @@ struct SCSIBusInfo { int tcq; int max_channel, max_target, max_lun; void (*transfer_data)(SCSIRequest *req, uint32_t arg); -void (*complete)(SCSIRequest *req, uint32_t arg); +void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid); void (*cancel)(SCSIRequest *req); }; diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c index ffce261..2167017 100644 --- a/hw/spapr_vscsi.c +++ b/hw/spapr_vscsi.c @@ -494,7 +494,7 @@ static void vscsi_transfer_data(SCSIRequest *sreq, uint32_t len) } /* Callback to indicate that the SCSI layer has completed a transfer. */ -static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status) +static void vscsi_command_complete(SCSIRequest *sreq, uint32_t status, size_t resid) { VSCSIState *s = DO_UPCAST(VSCSIState, vdev.qdev, sreq-bus-qbus.parent); vscsi_req *req = sreq-hba_private; diff --git a/hw/usb-msd.c b/hw/usb-msd.c index c933efe..5fbd2d0 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -223,7 +223,7 @@ static void usb_msd_transfer_data(SCSIRequest *req, uint32_t len) } } -static void usb_msd_command_complete(SCSIRequest *req, uint32_t status) +static void usb_msd_command_complete(SCSIRequest *req, uint32_t status, size_t resid) { MSDState *s = DO_UPCAST(MSDState, dev.qdev, req-bus-qbus.parent); USBPacket *p = s-packet; -- 1.7.7.6
[Qemu-devel] [PATCH v4 07/18] scsi-disk: enable scatter/gather functionality
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-bus.c |1 + hw/scsi-disk.c | 63 --- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index 0c471e0..dce6208 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -87,6 +87,7 @@ static void scsi_dma_restart_bh(void *opaque) scsi_req_continue(req); break; case SCSI_XFER_NONE: +assert(!req-sg); scsi_req_dequeue(req); scsi_req_enqueue(req); break; diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index c12e3a6..2a24840 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -38,6 +38,7 @@ do { fprintf(stderr, scsi-disk: fmt , ## __VA_ARGS__); } while (0) #include sysemu.h #include blockdev.h #include block_int.h +#include dma.h #ifdef __linux #include scsi/sg.h @@ -123,6 +124,27 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r) return r-qiov.size / 512; } +static void scsi_dma_complete(void *opaque, int ret) +{ +SCSIDiskReq *r = (SCSIDiskReq *)opaque; +SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r-req.dev); + +bdrv_acct_done(s-qdev.conf.bs, r-acct); + +if (ret) { +if (scsi_handle_rw_error(r, -ret)) { +goto done; +} +} + +r-sector += r-sector_count; +r-sector_count = 0; +scsi_req_complete(r-req, GOOD); + +done: +scsi_req_unref(r-req); +} + static void scsi_read_complete(void * opaque, int ret) { SCSIDiskReq *r = (SCSIDiskReq *)opaque; @@ -213,10 +235,17 @@ static void scsi_read_data(SCSIRequest *req) return; } -n = scsi_init_iovec(r); -bdrv_acct_start(s-qdev.conf.bs, r-acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); -r-req.aiocb = bdrv_aio_readv(s-qdev.conf.bs, r-sector, r-qiov, n, - scsi_read_complete, r); +if (r-req.sg) { +dma_acct_start(s-qdev.conf.bs, r-acct, r-req.sg, BDRV_ACCT_READ); +r-req.resid -= r-req.sg-size; +r-req.aiocb = dma_bdrv_read(s-qdev.conf.bs, r-req.sg, r-sector, + scsi_dma_complete, r); +} else { +n = scsi_init_iovec(r); +bdrv_acct_start(s-qdev.conf.bs, r-acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); +r-req.aiocb = bdrv_aio_readv(s-qdev.conf.bs, r-sector, r-qiov, n, + scsi_read_complete, r); +} } /* @@ -315,18 +344,26 @@ static void scsi_write_data(SCSIRequest *req) return; } -n = r-qiov.size / 512; -if (n) { -if (s-tray_open) { -scsi_write_complete(r, -ENOMEDIUM); -return; -} +if (!r-req.sg !r-qiov.size) { +/* Called for the first time. Ask the driver to send us more data. */ +scsi_write_complete(r, 0); +return; +} +if (s-tray_open) { +scsi_write_complete(r, -ENOMEDIUM); +return; +} + +if (r-req.sg) { +dma_acct_start(s-qdev.conf.bs, r-acct, r-req.sg, BDRV_ACCT_WRITE); +r-req.resid -= r-req.sg-size; +r-req.aiocb = dma_bdrv_write(s-qdev.conf.bs, r-req.sg, r-sector, + scsi_dma_complete, r); +} else { +n = r-qiov.size / 512; bdrv_acct_start(s-qdev.conf.bs, r-acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_WRITE); r-req.aiocb = bdrv_aio_writev(s-qdev.conf.bs, r-sector, r-qiov, n, scsi_write_complete, r); -} else { -/* Called for the first time. Ask the driver to send us more data. */ -scsi_write_complete(r, 0); } } -- 1.7.7.6
[Qemu-devel] [PATCH v4 11/18] virtio-scsi: Add virtio-scsi stub device
From: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Add a useless virtio SCSI HBA device: qemu -device virtio-scsi-pci Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- Makefile.target |1 + default-configs/pci.mak |1 + default-configs/s390x-softmmu.mak |1 + hw/pci.h |1 + hw/s390-virtio-bus.c | 33 ++ hw/s390-virtio-bus.h |2 + hw/virtio-pci.c | 56 + hw/virtio-pci.h |2 + hw/virtio-scsi.c | 228 + hw/virtio-scsi.h | 36 ++ hw/virtio.h |3 + 11 files changed, 364 insertions(+), 0 deletions(-) create mode 100644 hw/virtio-scsi.c create mode 100644 hw/virtio-scsi.h diff --git a/Makefile.target b/Makefile.target index 651500e..eef4acc 100644 --- a/Makefile.target +++ b/Makefile.target @@ -200,6 +200,7 @@ obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o balloon.o ioport.o # need to fix this properly obj-$(CONFIG_NO_PCI) += pci-stub.o obj-$(CONFIG_VIRTIO) += virtio.o virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial-bus.o +obj-$(CONFIG_VIRTIO_SCSI) += virtio-scsi.o obj-y += vhost_net.o obj-$(CONFIG_VHOST_NET) += vhost.o obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/virtio-9p-device.o diff --git a/default-configs/pci.mak b/default-configs/pci.mak index 9d3e1db..21e4ccf 100644 --- a/default-configs/pci.mak +++ b/default-configs/pci.mak @@ -1,5 +1,6 @@ CONFIG_PCI=y CONFIG_VIRTIO_PCI=y +CONFIG_VIRTIO_SCSI=y CONFIG_VIRTIO=y CONFIG_USB_UHCI=y CONFIG_USB_OHCI=y diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak index 3005729..e588803 100644 --- a/default-configs/s390x-softmmu.mak +++ b/default-configs/s390x-softmmu.mak @@ -1 +1,2 @@ CONFIG_VIRTIO=y +CONFIG_VIRTIO_SCSI=y diff --git a/hw/pci.h b/hw/pci.h index 33b0b18..ff4c12d 100644 --- a/hw/pci.h +++ b/hw/pci.h @@ -75,6 +75,7 @@ #define PCI_DEVICE_ID_VIRTIO_BLOCK 0x1001 #define PCI_DEVICE_ID_VIRTIO_BALLOON 0x1002 #define PCI_DEVICE_ID_VIRTIO_CONSOLE 0x1003 +#define PCI_DEVICE_ID_VIRTIO_SCSI0x1004 #define FMT_PCIBUS PRIx64 diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c index 9d48056..c450e4b 100644 --- a/hw/s390-virtio-bus.c +++ b/hw/s390-virtio-bus.c @@ -169,6 +169,18 @@ static int s390_virtio_serial_init(VirtIOS390Device *dev) return r; } +static int s390_virtio_scsi_init(VirtIOS390Device *dev) +{ +VirtIODevice *vdev; + +vdev = virtio_scsi_init((DeviceState *)dev, dev-scsi); +if (!vdev) { +return -1; +} + +return s390_virtio_device_init(dev, vdev); +} + static uint64_t s390_virtio_device_vq_token(VirtIOS390Device *dev, int vq) { ram_addr_t token_off; @@ -433,6 +445,26 @@ static TypeInfo virtio_s390_device_info = { .abstract = true, }; +static Property s390_virtio_scsi_properties[] = { +DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOS390Device, host_features, scsi), +DEFINE_PROP_END_OF_LIST(), +}; + +static void s390_virtio_scsi_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +VirtIOS390DeviceClass *k = VIRTIO_S390_DEVICE_CLASS(klass); + +k-init = s390_virtio_scsi_init; +dc-props = s390_virtio_scsi_properties; +} + +static TypeInfo s390_virtio_scsi = { +.name = virtio-scsi-s390, +.parent= TYPE_VIRTIO_S390_DEVICE, +.instance_size = sizeof(VirtIOS390Device), +.class_init= s390_virtio_scsi_class_init, +}; /* S390 Virtio Bus Bridge Device ***/ /* Only required to have the virtio bus as child in the system bus */ @@ -465,6 +497,7 @@ static void s390_virtio_register_types(void) type_register_static(s390_virtio_serial); type_register_static(s390_virtio_blk); type_register_static(s390_virtio_net); +type_register_static(s390_virtio_scsi); type_register_static(s390_virtio_bridge_info); } diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h index b5e59b7..0e60bc0 100644 --- a/hw/s390-virtio-bus.h +++ b/hw/s390-virtio-bus.h @@ -19,6 +19,7 @@ #include virtio-net.h #include virtio-serial.h +#include virtio-scsi.h #define VIRTIO_DEV_OFFS_TYPE 0 /* 8 bits */ #define VIRTIO_DEV_OFFS_NUM_VQ 1 /* 8 bits */ @@ -67,6 +68,7 @@ struct VirtIOS390Device { uint32_t host_features; virtio_serial_conf serial; virtio_net_conf net; +VirtIOSCSIConf scsi; }; typedef struct VirtIOS390Bus { diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 907b52a..a0fb7c1 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -21,6 +21,7 @@ #include virtio-blk.h #include virtio-net.h #include virtio-serial.h +#include virtio-scsi.h #include pci.h #include
[Qemu-devel] [PATCH v4 15/18] virtio-scsi: add migration support
Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/virtio-scsi.c | 50 +- 1 files changed, 49 insertions(+), 1 deletions(-) diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c index 380073a..7f850d1 100644 --- a/hw/virtio-scsi.c +++ b/hw/virtio-scsi.c @@ -237,6 +237,34 @@ static VirtIOSCSIReq *virtio_scsi_pop_req(VirtIOSCSI *s, VirtQueue *vq) return req; } +static void virtio_scsi_save_request(QEMUFile *f, SCSIRequest *sreq) +{ +VirtIOSCSIReq *req = sreq-hba_private; + +qemu_put_buffer(f, (unsigned char *)req-elem, sizeof(req-elem)); +} + +static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq) +{ +SCSIBus *bus = sreq-bus; +VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); +VirtIOSCSIReq *req; + +req = g_malloc(sizeof(*req)); +qemu_get_buffer(f, (unsigned char *)req-elem, sizeof(req-elem)); +virtio_scsi_parse_req(s, s-cmd_vq, req); + +scsi_req_ref(sreq); +req-sreq = sreq; +if (req-sreq-cmd.mode != SCSI_XFER_NONE) { +int req_mode = +(req-elem.in_num 1 ? SCSI_XFER_FROM_DEV : SCSI_XFER_TO_DEV); + +assert(req-sreq-cmd.mode == req_mode); +} +return req; +} + static void virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) { SCSIDevice *d = virtio_scsi_device_find(s, req-req.cmd-lun); @@ -518,6 +546,22 @@ static void virtio_scsi_reset(VirtIODevice *vdev) s-cdb_size = VIRTIO_SCSI_CDB_SIZE; } +/* The device does not have anything to save beyond the virtio data. + * Request data is saved with callbacks from SCSI devices. + */ +static void virtio_scsi_save(QEMUFile *f, void *opaque) +{ +VirtIOSCSI *s = opaque; +virtio_save(s-vdev, f); +} + +static int virtio_scsi_load(QEMUFile *f, void *opaque, int version_id) +{ +VirtIOSCSI *s = opaque; +virtio_load(s-vdev, f); +return 0; +} + static struct SCSIBusInfo virtio_scsi_scsi_info = { .tcq = true, .max_channel = VIRTIO_SCSI_MAX_CHANNEL, @@ -527,11 +571,14 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = { .complete = virtio_scsi_command_complete, .cancel = virtio_scsi_request_cancelled, .get_sg_list = virtio_scsi_get_sg_list, +.save_request = virtio_scsi_save_request, +.load_request = virtio_scsi_load_request, }; VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) { VirtIOSCSI *s; +static int virtio_scsi_id; s = (VirtIOSCSI *)virtio_common_init(virtio-scsi, VIRTIO_ID_SCSI, sizeof(VirtIOSCSIConfig), @@ -558,7 +605,8 @@ VirtIODevice *virtio_scsi_init(DeviceState *dev, VirtIOSCSIConf *proxyconf) scsi_bus_legacy_handle_cmdline(s-bus); } -/* TODO savevm */ +register_savevm(dev, virtio-scsi, virtio_scsi_id++, 1, +virtio_scsi_save, virtio_scsi_load, s); return s-vdev; } -- 1.7.7.6
Re: [Qemu-devel] [Spice-devel] [RFC v4 4/9] qxl: screen_dump in vga: do a single ppm_save
On 02/22/12 15:25, Alon Levy wrote: On Wed, Feb 22, 2012 at 02:58:10PM +0100, Gerd Hoffmann wrote: And the only user of screen_dump_filename is: static void vga_save_dpy_update(DisplayState *ds, int x, int y, int w, int h) { if (screen_dump_filename) { ppm_save(screen_dump_filename, ds-surface); upstream/master this here: screen_dump_filename = NULL; That's wrong, you'll get the screenshot after the first update, who's to say it is fully rendered? vga code actually does a single, fullscreen update after vga_invalidate_display(), so it should work fine. The corner case where this fails is when console switching is needed, i.e. switch to monitor console via ctrl-alt-2, then type the screenshot command there ... Are you talking about sdl console or linux guest console? ctrl-alt-2 or ctrl-alt-F2? I can try both. ctrl-alt-2 sdl console. cheers, Gerd
Re: [Qemu-devel] [PULL v2 0/9] qdev deconstruction, command-line episode
On 02/21/2012 11:13 AM, Paolo Bonzini wrote: Anthony, I'm sending a pull request since you informally said on IRC that you're okay with the patches. The following changes since commit 99c7f87826337fa81f2f0f9baa9ca0a44faf90e9: input: send kbd+mouse events only to running guests. (2012-02-17 11:02:55 -0600) Pulled. Thanks. Regards, Anthony Liguori are available in the git repository at: git://github.com/bonzini/qemu.git qdev-props-for-anthony v1-v2: Fix bug in parsing booleans and strengthen test suite. The interdiff is attached. Paolo Bonzini (9): qapi: allow sharing enum implementation across visitors qapi: drop qmp_input_end_optional qapi: add string-based visitors qapi: add tests for string-based visitors qom: add generic string parsing/printing qdev: accept both strings and integers for PCI addresses qdev: accept hex properties only if prefixed by 0x qdev: use built-in QOM string parser qdev: drop unnecessary parse/print methods .gitignore |2 + Makefile.objs|5 +- hw/qdev-properties.c | 186 +--- include/qemu/object.h| 24 + qapi/qapi-visit-core.c | 51 +++ qapi/qapi-visit-impl.h | 23 + qapi/qmp-input-visitor.c | 39 + qapi/qmp-output-visitor.c| 22 +- qapi/string-input-visitor.c | 138 + qapi/string-input-visitor.h | 25 ++ qapi/string-output-visitor.c | 89 +++ qapi/string-output-visitor.h | 26 ++ qom/object.c | 24 + test-string-input-visitor.c | 195 ++ test-string-output-visitor.c | 188 tests/Makefile | 12 ++- 16 files changed, 841 insertions(+), 208 deletions(-) create mode 100644 qapi/qapi-visit-impl.h create mode 100644 qapi/string-input-visitor.c create mode 100644 qapi/string-input-visitor.h create mode 100644 qapi/string-output-visitor.c create mode 100644 qapi/string-output-visitor.h create mode 100644 test-string-input-visitor.c create mode 100644 test-string-output-visitor.c diff --git a/include/qemu/object.h b/include/qemu/object.h index 2081ca0..ff6be14 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -742,7 +742,7 @@ void object_property_parse(Object *obj, const char *string, const char *name, struct Error **errp); /** - * object_property_set: + * object_property_print: * @obj: the object * @name: the name of the property * @errp: returns an error if this function fails diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c index ceee699..497eb9a 100644 --- a/qapi/string-input-visitor.c +++ b/qapi/string-input-visitor.c @@ -47,13 +47,15 @@ static void parse_type_bool(Visitor *v, bool *obj, const char *name, StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v); if (siv-string) { -if (strcasecmp(siv-string, on) || strcasecmp(siv-string, yes) || -strcasecmp(siv-string, true)) { +if (!strcasecmp(siv-string, on) || +!strcasecmp(siv-string, yes) || +!strcasecmp(siv-string, true)) { *obj = true; return; } -if (strcasecmp(siv-string, off) || strcasecmp(siv-string, no) || -strcasecmp(siv-string, false)) { +if (!strcasecmp(siv-string, off) || +!strcasecmp(siv-string, no) || +!strcasecmp(siv-string, false)) { *obj = false; return; } diff --git a/test-string-input-visitor.c b/test-string-input-visitor.c index ba2cc40..5370e32 100644 --- a/test-string-input-visitor.c +++ b/test-string-input-visitor.c @@ -75,6 +75,41 @@ static void test_visitor_in_bool(TestInputVisitorData *data, visit_type_bool(v,res, NULL,errp); g_assert(!error_is_set(errp)); g_assert_cmpint(res, ==, true); +visitor_input_teardown(data, unused); + +v = visitor_input_test_init(data, yes); + +visit_type_bool(v,res, NULL,errp); +g_assert(!error_is_set(errp)); +g_assert_cmpint(res, ==, true); +visitor_input_teardown(data, unused); + +v = visitor_input_test_init(data, on); + +visit_type_bool(v,res, NULL,errp); +g_assert(!error_is_set(errp)); +g_assert_cmpint(res, ==, true); +visitor_input_teardown(data, unused); + +v = visitor_input_test_init(data, false); + +visit_type_bool(v,res, NULL,errp); +g_assert(!error_is_set(errp)); +g_assert_cmpint(res, ==, false); +visitor_input_teardown(data, unused); + +v = visitor_input_test_init(data, no); + +visit_type_bool(v,res, NULL,errp); +g_assert(!error_is_set(errp)); +g_assert_cmpint(res, ==, false); +visitor_input_teardown(data, unused); + +v =
Re: [Qemu-devel] [PULL] spice patch queue
On 02/21/2012 04:59 AM, Gerd Hoffmann wrote: Hi, Here is the spice patch queue with a collection of little improvements and bugfixes. No major stuff. See individual patches for details. Pulled. Thanks. Regards, Anthony Liguori please pull, Gerd The following changes since commit 99c7f87826337fa81f2f0f9baa9ca0a44faf90e9: input: send kbd+mouse events only to running guests. (2012-02-17 11:02:55 -0600) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu spice.v48 Daniel P. Berrange (1): Add SPICE support to add_client monitor command Gerd Hoffmann (5): qxl: fix warnings on 32bit qxl: don't render stuff when the vm is stopped. qxl: drop vram bar minimum size qxl: move ram size init to new function qxl: add user-friendly bar size properties Yonit Halperin (3): qxl: set only off-screen surfaces dirty instead of the whole vram qxl: make sure primary surface is saved on migration also in compat mode spice: support ipv6 channel address in monitor events and in spice info hw/qxl-render.c | 12 +++ hw/qxl.c| 109 +++ hw/qxl.h|4 ++ monitor.c |9 - qmp-commands.hx |6 ++- ui/qemu-spice.h |7 ui/spice-core.c | 50 +++--- 7 files changed, 150 insertions(+), 47 deletions(-)
Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback
Hi, I was thinking to add a new command since I'll want to add the id, and if I'm already adding a new command I'll put in a display number too. Big question is what the display number is supposed to be ... cheers, Gerd
[Qemu-devel] [PATCH v4 10/18] scsi-disk: add migration support
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-disk.c | 59 --- 1 files changed, 55 insertions(+), 4 deletions(-) diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 2a24840..ec8e7cb 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -111,12 +111,12 @@ static void scsi_cancel_io(SCSIRequest *req) r-req.aiocb = NULL; } -static uint32_t scsi_init_iovec(SCSIDiskReq *r) +static uint32_t scsi_init_iovec(SCSIDiskReq *r, size_t size) { SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r-req.dev); if (!r-iov.iov_base) { -r-buflen = SCSI_DMA_BUF_SIZE; +r-buflen = size; r-iov.iov_base = qemu_blockalign(s-qdev.conf.bs, r-buflen); } r-iov.iov_len = MIN(r-sector_count * 512, r-buflen); @@ -124,6 +124,35 @@ static uint32_t scsi_init_iovec(SCSIDiskReq *r) return r-qiov.size / 512; } +static void scsi_disk_save_request(QEMUFile *f, SCSIRequest *req) +{ +SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); + +qemu_put_be64s(f, r-sector); +qemu_put_be32s(f, r-sector_count); +qemu_put_be32s(f, r-buflen); +if (r-buflen r-req.cmd.mode == SCSI_XFER_TO_DEV) { +qemu_put_buffer(f, r-iov.iov_base, r-iov.iov_len); +} +} + +static void scsi_disk_load_request(QEMUFile *f, SCSIRequest *req) +{ +SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req); + +qemu_get_be64s(f, r-sector); +qemu_get_be32s(f, r-sector_count); +qemu_get_be32s(f, r-buflen); +if (r-buflen) { +scsi_init_iovec(r, r-buflen); +if (r-req.cmd.mode == SCSI_XFER_TO_DEV) { +qemu_get_buffer(f, r-iov.iov_base, r-iov.iov_len); +} +} + +qemu_iovec_init_external(r-qiov, r-iov, 1); +} + static void scsi_dma_complete(void *opaque, int ret) { SCSIDiskReq *r = (SCSIDiskReq *)opaque; @@ -241,7 +270,7 @@ static void scsi_read_data(SCSIRequest *req) r-req.aiocb = dma_bdrv_read(s-qdev.conf.bs, r-req.sg, r-sector, scsi_dma_complete, r); } else { -n = scsi_init_iovec(r); +n = scsi_init_iovec(r, SCSI_DMA_BUF_SIZE); bdrv_acct_start(s-qdev.conf.bs, r-acct, n * BDRV_SECTOR_SIZE, BDRV_ACCT_READ); r-req.aiocb = bdrv_aio_readv(s-qdev.conf.bs, r-sector, r-qiov, n, scsi_read_complete, r); @@ -316,7 +345,7 @@ static void scsi_write_complete(void * opaque, int ret) if (r-sector_count == 0) { scsi_req_complete(r-req, GOOD); } else { -scsi_init_iovec(r); +scsi_init_iovec(r, SCSI_DMA_BUF_SIZE); DPRINTF(Write complete tag=0x%x more=%d\n, r-req.tag, r-qiov.size); scsi_req_data(r-req, r-qiov.size); } @@ -1621,6 +1650,8 @@ static const SCSIReqOps scsi_disk_reqops = { .write_data = scsi_write_data, .cancel_io= scsi_cancel_io, .get_buf = scsi_get_buf, +.load_request = scsi_disk_load_request, +.save_request = scsi_disk_save_request, }; static SCSIRequest *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun, @@ -1755,6 +1786,22 @@ static Property scsi_hd_properties[] = { DEFINE_PROP_END_OF_LIST(), }; +static const VMStateDescription vmstate_scsi_disk_state = { +.name = scsi-disk, +.version_id = 1, +.minimum_version_id = 1, +.minimum_version_id_old = 1, +.fields = (VMStateField[]) { +VMSTATE_SCSI_DEVICE(qdev, SCSIDiskState), +VMSTATE_BOOL(media_changed, SCSIDiskState), +VMSTATE_BOOL(media_event, SCSIDiskState), +VMSTATE_BOOL(eject_request, SCSIDiskState), +VMSTATE_BOOL(tray_open, SCSIDiskState), +VMSTATE_BOOL(tray_locked, SCSIDiskState), +VMSTATE_END_OF_LIST() +} +}; + static void scsi_hd_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -1768,6 +1815,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void *data) dc-desc = virtual SCSI disk; dc-reset = scsi_disk_reset; dc-props = scsi_hd_properties; +dc-vmsd = vmstate_scsi_disk_state; } static TypeInfo scsi_hd_info = { @@ -1795,6 +1843,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void *data) dc-desc = virtual SCSI CD-ROM; dc-reset = scsi_disk_reset; dc-props = scsi_cd_properties; +dc-vmsd = vmstate_scsi_disk_state; } static TypeInfo scsi_cd_info = { @@ -1822,6 +1871,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data) dc-desc = SCSI block device passthrough; dc-reset = scsi_disk_reset; dc-props = scsi_block_properties; +dc-vmsd = vmstate_scsi_disk_state; } static TypeInfo scsi_block_info = { @@ -1851,6 +1901,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass, void *data) dc-desc = virtual SCSI disk or CD-ROM (legacy); dc-reset = scsi_disk_reset; dc-props = scsi_disk_properties; +dc-vmsd = vmstate_scsi_disk_state; } static TypeInfo
Re: [Qemu-devel] [PATCH 1/3] qapi: Allow QMP/QAPI commands to have array inputs
On 02/20/2012 11:31 AM, Jeff Cody wrote: The QAPI scripts allow for generating commands that receive parameter input consisting of a list of custom structs, but the QMP input paramter checking did not support receiving a qlist as opposed to a qdict for input. What are you trying to send on the wire? Something like {execute: foo, arguments: [ a, b, c ]} ? That's not valid per the QMP protocol. arguments must always be a QMP dictionary. Regards, Anthony Liguori This patch allows for array input parameters, although no argument validation is performed for commands that accept a list input. Signed-off-by: Jeff Codyjc...@redhat.com --- monitor.c | 72 ++-- monitor.h |1 + 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/monitor.c b/monitor.c index a7df995..98e6017 100644 --- a/monitor.c +++ b/monitor.c @@ -125,8 +125,12 @@ typedef struct mon_cmd_t { void (*info)(Monitor *mon); void (*cmd)(Monitor *mon, const QDict *qdict); int (*cmd_new)(Monitor *mon, const QDict *params, QObject **ret_data); +int (*cmd_new_list)(Monitor *mon, const QList *params, + QObject **ret_data); int (*cmd_async)(Monitor *mon, const QDict *params, MonitorCompletion *cb, void *opaque); +int (*cmd_async_list)(Monitor *mon, const QList *params, + MonitorCompletion *cb, void *opaque); } mhandler; bool qapi; int flags; @@ -353,6 +357,11 @@ static inline bool handler_is_async(const mon_cmd_t *cmd) return cmd-flags MONITOR_CMD_ASYNC; } +static inline bool handler_accepts_array(const mon_cmd_t *cmd) +{ +return cmd-flags MONITOR_CMD_ARRAY_INPUT; +} + static inline int monitor_has_error(const Monitor *mon) { return mon-error != NULL; @@ -671,6 +680,12 @@ static int qmp_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd, return cmd-mhandler.cmd_async(mon, params, qmp_monitor_complete, mon); } +static int qmp_async_cmd_handler_array(Monitor *mon, const mon_cmd_t *cmd, + const QList *params) +{ +return cmd-mhandler.cmd_async_list(mon, params, qmp_monitor_complete, mon); +} + static void user_async_cmd_handler(Monitor *mon, const mon_cmd_t *cmd, const QDict *params) { @@ -4310,7 +4325,8 @@ static QDict *qmp_check_input_obj(QObject *input_obj) } has_exec_key = 1; } else if (!strcmp(arg_name, arguments)) { -if (qobject_type(arg_obj) != QTYPE_QDICT) { +if ((qobject_type(arg_obj) != QTYPE_QDICT) +(qobject_type(arg_obj) != QTYPE_QLIST)) { qerror_report(QERR_QMP_BAD_INPUT_OBJECT_MEMBER, arguments, object); return NULL; @@ -4345,11 +4361,26 @@ static void qmp_call_cmd(Monitor *mon, const mon_cmd_t *cmd, qobject_decref(data); } +static void qmp_call_cmd_array(Monitor *mon, const mon_cmd_t *cmd, + const QList *params) +{ +int ret; +QObject *data = NULL; + +mon_print_count_init(mon); + +ret = cmd-mhandler.cmd_new_list(mon, params,data); +handler_audit(mon, cmd, ret); +monitor_protocol_emitter(mon, data); +qobject_decref(data); +} + static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) { int err; QObject *obj; QDict *input, *args; +QList *args_list = NULL; const mon_cmd_t *cmd; const char *cmd_name; Monitor *mon = cur_mon; @@ -4386,26 +4417,42 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) } obj = qdict_get(input, arguments); -if (!obj) { -args = qdict_new(); +if (handler_accepts_array(cmd)) { +if (!obj || (qobject_type(obj) != QTYPE_QLIST)) { +args_list = qlist_new(); +} else { +args_list = qobject_to_qlist(obj); +QINCREF(args_list); +} } else { -args = qobject_to_qdict(obj); -QINCREF(args); -} - -err = qmp_check_client_args(cmd, args); -if (err 0) { -goto err_out; +if (!obj || (qobject_type(obj) != QTYPE_QDICT)) { +args = qdict_new(); +} else { +args = qobject_to_qdict(obj); +QINCREF(args); +} +err = qmp_check_client_args(cmd, args); +if (err 0) { +goto err_out; +} } if (handler_is_async(cmd)) { -err = qmp_async_cmd_handler(mon, cmd, args); +if (handler_accepts_array(cmd)) { +err = qmp_async_cmd_handler_array(mon, cmd, args_list); +} else { +err = qmp_async_cmd_handler(mon, cmd, args); +} if (err) { /* emit the error response */ goto err_out; } }
Re: [Qemu-devel] [Xen-devel] [PATCH V7 00/11] Xen PCI Passthrough
On Mon, 20 Feb 2012, Anthony PERARD wrote: On Mon, Feb 20, 2012 at 19:58, Tobias Geiger tobias.gei...@vido.info wrote: [00:06.0] pt_pci_read_config: address=0x0030 val=0x len=4 [00:06.0] pt_pci_write_config: address=0x0030 val=0x len=4 [00:06.0] pt_iomem_map: BAR 6, e_phys=0x maddr=0xc314 len=0x2 first_map=1 [00:06.0] pt_pci_read_config: address=0x0030 val=0xfffe0001 len=4 [00:06.0] pt_pci_write_config: address=0x0030 val=0x len=4 [00:06.0] pt_iomem_map: BAR 6, e_phys=0x maddr=0xc314 len=0x2 first_map=0 ... [00:06.0] pt_pci_read_config: address=0x0004 val=0x len=2 [00:06.0] pt_pci_write_config: address=0x0004 val=0x0004 len=2 qemu-system-x86_64: /usr/src/xen-with-upstream-qemu-patch/upstream-qemu/qemu/memory.c:1378: memory_region_del_subregion: Assertion `subregion-parent == mr' failed. This last line is why QEMU fail and I thinks it's because I never try a PCI device with a ROM pci bar. I'll try to fix this tomorrow. Could you tell me if this patch resolve the issue? diff --git a/hw/xen_pci_passthrough_config_init.c b/hw/xen_pci_passthrough_config_init.c index f1fffd1..37aa383 100644 --- a/hw/xen_pci_passthrough_config_init.c +++ b/hw/xen_pci_passthrough_config_init.c @@ -555,18 +555,16 @@ static int pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s, XenPTReg *reg_entry = NULL; XenPTRegion *base = NULL; PCIDevice *d = (PCIDevice *)s-dev; -PCIIORegion *r; uint32_t writable_mask = 0; uint32_t throughable_mask = 0; pcibus_t r_size = 0; uint32_t bar_emu_mask = 0; uint32_t bar_ro_mask = 0; -r = d-io_regions[PCI_ROM_SLOT]; -r_size = r-size; +r_size = d-io_regions[PCI_ROM_SLOT].size; base = s-bases[PCI_ROM_SLOT]; /* align memory type resource size */ -pt_get_emul_size(base-bar_flag, r_size); +r_size = pt_get_emul_size(base-bar_flag, r_size); /* set emulate mask and read-only mask */ bar_emu_mask = reg-emu_mask; @@ -576,19 +574,6 @@ static int pt_exp_rom_bar_reg_write(XenPCIPassthroughState *s, writable_mask = ~bar_ro_mask valid_mask; cfg_entry-data = PT_MERGE_VALUE(*value, cfg_entry-data, writable_mask); -/* update the corresponding virtual region address */ -/* - * When guest code tries to get block size of mmio, it will write all 1s - * into pci bar register. In this case, cfg_entry-data == writable_mask. - * Especially for devices with large mmio, the value of writable_mask - * is likely to be a guest physical address that has been mapped to ram - * rather than mmio. Remapping this value to mmio should be prevented. - */ - -if (cfg_entry-data != writable_mask) { -r-addr = cfg_entry-data; -} - /* create value for writing to I/O device register */ throughable_mask = ~bar_emu_mask valid_mask; *value = PT_MERGE_VALUE(*value, dev_value, throughable_mask); Thanks, -- Anthony PERARD
[Qemu-devel] [PATCH v4 06/18] scsi: add scatter/gather functionality
Scatter/gather functionality uses the newly added DMA helpers. The device can choose between doing DMA itself, or calling scsi_req_data as usual, which will use the newly added DMA helpers to copy piecewise to/from the destination area(s). Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- hw/scsi-bus.c | 28 ++-- hw/scsi.h |3 +++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index eb97c87..0c471e0 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -5,6 +5,7 @@ #include qdev.h #include blockdev.h #include trace.h +#include dma.h static char *scsibus_get_fw_dev_path(DeviceState *dev); static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf); @@ -651,6 +652,11 @@ int32_t scsi_req_enqueue(SCSIRequest *req) assert(!req-enqueued); scsi_req_ref(req); +if (req-bus-info-get_sg_list) { +req-sg = req-bus-info-get_sg_list(req); +} else { +req-sg = NULL; +} req-enqueued = true; QTAILQ_INSERT_TAIL(req-dev-requests, req, next); @@ -1275,14 +1281,32 @@ void scsi_req_continue(SCSIRequest *req) Once it completes, calling scsi_req_continue will restart I/O. */ void scsi_req_data(SCSIRequest *req, int len) { +uint8_t *buf; if (req-io_canceled) { trace_scsi_req_data_canceled(req-dev-id, req-lun, req-tag, len); return; } trace_scsi_req_data(req-dev-id, req-lun, req-tag, len); assert(req-cmd.mode != SCSI_XFER_NONE); -req-resid -= len; -req-bus-info-transfer_data(req, len); +if (!req-sg) { +req-resid -= len; +req-bus-info-transfer_data(req, len); +return; +} + +/* If the device calls scsi_req_data and the HBA specified a + * scatter/gather list, the transfer has to happen in a single + * step. */ +assert(!req-dma_started); +req-dma_started = true; + +buf = scsi_req_get_buf(req); +if (req-cmd.mode == SCSI_XFER_FROM_DEV) { +req-resid = dma_buf_read(buf, len, req-sg); +} else { +req-resid = dma_buf_write(buf, len, req-sg); +} +scsi_req_continue(req); } void scsi_req_print(SCSIRequest *req) diff --git a/hw/scsi.h b/hw/scsi.h index 8722ef9..34cdba8 100644 --- a/hw/scsi.h +++ b/hw/scsi.h @@ -49,6 +49,8 @@ struct SCSIRequest { size_tresid; SCSICommand cmd; BlockDriverAIOCB *aiocb; +QEMUSGList*sg; +bool dma_started; uint8_t sense[SCSI_SENSE_BUF_SIZE]; uint32_t sense_len; bool enqueued; @@ -115,6 +117,7 @@ struct SCSIBusInfo { void (*transfer_data)(SCSIRequest *req, uint32_t arg); void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid); void (*cancel)(SCSIRequest *req); +QEMUSGList *(*get_sg_list)(SCSIRequest *req); }; struct SCSIBus { -- 1.7.7.6
Re: [Qemu-devel] Help me about the FDC
3: or give me some introduce of FDC. http://en.wikipedia.org/wiki/Floppy_disk_controller HTH, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] Help me about the FDC
2: explain the struct of FDCtrl; In order to know what those fields in FDCtrl mean, you might need to read ftp://download.intel.com/design/archives/periphrl/docs/29047504.pdf first. As the comment in hw/fdc.c says, it's Intel 82078 floppy disk controller emulation. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
[Qemu-devel] [PATCH] split SCSI and LSI, add myself as SCSI maintainer
This has been the de facto situation for a while now. Add a tree, too. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- MAINTAINERS |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 647c413..0b3b3d8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -420,11 +420,16 @@ F: hw/pci* F: hw/piix* SCSI +M: Paolo Bonzini pbonz...@redhat.com +S: Supported +F: hw/virtio-scsi.* +F: hw/scsi* +T: git://github.com/bonzini/qemu.git scsi-next + +LSI53C895A M: Paul Brook p...@codesourcery.com -M: Kevin Wolf kw...@redhat.com S: Odd Fixes F: hw/lsi53c895a.c -F: hw/scsi* USB M: Gerd Hoffmann kra...@redhat.com -- 1.7.7.6
Re: [Qemu-devel] [PATCH 6/6] qmp: add balloon-get-memory-stats event
On Wed, Feb 22, 2012 at 10:48:13AM -0200, Luiz Capitulino wrote: On Fri, 17 Feb 2012 15:51:33 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: On Fri, Feb 17, 2012 at 03:16:22PM -0200, Luiz Capitulino wrote: On Fri, 17 Feb 2012 10:55:41 -0600 Anthony Liguori aligu...@us.ibm.com wrote: On 02/08/2012 02:30 PM, Luiz Capitulino wrote: This commit adds a QMP API for the guest provided memory statistics (long disabled by commit 07b0403dfc2b2ac179ae5b48105096cc2d03375a). The approach taken by the original commit (625a5befc2e3200b396594f002218d235e375da5) was to extend the query-balloon command. It introduced a severe bug though: query-balloon would hang if the guest didn't respond. The approach taken by this commit is asynchronous and thus avoids any QMP hangs. First, a client has to issue the balloon-get-memory-stats command. That command gets the process started by only sending a request to the guest, it doesn't block. When the memory stats are made available by the guest, they are returned to the client as an QMP event. Signed-off-by: Luiz Capitulinolcapitul...@redhat.com Do we need this to be stable in 1.1? Well, this is disabled for a long time already and libvirt needs it, so I'd say asap, but isn't it possible to implement this with current QOM? We can do this pretty nicely through QOM. We can have a polling property in the virtio-balloon driver, that when set, will enable the virtio-balloon device to poll the guest for statistics. We can also have properties for each of the memory statistics and a timestamp for when the last update was. I think this is a friendlier approach for clients, and a cleaner approach from a QEMU perspective. I agree it's friendlier, but is it a good idea to keep polling the guest for something that may never be needed by a mngt app (real question)? Probably not, but then again you'd only need like 1-second granularity. I've talked with Anthony by irc about the implementation details of this and it will be possible to enable/disable the polling, so this is not an issue anymore. Also, I think we can do away with the polling once async QMP is in place, so we wouldn't be stuck with it necessarilly. This is what this series does :) I don't think it's necessary to wait for async support, we're accepting ad-hoc async mechanisms for other commands and could use one here too if needed. I don't mind the ad-hoc implementation details, I just think in this case it's leaking out into our APIs. With proper async QMP in place we just re-enable query-balloon and existing synchronous QMP clients and libvirt interfaces Just Work (albeit with potential for a timed-out response). There's no need for a specialized balloon-get-memory-stats command at all. And if they want stats asynchronously, proper async QMP does that as well: they just need to make their QMP client async-aware (basically, don't wait around for a response, and tag the query-balloon request so you can match the response to the query). So balloon-get-memory-stats is already on the path to being deprecated. We should instead focus on just re-enabling query-balloon, and if that can be achieved with this approach, then I'm all for it, but I think we all seem to agree that a timer-based mechanism would be needed instead. We could allow the mngt app to do the polling by adding a query-balloon-stats command (instead of balloon-get-memory-stats event). This command could return the latest available stats if any (with a timestamp) and query the guest for new stats. The downside there is you could read some really stale data that way, to the point where any app that really cared would likely throw out the first result. Having stale data will be possible with any timer based polling... Sorry, thought you were suggesting we do lazy polling where we only queue up a balloon request after a query-balloon has been issued, in which case the age of the response is unbounded... well, from a QMP standpoint, I guess that *is* what we'd be doing, and we'd just be telling management tools to add a wrapper around the interface as a work-around, which seems suggests it's not the right interface.