[Qemu-devel] [PATCH v11 8/9] hw/pc_piix: add pc-1.1

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Mitsyanko Igor

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

2012-02-22 Thread sean darcy
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

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Peter Maydell
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)

2012-02-22 Thread Craig Ringer
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

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Jordan Justen
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

2012-02-22 Thread Evgeny Voevodin

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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Stefan Hajnoczi
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!

2012-02-22 Thread Zhi Hui Li





Re: [Qemu-devel] [PATCH v2] Fix dependency issue introduced by commit 7b93fadf3a38d1ed65ea5536a52efc2772c6e3b8

2012-02-22 Thread Andreas Färber
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Andreas Färber
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()

2012-02-22 Thread Andreas Färber
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Andreas Färber
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

2012-02-22 Thread Hannes Reinecke
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

2012-02-22 Thread Igor Mitsyanko
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

2012-02-22 Thread Igor Mitsyanko
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

2012-02-22 Thread Igor Mitsyanko
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

2012-02-22 Thread Igor Mitsyanko
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

2012-02-22 Thread Igor Mitsyanko
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*

2012-02-22 Thread Igor Mitsyanko
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

2012-02-22 Thread Craig Ringer
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Gerd Hoffmann
  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

2012-02-22 Thread Paolo Bonzini
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*

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread andrzej zaborowski
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

2012-02-22 Thread Gerd Hoffmann
  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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Mitsyanko Igor

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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Gerd Hoffmann
  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

2012-02-22 Thread andrzej zaborowski
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Mitsyanko Igor

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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread andrzej zaborowski
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

2012-02-22 Thread Andreas Färber
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread andrzej zaborowski
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

2012-02-22 Thread 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.



Re: [Qemu-devel] [PATCH 6/6] qmp: add balloon-get-memory-stats event

2012-02-22 Thread Luiz Capitulino
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

2012-02-22 Thread Peter Zijlstra
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

2012-02-22 Thread Luiz Capitulino
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

2012-02-22 Thread Kevin Wolf
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

2012-02-22 Thread Kevin Wolf
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

2012-02-22 Thread Luiz Capitulino
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Anthony Liguori

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

2012-02-22 Thread Mitsyanko Igor

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

2012-02-22 Thread Mitsyanko Igor



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

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Luiz Capitulino
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*

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Peter Maydell
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

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Gerd Hoffmann
 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

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Juan Quintela
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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Gerd Hoffmann
  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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Alon Levy
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

2012-02-22 Thread Paolo Bonzini
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?

2012-02-22 Thread Anthony Liguori

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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Gerd Hoffmann
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

2012-02-22 Thread Anthony Liguori

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

2012-02-22 Thread Anthony Liguori

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

2012-02-22 Thread Gerd Hoffmann
  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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Anthony Liguori

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

2012-02-22 Thread Anthony PERARD
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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread 陳韋任
 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

2012-02-22 Thread 陳韋任
 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

2012-02-22 Thread Paolo Bonzini
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

2012-02-22 Thread Michael Roth
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.



  1   2   3   >