[Qemu-devel] [...@redhat.com: [PATCHv2 0/3] qemu: memory notifiers]

2010-01-14 Thread Michael S. Tsirkin
Avi, Marcelo, if there are no objections, maybe this series can be
merged through the new shiny qemu-kvm upstream branch? Thanks!

- Forwarded message from Michael S. Tsirkin m...@redhat.com -

Date: Mon, 4 Jan 2010 21:48:56 +0200
From: Michael S. Tsirkin m...@redhat.com
To: Anthony Liguori anth...@codemonkey.ws, qemu-devel@nongnu.org,
a...@redhat.com, g...@redhat.com
Subject: [PATCHv2 0/3] qemu: memory notifiers
User-Agent: Mutt/1.5.19 (2009-01-05)

This patch against qemu upstream adds notifiers hook which lets backends
get notified on memory changes, and converts kvm to use it.  It survived
light testing. Avi, could you please take a look at this patch?
Thanks!


---
 cpu-common.h |   19 +
 exec.c   |   62 +++--
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/cpu-common.h b/cpu-common.h
index 6302372..0ec9b72 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -8,6 +8,7 @@
 #endif
 
 #include bswap.h
+#include qemu-queue.h
 
 /* address in the RAM (different from a physical address) */
 typedef unsigned long ram_addr_t;
@@ -61,6 +62,24 @@ void cpu_physical_memory_unmap(void *buffer, 
target_phys_addr_t len,
 void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque));
 void cpu_unregister_map_client(void *cookie);
 
+struct CPUPhysMemoryClient;
+typedef struct CPUPhysMemoryClient CPUPhysMemoryClient;
+struct CPUPhysMemoryClient {
+void (*set_memory)(struct CPUPhysMemoryClient *client,
+   target_phys_addr_t start_addr,
+   ram_addr_t size,
+   ram_addr_t phys_offset);
+int (*sync_dirty_bitmap)(struct CPUPhysMemoryClient *client,
+ target_phys_addr_t start_addr,
+ target_phys_addr_t end_addr);
+int (*migration_log)(struct CPUPhysMemoryClient *client,
+ int enable);
+QLIST_ENTRY(CPUPhysMemoryClient) list;
+};
+
+void cpu_register_phys_memory_client(CPUPhysMemoryClient *);
+void cpu_unregister_phys_memory_client(CPUPhysMemoryClient *);
+
 uint32_t ldub_phys(target_phys_addr_t addr);
 uint32_t lduw_phys(target_phys_addr_t addr);
 uint32_t ldl_phys(target_phys_addr_t addr);
diff --git a/exec.c b/exec.c
index 7b7fb5b..daebde5 100644
--- a/exec.c
+++ b/exec.c
@@ -1880,11 +1880,16 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start, 
ram_addr_t end,
 
 int cpu_physical_memory_set_dirty_tracking(int enable)
 {
+int ret;
 in_migration = enable;
 if (kvm_enabled()) {
-return kvm_set_migration_log(enable);
+ret = kvm_set_migration_log(enable);
 }
-return 0;
+if (ret  0) {
+return ret;
+}
+ret = cpu_notify_migration_log(!!enable);
+return ret;
 }
 
 int cpu_physical_memory_get_dirty_tracking(void)
@@ -1897,8 +1902,13 @@ int cpu_physical_sync_dirty_bitmap(target_phys_addr_t 
start_addr,
 {
 int ret = 0;
 
-if (kvm_enabled())
+if (kvm_enabled()) {
 ret = kvm_physical_sync_dirty_bitmap(start_addr, end_addr);
+}
+if (ret  0) {
+return ret;
+}
+ret = cpu_notify_sync_dirty_bitmap(start_addr, end_addr);
 return ret;
 }
 
@@ -2313,6 +2323,8 @@ void 
cpu_register_physical_memory_offset(target_phys_addr_t start_addr,
 if (kvm_enabled())
 kvm_set_phys_mem(start_addr, size, phys_offset);
 
+cpu_notify_set_memory(start_addr, size, phys_offset);
+
 if (phys_offset == IO_MEM_UNASSIGNED) {
 region_offset = start_addr;
 }
@@ -3214,6 +3226,50 @@ static void cpu_notify_map_clients(void)
 }
 }
 
+static QLIST_HEAD(memory_client_list, CPUPhysMemoryClient) memory_client_list
+= QLIST_HEAD_INITIALIZER(memory_client_list);
+
+void cpu_register_phys_memory_client(CPUPhysMemoryClient *client)
+{
+QLIST_INSERT_HEAD(memory_client_list, client, list);
+}
+
+void cpu_unregister_phys_memory_client(CPUPhysMemoryClient *client)
+{
+QLIST_REMOVE(client, list);
+}
+
+static void cpu_notify_set_memory(target_phys_addr_t start_addr,
+ ram_addr_t size,
+ ram_addr_t phys_offset)
+{
+CPUPhysMemoryClient *client;
+QLIST_FOREACH(client, memory_client_list, list) {
+client-set_memory(client, start_addr, size, phys_offset);
+}
+}
+
+static int cpu_notify_sync_dirty_bitmap(target_phys_addr_t start,
+   target_phys_addr_t end)
+{
+QLIST_FOREACH(client, memory_client_list, list) {
+int r = client-sync_dirty_bitmap(client, start, end);
+if (r  0)
+return r;
+}
+return 0;
+}
+
+static int cpu_notify_migration_log(int enable)
+{
+QLIST_FOREACH(client, memory_client_list, list) {
+int r = client-migration_log(client, enable);
+if (r  0)
+return r;
+}
+return 0;
+}
+
 /* Map a physical memory region into a host virtual address.
  * May map a 

Re: [Qemu-devel] [PATCH] target-arm: ARMv4 emulation

2010-01-14 Thread Filip Navara
On Fri, Dec 18, 2009 at 11:05 PM, Aurelien Jarno aurel...@aurel32.netwrote:

 On Fri, Dec 18, 2009 at 10:32:37PM +0100, Marc Andre Tanner wrote:
  On Tue, Dec 01, 2009 at 09:41:43PM +0100, Filip Navara wrote:
   While most of the ARMv5 instructions are backward compatible with
 ARMv4, there
   are few important differences. Most notably the stack pop and load
 instructions
   ignore the lowest bit, which is used by ARMv5 to switch to Thumb mode.
 A
   base-updated data-abort model is used on ARM7TDMI, CP15 coprocessor is
 not
   present and several instructions of later architectures are not
 implemented.
  
   This patch introduces flags for the V5, CP15 and ABORT_BU (base-updated
 abort
   model) features. When V5 feature is not set the bit 0 on POP, LD and
 LDM of PC
   register is ignored and doesn't swith to/from Thumb mode and several
   instructions are treated as unimplemented (BLX, PLD, BKPT, LDRD, STRD).
  
   Added are processor definitions for ARM7TDMI and ARM920T.
  
   Based on patches by Ulrich Hecht u...@suse.de and Vincent Sanders 
 vi...@kyllikki.org.
 
  Could someone please review + apply this patch, bonus points if it ends
  up in 0.12.
 

 ARM stuff is usually reviewed by Paul Brook. Paul, do you plan to review
 it, or should I do it?

 I would appreciate at least some progress on this patch. A no, because...
would be fine with me too.

Best regards,
Filip Navara


Re: [Qemu-devel] [PATCH] target-arm: ARMv4 emulation

2010-01-14 Thread Laurent Desnogues
Since you wanted a no, because... there's at least
one :-)

On Tue, Dec 1, 2009 at 9:41 PM, Filip Navara filip.nav...@gmail.com wrote:
 While most of the ARMv5 instructions are backward compatible with ARMv4, there
 are few important differences. Most notably the stack pop and load 
 instructions
 ignore the lowest bit, which is used by ARMv5 to switch to Thumb mode. A
 base-updated data-abort model is used on ARM7TDMI, CP15 coprocessor is not
 present and several instructions of later architectures are not implemented.

 This patch introduces flags for the V5, CP15 and ABORT_BU (base-updated abort
 model) features. When V5 feature is not set the bit 0 on POP, LD and LDM of PC
 register is ignored and doesn't swith to/from Thumb mode and several
 instructions are treated as unimplemented (BLX, PLD, BKPT, LDRD, STRD).

 Added are processor definitions for ARM7TDMI and ARM920T.

 Based on patches by Ulrich Hecht u...@suse.de and Vincent Sanders 
 vi...@kyllikki.org.

 Signed-off-by: Filip Navara filip.nav...@gmail.com
 ---
  target-arm/cpu.h       |    7 ++-
  target-arm/helper.c    |   31 
  target-arm/translate.c |  119 ++-
  3 files changed, 123 insertions(+), 34 deletions(-)

 diff --git a/target-arm/cpu.h b/target-arm/cpu.h
 index 4a1c53f..b8e1e4b 100644
 --- a/target-arm/cpu.h
 +++ b/target-arm/cpu.h
 @@ -334,6 +334,7 @@ enum arm_features {
     ARM_FEATURE_AUXCR,  /* ARM1026 Auxiliary control register.  */
     ARM_FEATURE_XSCALE, /* Intel XScale extensions.  */
     ARM_FEATURE_IWMMXT, /* Intel iwMMXt extension.  */
 +    ARM_FEATURE_V5,
     ARM_FEATURE_V6,
     ARM_FEATURE_V6K,
     ARM_FEATURE_V7,
 @@ -345,7 +346,9 @@ enum arm_features {
     ARM_FEATURE_DIV,
     ARM_FEATURE_M, /* Microcontroller profile.  */
     ARM_FEATURE_OMAPCP, /* OMAP specific CP15 ops handling.  */
 -    ARM_FEATURE_THUMB2EE
 +    ARM_FEATURE_THUMB2EE,
 +    ARM_FEATURE_CP15, /* ARM7TDMI, ARM7TDMI-S, ARM7EJ-S, and ARM9TDMI cores 
 do not have a CP15 */
 +    ARM_FEATURE_ABORT_BU /* base updated abort model, e.g. ARMxTDMI */
  };

  static inline int arm_feature(CPUARMState *env, int feature)
 @@ -371,6 +374,8 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
  #define IS_M(env) arm_feature(env, ARM_FEATURE_M)
  #define ARM_CPUID(env) (env-cp15.c0_cpuid)

 +#define ARM_CPUID_ARM7TDMI    0x41807000 /* guess; no CP15 on ARM7TDMI */
 +#define ARM_CPUID_ARM920T     0x41129200
  #define ARM_CPUID_ARM1026     0x4106a262
  #define ARM_CPUID_ARM926      0x41069265
  #define ARM_CPUID_ARM946      0x41059461
 diff --git a/target-arm/helper.c b/target-arm/helper.c
 index b3aec99..b82959f 100644
 --- a/target-arm/helper.c
 +++ b/target-arm/helper.c
 @@ -44,19 +44,34 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
 id)
  {
     env-cp15.c0_cpuid = id;
     switch (id) {
 +    case ARM_CPUID_ARM7TDMI:
 +        set_feature(env, ARM_FEATURE_ABORT_BU);
 +        break;
 +    case ARM_CPUID_ARM920T:
 +        set_feature(env, ARM_FEATURE_ABORT_BU);

ARM920T is using an ARM9TDMI which uses the Base Restored
Data Abort Model and not the Updated one.

Ref: 
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0180a/ch02s01s01.html


Laurent

 +        set_feature(env, ARM_FEATURE_CP15);
 +        env-cp15.c0_cachetype = 0x0d172172;
 +        env-cp15.c1_sys = 0x0078;
 +        break;
     case ARM_CPUID_ARM926:
 +        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_VFP);
 +        set_feature(env, ARM_FEATURE_CP15);
         env-vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
         env-cp15.c0_cachetype = 0x1dd20d2;
         env-cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM946:
 +        set_feature(env, ARM_FEATURE_V5);
 +        set_feature(env, ARM_FEATURE_CP15);
         set_feature(env, ARM_FEATURE_MPU);
         env-cp15.c0_cachetype = 0x0f004006;
         env-cp15.c1_sys = 0x0078;
         break;
     case ARM_CPUID_ARM1026:
 +        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_VFP);
 +        set_feature(env, ARM_FEATURE_CP15);
         set_feature(env, ARM_FEATURE_AUXCR);
         env-vfp.xregs[ARM_VFP_FPSID] = 0x410110a0;
         env-cp15.c0_cachetype = 0x1dd20d2;
 @@ -64,8 +79,10 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
 id)
         break;
     case ARM_CPUID_ARM1136_R2:
     case ARM_CPUID_ARM1136:
 +        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_VFP);
 +        set_feature(env, ARM_FEATURE_CP15);
         set_feature(env, ARM_FEATURE_AUXCR);
         env-vfp.xregs[ARM_VFP_FPSID] = 0x410120b4;
         env-vfp.xregs[ARM_VFP_MVFR0] = 0x;
 @@ -75,9 +92,11 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t 
 id)
         env-cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_ARM11MPCORE:
 +        set_feature(env, ARM_FEATURE_V5);
         

Re: [Qemu-devel] [PATCH 1/2] block: flush backing_hd in the right place

2010-01-14 Thread Kevin Wolf
Am 14.01.2010 00:26, schrieb Anthony Liguori:
 On 01/12/2010 06:49 AM, Christoph Hellwig wrote:
 The backing device is only modified from bdrv_commit.  So instead of
 flushing it every time bdrv_flush is called for the front-end device
 only flush it after we're written data to it in bdrv_commit.

 Signed-off-by: Christoph Hellwigh...@lst.de

 
 Applied.  Thanks.

Anthony, you seem to have missed the v2 patch that considered my review
comments. Can you please apply the diff between v1 and v2 on top?

Kevin




[Qemu-devel] Re: [SeaBIOS] Advise on updating SeaBIOS in stable

2010-01-14 Thread Gerd Hoffmann

  Hi,


If you're looking to pull in 32bit pcibios support, then I don't think
it would be worthwhile to rebase to a stable branch, as the 32bit
pcibios support is easily the biggest user visible change in v0.5.1
(in the sense that Linux will call 32bit pcibios if it's available).


Unless there's a strong demand for it, I'd like to hold off on 32bit
pcibios support.


I think someone mentioned bochs-based pcbios in qemu 0.11 has 32bit 
pcibios support, so not having that in 0.12 would be a regression. 
Dunno how much this is a problem in practice though.


cheers,
  Gerd




Re: [Qemu-devel] [PATCH v3] raw-posix: Detect CDROM via ioctl

2010-01-14 Thread malc
On Wed, 13 Jan 2010, Cole Robinson wrote:

 On 01/13/2010 07:11 PM, malc wrote:
  On Wed, 13 Jan 2010, Cole Robinson wrote:
  
  Current CDROM detection is hardcoded based on source file name.
  Make this smarter by attempting a CDROM specific ioctl.
 
  This makes '-cdrom /dev/sr0' succeed with no media present.
 
  v2:
  Give ioctl check higher priority than filename check,
 
  v3:
  Actually initialize 'prio' variable
  Check for ioctl success rather than absence of specific failure
  
  Does it even compile on BSDs, Darwin etc?
  
 
 The changed functions are all under #ifdef __linux__, so I assume its fine.
 Haven't tested though.

Fine if so, it's not immediatelly aparent from the patches nor from
the commit messages though.

-- 
mailto:av1...@comtv.ru




[Qemu-devel] Re: [PATCH 1/3] eepro100: Restructure code (new function tx_command)

2010-01-14 Thread Michael S. Tsirkin
On Sun, Dec 20, 2009 at 04:52:22PM +0100, Stefan Weil wrote:
 Handling of transmit commands is rather complex,
 so about 80 lines of code were moved from function
 action_command to the new function tx_command.
 
 The two new values tx and cb_address in the
 eepro100 status structure made this possible without
 passing too many parameters.
 
 In addition, the moved code was cleaned a little bit:
 old comments marked with //~ were removed, C++ style
 comments were replaced by C style comments, C++ like
 variable declarations after code were reordered.
 
 Simplified mode is still broken. Nor did I fix
 endianess issues. Both problems will be fixed in
 additional patches (which need this one).
 
 Signed-off-by: Stefan Weil w...@mail.berlios.de

Thanks, applied.
I merged this with the following commit: no reason
to change the same line twice.

 ---
  hw/eepro100.c |  192 
 +
  1 files changed, 98 insertions(+), 94 deletions(-)
 
 diff --git a/hw/eepro100.c b/hw/eepro100.c
 index 2a9e3b5..5635f61 100644
 --- a/hw/eepro100.c
 +++ b/hw/eepro100.c
 @@ -213,6 +213,10 @@ typedef struct {
  uint32_t ru_offset; /* RU address offset */
  uint32_t statsaddr; /* pointer to eepro100_stats_t */
  
 +/* Temporary data. */
 +eepro100_tx_t tx;
 +uint32_t cb_address;
 +
  /* Statistical counters. Also used for wake-up packet (i82559). */
  eepro100_stats_t statistics;
  
 @@ -748,17 +752,100 @@ static void dump_statistics(EEPRO100State * s)
  //~ missing(CU dump statistical counters);
  }
  
 +static void tx_command(EEPRO100State *s)
 +{
 +uint32_t tbd_array = le32_to_cpu(s-tx.tx_desc_addr);
 +uint16_t tcb_bytes = (le16_to_cpu(s-tx.tcb_bytes)  0x3fff);
 +/* Sends larger than MAX_ETH_FRAME_SIZE are allowed, up to 2600 bytes. */
 +uint8_t buf[2600];
 +uint16_t size = 0;
 +uint32_t tbd_address = s-cb_address + 0x10;
 +TRACE(RXTX, logout
 +(transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD 
 count %u\n,
 + tbd_array, tcb_bytes, s-tx.tbd_count));
 +
 +if (tcb_bytes  2600) {
 +logout(TCB byte count too large, using 2600\n);
 +tcb_bytes = 2600;
 +}
 +if (!((tcb_bytes  0) || (tbd_array != 0x))) {
 +logout
 +(illegal values of TBD array address and TCB byte count!\n);
 +}
 +assert(tcb_bytes = sizeof(buf));
 +while (size  tcb_bytes) {
 +uint32_t tx_buffer_address = ldl_phys(tbd_address);
 +uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
 +//~ uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
 +tbd_address += 8;
 +TRACE(RXTX, logout
 +(TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n,
 + tx_buffer_address, tx_buffer_size));
 +tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
 +cpu_physical_memory_read(tx_buffer_address, buf[size],
 + tx_buffer_size);
 +size += tx_buffer_size;
 +}
 +if (tbd_array == 0x) {
 +/* Simplified mode. Was already handled by code above. */
 +} else {
 +/* Flexible mode. */
 +uint8_t tbd_count = 0;
 +if (s-has_extended_tcb_support  !(s-configuration[6]  BIT(4))) {
 +/* Extended Flexible TCB. */
 +for (; tbd_count  2; tbd_count++) {
 +uint32_t tx_buffer_address = ldl_phys(tbd_address);
 +uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
 +uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
 +tbd_address += 8;
 +TRACE(RXTX, logout
 +(TBD (extended flexible mode): buffer address 0x%08x, 
 size 0x%04x\n,
 + tx_buffer_address, tx_buffer_size));
 +tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
 +cpu_physical_memory_read(tx_buffer_address, buf[size],
 + tx_buffer_size);
 +size += tx_buffer_size;
 +if (tx_buffer_el  1) {
 +break;
 +}
 +}
 +}
 +tbd_address = tbd_array;
 +for (; tbd_count  s-tx.tbd_count; tbd_count++) {
 +uint32_t tx_buffer_address = ldl_phys(tbd_address);
 +uint16_t tx_buffer_size = lduw_phys(tbd_address + 4);
 +uint16_t tx_buffer_el = lduw_phys(tbd_address + 6);
 +tbd_address += 8;
 +TRACE(RXTX, logout
 +(TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n,
 + tx_buffer_address, tx_buffer_size));
 +tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
 +cpu_physical_memory_read(tx_buffer_address, buf[size],
 + tx_buffer_size);
 +size += tx_buffer_size;
 +if (tx_buffer_el  1) 

[Qemu-devel] [PATCH 8/8] Move virtio-serial to Makefile.objs

2010-01-14 Thread Amit Shah
There's nothing target-dependent in the virtio-serial code so allow it
to be compiled just once for all the targets.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 Makefile.objs   |2 +-
 Makefile.target |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 5802d39..77ff7f6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -127,7 +127,7 @@ user-obj-y += cutils.o cache-utils.o
 
 hw-obj-y =
 hw-obj-y += loader.o
-hw-obj-y += virtio.o
+hw-obj-y += virtio.o virtio-serial.o
 hw-obj-y += fw_cfg.o
 hw-obj-y += watchdog.o
 hw-obj-$(CONFIG_ECC) += ecc.o
diff --git a/Makefile.target b/Makefile.target
index 60df16d..0bf2253 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -172,7 +172,7 @@ ifdef CONFIG_SOFTMMU
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial.o 
virtio-serial-bus.o virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o 
virtio-serial-bus.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 LIBS+=-lz
-- 
1.6.2.5





[Qemu-devel] [PATCH 4/8] virtio-serial-bus: Add a port 'name' property for port discovery in guests

2010-01-14 Thread Amit Shah
The port 'id' or number is internal state between the guest kernel and
our bus implementation. This is invocation-dependent and isn't part of
the guest-host ABI.

To correcly enumerate and map ports between the host and the guest, the
'name' property is used.

Example:

-device virtserialport,name=org.qemu.port.0

This invocation will get us a char device in the guest at:

/dev/virtio-ports/org.qemu.port.0

which can be a symlink to

/dev/vport0p3

This 'name' property is exposed by the guest kernel in a sysfs
attribute:

/sys/kernel/virtio-ports/vport0p3/name

A simple udev script can pick up this name and create the symlink
mentioned above.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |   17 +
 hw/virtio-serial.c |1 +
 hw/virtio-serial.h |8 
 3 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 276de6a..2576140 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -226,6 +226,8 @@ static void handle_control_message(VirtIOSerial *vser, void 
*buf)
 {
 struct VirtIOSerialPort *port;
 struct virtio_console_control cpkt, *gcpkt;
+uint8_t *buffer;
+size_t buffer_len;
 
 gcpkt = buf;
 port = find_port_by_id(vser, ldl_p(gcpkt-id));
@@ -248,6 +250,21 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf)
 send_control_event(port, VIRTIO_CONSOLE_CONSOLE_PORT, 1);
 }
 
+if (port-name) {
+stw_p(cpkt.event, VIRTIO_CONSOLE_PORT_NAME);
+stw_p(cpkt.value, 1);
+
+buffer_len = sizeof(cpkt) + strlen(port-name) + 1;
+buffer = qemu_malloc(buffer_len);
+
+memcpy(buffer, cpkt, sizeof(cpkt));
+memcpy(buffer + sizeof(cpkt), port-name, strlen(port-name));
+buffer[buffer_len - 1] = 0;
+
+send_control_msg(port, buffer, buffer_len);
+qemu_free(buffer);
+}
+
 if (port-host_connected) {
 send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
 }
diff --git a/hw/virtio-serial.c b/hw/virtio-serial.c
index d9a6f32..470446b 100644
--- a/hw/virtio-serial.c
+++ b/hw/virtio-serial.c
@@ -102,6 +102,7 @@ static VirtIOSerialPortInfo virtconsole_info = {
 .qdev.props = (Property[]) {
 DEFINE_PROP_UINT8(is_console, VirtConsole, port.is_console, 1),
 DEFINE_PROP_CHR(chardev, VirtConsole, chr),
+DEFINE_PROP_STRING(name, VirtConsole, port.name),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 1576eef..b669c7f 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -54,6 +54,7 @@ struct virtio_console_header {
 #define VIRTIO_CONSOLE_CONSOLE_PORT1
 #define VIRTIO_CONSOLE_RESIZE  2
 #define VIRTIO_CONSOLE_PORT_OPEN   3
+#define VIRTIO_CONSOLE_PORT_NAME   4
 
 /* == In-qemu interface == */
 
@@ -88,6 +89,13 @@ struct VirtIOSerialPort {
 VirtQueue *ivq, *ovq;
 
 /*
+ * This name is sent to the guest and exported via sysfs.
+ * The guest could create symlinks based on this information.
+ * The name is in the reverse fqdn format, like org.qemu.console.0
+ */
+char *name;
+
+/*
  * This id helps identify ports between the guest and the host.
  * The guest sends a header with this id with each data packet
  * that it sends and the host can then find out which associated
-- 
1.6.2.5





[Qemu-devel] [PATCH 2/8] virtio-console: qdev conversion, new virtio-serial-bus

2010-01-14 Thread Amit Shah
This commit converts the virtio-console device to create a new
virtio-serial bus that can host console and generic serial ports. The
file hosting this code is now called virtio-serial-bus.c.

The virtio console is now a very simple qdev device that sits on the
virtio-serial-bus and communicates between the bus and qemu's chardevs.

This commit also includes a few changes to the virtio backing code for
pci and s390 to spawn the virtio-serial bus.

As a result of the qdev conversion, we get rid of a lot of legacy code.
The old-style way of instantiating a virtio console using

-virtioconsole ...

is maintained, but the new, preferred way is to use

-device virtio-serial -device virtconsole,chardev=...

With this commit, multiple devices as well as multiple ports with a
single device can be supported.

For multiple ports support, each port gets an IO vq pair. Since the
guest needs to know in advance how many vqs a particular device will
need, we have to set this number as a property of the virtio-serial
device and also as a config option.

In addition, we also spawn a pair of control IO vqs. This is an internal
channel meant for guest-host communication for things like port
open/close, sending port properties over to the guest, etc.

This commit is a part of a series of other commits to get the full
implementation of multiport support. Future commits will add other
support as well as ride on the savevm version that we bump up here.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 Makefile.target|2 +-
 hw/pc.c|   11 +-
 hw/ppc440_bamboo.c |7 -
 hw/qdev.c  |   10 +-
 hw/s390-virtio-bus.c   |   17 +-
 hw/s390-virtio-bus.h   |2 +
 hw/s390-virtio.c   |8 -
 hw/virtio-console.c|  143 -
 hw/virtio-console.h|   19 --
 hw/virtio-pci.c|   13 +-
 hw/virtio-serial-bus.c |  539 
 hw/virtio-serial.c |  107 ++
 hw/virtio-serial.h |  162 +++
 hw/virtio.h|2 +-
 qemu-options.hx|4 +
 sysemu.h   |6 -
 vl.c   |2 +
 17 files changed, 841 insertions(+), 213 deletions(-)
 delete mode 100644 hw/virtio-console.c
 delete mode 100644 hw/virtio-console.h
 create mode 100644 hw/virtio-serial-bus.c
 create mode 100644 hw/virtio-serial.c
 create mode 100644 hw/virtio-serial.h

diff --git a/Makefile.target b/Makefile.target
index e661478..60df16d 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -172,7 +172,7 @@ ifdef CONFIG_SOFTMMU
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
-obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o 
virtio-pci.o
+obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-serial.o 
virtio-serial-bus.o virtio-pci.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_ISA_MMIO) += isa_mmio.o
 LIBS+=-lz
diff --git a/hw/pc.c b/hw/pc.c
index a93c5f2..3aadfa9 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1018,15 +1018,6 @@ static void pc_init1(ram_addr_t ram_size,
 pci_create_simple(pci_bus, -1, lsi53c895a);
 }
 }
-
-/* Add virtio console devices */
-if (pci_enabled) {
-for(i = 0; i  MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pci_bus, -1, virtio-console-pci);
-}
-}
-}
 }
 
 static void pc_init_pci(ram_addr_t ram_size,
@@ -1102,7 +1093,7 @@ static QEMUMachine pc_machine_v0_10 = {
 .property = class,
 .value= stringify(PCI_CLASS_STORAGE_OTHER),
 },{
-.driver   = virtio-console-pci,
+.driver   = virtio-serial-pci,
 .property = class,
 .value= stringify(PCI_CLASS_DISPLAY_OTHER),
 },{
diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c
index a488240..1ab9872 100644
--- a/hw/ppc440_bamboo.c
+++ b/hw/ppc440_bamboo.c
@@ -108,13 +108,6 @@ static void bamboo_init(ram_addr_t ram_size,
 env = ppc440ep_init(ram_size, pcibus, pci_irq_nrs, 1, cpu_model);
 
 if (pcibus) {
-/* Add virtio console devices */
-for(i = 0; i  MAX_VIRTIO_CONSOLES; i++) {
-if (virtcon_hds[i]) {
-pci_create_simple(pcibus, -1, virtio-console-pci);
-}
-}
-
 /* Register network interfaces. */
 for (i = 0; i  nb_nics; i++) {
 /* There are no PCI NICs on the Bamboo board, but there are
diff --git a/hw/qdev.c b/hw/qdev.c
index b6bd4ae..c643576 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -321,13 +321,9 @@ void qdev_machine_creation_done(void)
 CharDriverState *qdev_init_chardev(DeviceState *dev)
 {
 static int next_serial;
-static int next_virtconsole;
-/* FIXME: This is a nasty hack that needs to go away.  */
-if (strncmp(dev-info-name, virtio, 6) == 0) {

[Qemu-devel] [PATCH 2/3] scsi: device version property

2010-01-14 Thread Gerd Hoffmann
This patch adds a new property named 'ver' to scsi-disk which allows to
specify the version which the virtual disk/cdrom should report to the
guest.  By default this is the qemu version (i.e. 0.12).  usage:

  -drive if=none,id=disk,file=...
  -device lsi
  -device scsi-disk,drive=disk,bus=scsi.0,unit=0,ver=42

You can also switch the version for all scsi drives using:

  -global scsi-disk.ver=42

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/scsi-disk.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index eb5b5a8..e3924de 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -65,6 +65,7 @@ struct SCSIDiskState
 int cluster_size;
 uint64_t max_lba;
 QEMUBH *bh;
+char *version;
 };
 
 static SCSIDiskReq *scsi_new_request(SCSIDevice *d, uint32_t tag, uint32_t lun)
@@ -315,6 +316,7 @@ static uint8_t *scsi_get_buf(SCSIDevice *d, uint32_t tag)
 static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
 {
 BlockDriverState *bdrv = req-dev-dinfo-bdrv;
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req-dev);
 int buflen = 0;
 
 if (req-cmd.buf[1]  0x2) {
@@ -432,7 +434,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 memcpy(outbuf[16], QEMU HARDDISK   , 16);
 }
 memcpy(outbuf[8], QEMU, 8);
-memcpy(outbuf[32], QEMU_VERSION, 4);
+memcpy(outbuf[32], s-version ? s-version : QEMU_VERSION, 4);
 /* Identify device as SCSI-3 rev 1.
Some later commands are also implemented. */
 outbuf[2] = 3;
@@ -1029,6 +1031,7 @@ static SCSIDeviceInfo scsi_disk_info = {
 .get_buf  = scsi_get_buf,
 .qdev.props   = (Property[]) {
 DEFINE_PROP_DRIVE(drive, SCSIDiskState, qdev.dinfo),
+DEFINE_PROP_STRING(ver,  SCSIDiskState, version),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
-- 
1.6.5.2





[Qemu-devel] [PATCH 1/3] ide: device version property

2010-01-14 Thread Gerd Hoffmann
This patch adds a new property named 'ver' to ide-drive which allows to
specify the version which the virtual disk/cdrom should report to the
guest.  By default this is the qemu version (i.e. 0.12).  usage:

  -drive if=none,id=disk,file=...
  -device ide-drive,bus=ide.0,unit=0,drive=disk,ver=42

You can also switch the version for all ide drives using:

  -global ide-drive.ver=42

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/ide/core.c |   19 ---
 hw/ide/internal.h |4 +++-
 hw/ide/qdev.c |3 ++-
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 76c3820..b6643e8 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -115,7 +115,7 @@ static void ide_identify(IDEState *s)
 put_le16(p + 20, 3); /* XXX: retired, remove ? */
 put_le16(p + 21, 512); /* cache size in sectors */
 put_le16(p + 22, 4); /* ecc bytes */
-padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
+padstr((char *)(p + 23), s-version, 8); /* firmware version */
 padstr((char *)(p + 27), QEMU HARDDISK, 40); /* model */
 #if MAX_MULT_SECTORS  1
 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
@@ -186,7 +186,7 @@ static void ide_atapi_identify(IDEState *s)
 put_le16(p + 20, 3); /* buffer type */
 put_le16(p + 21, 512); /* cache size in sectors */
 put_le16(p + 22, 4); /* ecc bytes */
-padstr((char *)(p + 23), QEMU_VERSION, 8); /* firmware version */
+padstr((char *)(p + 23), s-version, 8); /* firmware version */
 padstr((char *)(p + 27), QEMU DVD-ROM, 40); /* model */
 put_le16(p + 48, 1); /* dword I/O (XXX: should not be set on CDROM) */
 #ifdef USE_DMA_CDROM
@@ -238,7 +238,7 @@ static void ide_cfata_identify(IDEState *s)
 put_le16(p + 8, s-nb_sectors);/* Sectors per card */
 padstr((char *)(p + 10), s-drive_serial_str, 20); /* serial number */
 put_le16(p + 22, 0x0004);  /* ECC bytes */
-padstr((char *) (p + 23), QEMU_VERSION, 8);/* Firmware Revision */
+padstr((char *) (p + 23), s-version, 8);  /* Firmware Revision */
 padstr((char *) (p + 27), QEMU MICRODRIVE, 40);/* Model number */
 #if MAX_MULT_SECTORS  1
 put_le16(p + 47, 0x8000 | MAX_MULT_SECTORS);
@@ -1591,7 +1591,7 @@ static void ide_atapi_cmd(IDEState *s)
 buf[7] = 0; /* reserved */
 padstr8(buf + 8, 8, QEMU);
 padstr8(buf + 16, 16, QEMU DVD-ROM);
-padstr8(buf + 32, 4, QEMU_VERSION);
+padstr8(buf + 32, 4, s-version);
 ide_atapi_cmd_reply(s, 36, max_len);
 break;
 case GPCMD_GET_CONFIGURATION:
@@ -2590,7 +2590,7 @@ void ide_bus_reset(IDEBus *bus)
 ide_clear_hob(bus);
 }
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo)
+void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version)
 {
 int cylinders, heads, secs;
 uint64_t nb_sectors;
@@ -2619,6 +2619,11 @@ void ide_init_drive(IDEState *s, DriveInfo *dinfo)
 if (strlen(s-drive_serial_str) == 0)
 snprintf(s-drive_serial_str, sizeof(s-drive_serial_str),
  QM%05d, s-drive_serial);
+if (version) {
+pstrcpy(s-version, sizeof(s-version), version);
+} else {
+pstrcpy(s-version, sizeof(s-version), QEMU_VERSION);
+}
 ide_reset(s);
 }
 
@@ -2639,9 +2644,9 @@ void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo 
*hd1,
 s-sector_write_timer = qemu_new_timer(vm_clock,
ide_sector_write_timer_cb, s);
 if (i == 0)
-ide_init_drive(s, hd0);
+ide_init_drive(s, hd0, NULL);
 if (i == 1)
-ide_init_drive(s, hd1);
+ide_init_drive(s, hd1, NULL);
 }
 bus-irq = irq;
 }
diff --git a/hw/ide/internal.h b/hw/ide/internal.h
index eb5b404..1cc4b55 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -397,6 +397,7 @@ struct IDEState {
 /* set for lba48 access */
 uint8_t lba48;
 BlockDriverState *bs;
+char version[9];
 /* ATAPI specific */
 uint8_t sense_key;
 uint8_t asc;
@@ -449,6 +450,7 @@ struct IDEDevice {
 DeviceState qdev;
 uint32_t unit;
 DriveInfo *dinfo;
+char *version;
 };
 
 typedef int (*ide_qdev_initfn)(IDEDevice *dev);
@@ -549,7 +551,7 @@ uint32_t ide_data_readw(void *opaque, uint32_t addr);
 void ide_data_writel(void *opaque, uint32_t addr, uint32_t val);
 uint32_t ide_data_readl(void *opaque, uint32_t addr);
 
-void ide_init_drive(IDEState *s, DriveInfo *dinfo);
+void ide_init_drive(IDEState *s, DriveInfo *dinfo, const char *version);
 void ide_init2(IDEBus *bus, DriveInfo *hd0, DriveInfo *hd1,
qemu_irq irq);
 void ide_init_ioport(IDEBus *bus, int iobase, int iobase2);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 81e7995..0b84a4f 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -99,7 +99,7 @@ typedef struct IDEDrive {
 static int ide_drive_initfn(IDEDevice *dev)
 {
 IDEBus *bus = DO_UPCAST(IDEBus, 

[Qemu-devel] [PATCH 0/3] add disk/cdrom version properties

2010-01-14 Thread Gerd Hoffmann
  Hi,

This patch series makes the version reported by ide and scsi drives
configurable using qdev properties.  Should help keeping the virtual
hardware more constant on qemu updates, so it becomes less likely
that Windows wants be re-activated.

Maybe 0.12 candidate?

cheers,
  Gerd





[Qemu-devel] [PATCH 6/8] virtio-serial-bus: Add ability to hot-unplug ports

2010-01-14 Thread Amit Shah
Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |2 ++
 hw/virtio-serial.h |1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 1ec67d2..d045ea5 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -686,6 +686,8 @@ static int virtser_port_qdev_exit(DeviceState *qdev)
 VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, dev-qdev);
 VirtIOSerial *vser = port-vser;
 
+send_control_event(port, VIRTIO_CONSOLE_PORT_REMOVE, 1);
+
 /*
  * Don't decrement nr_ports here; thus we keep a linearly
  * increasing port id. Not utilising an id again saves us a couple
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index ea87b7d..987835d 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -52,6 +52,7 @@ struct virtio_console_control {
 #define VIRTIO_CONSOLE_PORT_OPEN   3
 #define VIRTIO_CONSOLE_PORT_NAME   4
 #define VIRTIO_CONSOLE_THROTTLE_PORT   5
+#define VIRTIO_CONSOLE_PORT_REMOVE 6
 
 /* == In-qemu interface == */
 
-- 
1.6.2.5





[Qemu-devel] [PATCH 5/8] virtio-serial-bus: Add support for buffering guest output, throttling guests

2010-01-14 Thread Amit Shah
We have to buffer data from guest as ports may not consume all the data
in one go or the guest could be fast in sending data and the apps may
not consume at the same rate.

We keep caching data the guest sends us till a port accepts it. However,
this could lead to an OOM condition where a rogue process on the guest
could continue pumping in data while the host continues to cache it.

We introduce a per-port byte-limit property to alleviate this condition.
When this limit is reached, we send a control message to the guest
indicating it to not send us any more data till further indication. When
the number of bytes cached go lesser than the limit specified, we tell
the guest to restart sending data.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |  215 +++-
 hw/virtio-serial.c |6 --
 hw/virtio-serial.h |   30 ++-
 3 files changed, 203 insertions(+), 48 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 2576140..1ec67d2 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -44,6 +44,18 @@ struct VirtIOSerial {
 struct virtio_console_config config;
 };
 
+/* This struct holds individual buffers received for each port */
+typedef struct VirtIOSerialPortBuffer {
+QTAILQ_ENTRY(VirtIOSerialPortBuffer) next;
+
+uint8_t *buf;
+
+size_t len; /* length of the buffer */
+size_t offset; /* offset from which to consume data in the buffer */
+
+bool previous_failure; /* Did sending out this buffer fail previously? */
+} VirtIOSerialPortBuffer;
+
 static VirtIOSerialPort *find_port_by_id(VirtIOSerial *vser, uint32_t id)
 {
 VirtIOSerialPort *port;
@@ -75,11 +87,9 @@ static size_t write_to_port(VirtIOSerialPort *port,
 const uint8_t *buf, size_t size)
 {
 VirtQueueElement elem;
-struct virtio_console_header header;
 VirtQueue *vq;
 size_t offset = 0;
 size_t len = 0;
-int header_len;
 
 vq = port-ivq;
 if (!virtio_queue_ready(vq)) {
@@ -88,8 +98,6 @@ static size_t write_to_port(VirtIOSerialPort *port,
 if (!size) {
 return 0;
 }
-header.flags = 0;
-header_len = use_multiport(port-vser) ? sizeof(header) : 0;
 
 while (offset  size) {
 int i;
@@ -97,26 +105,14 @@ static size_t write_to_port(VirtIOSerialPort *port,
 if (!virtqueue_pop(vq, elem)) {
 break;
 }
-if (elem.in_sg[0].iov_len  header_len) {
-/* We can't even store our port number in this buffer. Bug? */
-qemu_error(virtio-serial: size %zd less than expected\n,
-elem.in_sg[0].iov_len);
-exit(1);
-}
-if (header_len) {
-memcpy(elem.in_sg[0].iov_base, header, header_len);
-}
 
 for (i = 0; offset  size  i  elem.in_num; i++) {
-/* Copy the header only in the first sg. */
-len = MIN(elem.in_sg[i].iov_len - header_len, size - offset);
+len = MIN(elem.in_sg[i].iov_len, size - offset);
 
-memcpy(elem.in_sg[i].iov_base + header_len, buf + offset, len);
+memcpy(elem.in_sg[i].iov_base, buf + offset, len);
 offset += len;
-header_len = 0;
 }
-header_len = use_multiport(port-vser) ? sizeof(header) : 0;
-virtqueue_push(vq, elem, len + header_len);
+virtqueue_push(vq, elem, len);
 }
 
 virtio_notify(port-vser-vdev, vq);
@@ -157,6 +153,96 @@ static size_t send_control_event(VirtIOSerialPort *port, 
uint16_t event,
 return send_control_msg(port, cpkt, sizeof(cpkt));
 }
 
+static void init_buf(VirtIOSerialPortBuffer *buf, uint8_t *buffer, size_t len)
+{
+buf-buf = buffer;
+buf-len = len;
+buf-offset = 0;
+buf-previous_failure = false;
+}
+
+static VirtIOSerialPortBuffer *alloc_buf(size_t len)
+{
+VirtIOSerialPortBuffer *buf;
+
+buf = qemu_malloc(sizeof(*buf));
+buf-buf = qemu_malloc(len);
+
+init_buf(buf, buf-buf, len);
+
+return buf;
+}
+
+static void free_buf(VirtIOSerialPortBuffer *buf)
+{
+qemu_free(buf-buf);
+qemu_free(buf);
+}
+
+static void flush_queue(VirtIOSerialPort *port)
+{
+VirtIOSerialPortBuffer *buf;
+size_t out_size;
+ssize_t ret;
+
+while ((buf = QTAILQ_FIRST(port-unflushed_buffers))) {
+QTAILQ_REMOVE(port-unflushed_buffers, buf, next);
+
+out_size = buf-len - buf-offset;
+if (!port-host_connected) {
+port-nr_bytes -= buf-len + buf-offset;
+free_buf(buf);
+continue;
+}
+
+ret = port-info-have_data(port, buf-buf + buf-offset, out_size);
+if (ret  out_size) {
+QTAILQ_INSERT_HEAD(port-unflushed_buffers, buf, next);
+}
+if (ret = 0) {
+/* We're not progressing at all */
+if (buf-previous_failure) {
+break;
+}
+buf-previous_failure = 

[Qemu-devel] Re: Advise on updating SeaBIOS in stable

2010-01-14 Thread Kevin O'Connor
On Wed, Jan 13, 2010 at 05:58:35PM -0600, Anthony Liguori wrote:
 I actually need the compiler fix to build on my laptop (F12) so I've
 included that too.  Care to take a look at
 git://git.qemu.org/seabios.git stable-0.5.0?  It survives some light
 testing and I'll be doing more thorough testing overnight.

I'd also include: 2ceeec9d, and 085debd9.  The first fixes a binutils
oddity, and the second is a straight-forward bug fix.

 If you want to add some more and/or tag a release, I'll resync again
 before cutting 0.12.2.
 
 If you're looking to pull in 32bit pcibios support, then I don't think
 it would be worthwhile to rebase to a stable branch, as the 32bit
 pcibios support is easily the biggest user visible change in v0.5.1
 (in the sense that Linux will call 32bit pcibios if it's available).
 
 Unless there's a strong demand for it, I'd like to hold off on 32bit
 pcibios support.

That makes sense.  I'll pull your branch into my tree as well.
However, I don't think I'll get time to look much closer at this until
tomorrow night.

-Kevin




[Qemu-devel] [PATCH 0/8] virtio-console: Move to qdev, multiple devices, generic ports

2010-01-14 Thread Amit Shah
Hello people,

This iteration of the series removes the START and END flags (and
hence the header associated with each buffer). That's the major change
since the last submission.

Please review.

Obligatory disclaimer:
This series splits up the patches by functionality. Note, however,
that patches 2-6 introduce some functionality that's advertised to the
guest as having to work all at once or not at all. Also, the savevm
version is bumped only once but save/restore state is added in each of
the patches. They are split only for easier reviewability.

The older virtio-console.c file is completely removed and a new
virtio-serial.c is introduced so that reviewing is easier. I can send a
later patch to rename virtio-serial.c back to virtio-console.c.

Amit Shah (8):
  virtio: Remove duplicate macro definition for max. virtqueues, bump
up the max
  virtio-console: qdev conversion, new virtio-serial-bus
  virtio-serial-bus: Maintain guest and host port open/close state
  virtio-serial-bus: Add a port 'name' property for port discovery in
guests
  virtio-serial-bus: Add support for buffering guest output, throttling
guests
  virtio-serial-bus: Add ability to hot-unplug ports
  virtio-serial: Add a 'virtserialport' device for generic serial port
support
  Move virtio-serial to Makefile.objs

 Makefile.objs  |2 +-
 Makefile.target|2 +-
 hw/pc.c|   11 +-
 hw/ppc440_bamboo.c |7 -
 hw/qdev.c  |   10 +-
 hw/s390-virtio-bus.c   |   17 +-
 hw/s390-virtio-bus.h   |2 +
 hw/s390-virtio.c   |8 -
 hw/virtio-console.c|  143 -
 hw/virtio-console.h|   19 --
 hw/virtio-pci.c|   13 +-
 hw/virtio-serial-bus.c |  788 
 hw/virtio-serial.c |  143 +
 hw/virtio-serial.h |  199 
 hw/virtio.c|2 -
 hw/virtio.h|4 +-
 qemu-options.hx|4 +
 sysemu.h   |6 -
 vl.c   |2 +
 19 files changed, 1165 insertions(+), 217 deletions(-)
 delete mode 100644 hw/virtio-console.c
 delete mode 100644 hw/virtio-console.h
 create mode 100644 hw/virtio-serial-bus.c
 create mode 100644 hw/virtio-serial.c
 create mode 100644 hw/virtio-serial.h





[Qemu-devel] [PATCH 3/8] virtio-serial-bus: Maintain guest and host port open/close state

2010-01-14 Thread Amit Shah
Via control channel messages, the guest can tell us whether a port got
opened or closed. Similarly, we can also indicate to the guest of host
port open/close events.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio-serial-bus.c |   91 
 hw/virtio-serial.c |6 +++
 hw/virtio-serial.h |6 +++
 3 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index fcb86a6..276de6a 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -160,11 +160,22 @@ static size_t send_control_event(VirtIOSerialPort *port, 
uint16_t event,
 /* Functions for use inside qemu to open and read from/write to ports */
 int virtio_serial_open(VirtIOSerialPort *port)
 {
+/* Don't allow opening an already-open port */
+if (port-host_connected) {
+return 0;
+}
+/* Send port open notification to the guest */
+port-host_connected = true;
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
 return 0;
 }
 
 int virtio_serial_close(VirtIOSerialPort *port)
 {
+port-host_connected = false;
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
 return 0;
 }
 
@@ -172,6 +183,9 @@ int virtio_serial_close(VirtIOSerialPort *port)
 ssize_t virtio_serial_write(VirtIOSerialPort *port, const uint8_t *buf,
 size_t size)
 {
+if (!port || !port-host_connected || !port-guest_connected) {
+return 0;
+}
 return write_to_port(port, buf, size);
 }
 
@@ -192,6 +206,9 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
 virtio_queue_empty(vq)) {
 return 0;
 }
+if (use_multiport(port-vser)  !port-guest_connected) {
+return 0;
+}
 
 size = 4096;
 if (virtqueue_avail_bytes(vq, size, 0)) {
@@ -230,6 +247,11 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf)
 if (port-is_console) {
 send_control_event(port, VIRTIO_CONSOLE_CONSOLE_PORT, 1);
 }
+
+if (port-host_connected) {
+send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+}
+
 /*
  * When the guest has asked us for this information it means
  * the guest is all setup and has its virtqueues
@@ -240,6 +262,19 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf)
 port-info-guest_ready(port);
 }
 break;
+
+case VIRTIO_CONSOLE_PORT_OPEN:
+port-guest_connected = cpkt.value;
+if (cpkt.value  port-info-guest_open) {
+/* Send the guest opened notification if an app is interested */
+port-info-guest_open(port);
+}
+
+if (!cpkt.value  port-info-guest_close) {
+/* Send the guest closed notification if an app is interested */
+port-info-guest_close(port);
+}
+break;
 }
 }
 
@@ -334,6 +369,8 @@ static void set_config(VirtIODevice *vdev, const uint8_t 
*config_data)
 static void virtio_serial_save(QEMUFile *f, void *opaque)
 {
 VirtIOSerial *s = opaque;
+VirtIOSerialPort *port;
+uint32_t nr_active_ports;
 
 /* The virtio device */
 virtio_save(s-vdev, f);
@@ -342,15 +379,42 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
 qemu_put_be16s(f, s-config.cols);
 qemu_put_be16s(f, s-config.rows);
 qemu_put_be32s(f, s-config.nr_ports);
+
+/* Items in struct VirtIOSerial */
+
+/* Do this because we might have hot-unplugged some ports */
+nr_active_ports = 0;
+QTAILQ_FOREACH(port, s-ports, next)
+nr_active_ports++;
+
+qemu_put_be32s(f, nr_active_ports);
+
+/*
+ * Items in struct VirtIOSerialPort.
+ */
+QTAILQ_FOREACH(port, s-ports, next) {
+/*
+ * We put the port number because we may not have an active
+ * port at id 0 that's reserved for a console port, or in case
+ * of ports that might have gotten unplugged
+ */
+qemu_put_be32s(f, port-id);
+qemu_put_byte(f, port-guest_connected);
+
+}
 }
 
 static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
 {
 VirtIOSerial *s = opaque;
+VirtIOSerialPort *port;
+uint32_t nr_active_ports;
+unsigned int i;
 
 if (version_id  2) {
 return -EINVAL;
 }
+
 /* The virtio device */
 virtio_load(s-vdev, f);
 
@@ -363,6 +427,21 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 qemu_get_be16s(f, s-config.rows);
 s-config.nr_ports = qemu_get_be32(f);
 
+/* Items in struct VirtIOSerial */
+
+qemu_get_be32s(f, nr_active_ports);
+
+/* Items in struct VirtIOSerialPort */
+for (i = 0; i  nr_active_ports; i++) {
+uint32_t id;
+
+id = qemu_get_be32(f);
+port = find_port_by_id(s, id);
+
+port-guest_connected = qemu_get_byte(f);
+
+}
+
 return 0;
 }
 
@@ -392,6 +471,10 @@ static void 

[Qemu-devel] [PATCH 1/8] virtio: Remove duplicate macro definition for max. virtqueues, bump up the max

2010-01-14 Thread Amit Shah
VIRTIO_PCI_QUEUE_MAX is redefined in hw/virtio.c. Let's just keep it in
hw/virtio.h.

Also, bump up the value of the maximum allowed virtqueues to 64. This is
in preparation to allow multiple ports per virtio-console device.

Signed-off-by: Amit Shah amit.s...@redhat.com
---
 hw/virtio.c |2 --
 hw/virtio.h |2 +-
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/hw/virtio.c b/hw/virtio.c
index fa7184a..7c020a3 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -75,8 +75,6 @@ struct VirtQueue
 void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
 };
 
-#define VIRTIO_PCI_QUEUE_MAX16
-
 /* virt queue functions */
 static void virtqueue_init(VirtQueue *vq)
 {
diff --git a/hw/virtio.h b/hw/virtio.h
index 3994cc9..7b2b327 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -90,7 +90,7 @@ typedef struct {
 unsigned (*get_features)(void * opaque);
 } VirtIOBindings;
 
-#define VIRTIO_PCI_QUEUE_MAX 16
+#define VIRTIO_PCI_QUEUE_MAX 64
 
 #define VIRTIO_NO_VECTOR 0x
 
-- 
1.6.2.5





[Qemu-devel] ppc softmmu build failure

2010-01-14 Thread Edgar E. Iglesias
I pulled from master today and Can't build ppc-softmmu anymore:

% make   
  LINK  ppcemb-softmmu/qemu-system-ppcemb
ppce500_mpc8544ds.o: In function `mpc8544_copy_soc_cell':
/home/edgar/src/c/qemu/git/qemu/hw/ppce500_mpc8544ds.c:57: undefined reference 
to `kvmppc_read_host_property'
collect2: ld returned 1 exit status
make[1]: *** [qemu-system-ppcemb] Error 1
make: *** [subdir-ppcemb-softmmu] Error 2

Cheers,
Edgar




Re: [Qemu-devel] [...@redhat.com: [PATCHv2 0/3] qemu: memory notifiers]

2010-01-14 Thread Anthony Liguori

On 01/14/2010 03:32 AM, Michael S. Tsirkin wrote:

Avi, Marcelo, if there are no objections, maybe this series can be
merged through the new shiny qemu-kvm upstream branch? Thanks!
   


FWIW, I've been waiting for at least a comment from Avi since that was 
requested in the original post.


Regards,

Anthony Liguori




Re: [Qemu-devel] [RFC][PATCH v2] suppressing queue notification with queue depth parameter (was: [RFC][PATCH] Queue notify support for virtio block device.)

2010-01-14 Thread Anthony Liguori

On 01/14/2010 04:33 AM, Vadim Rozenfeld wrote:

The following patch allows to suppress virtio queue notification with
the number of requests passed as parameter.

Changes from v1:
- code styling,
- parameter x-queue-depth-suppress-notify for queue depth adjustment.

repository: /home/vadimr/work/upstream/qemu
branch: master
commit d23a1c2fbd23e3da9a671a35c95324d2c630e4c9
Author: Vadim Rozenfeldvroze...@redhat.com
Date:   Thu Jan 14 08:09:26 2010 +0200

 [RFC][PATCH v2] suppressing queue notification with queue depth
parameter

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index cb1ae1f..93b584d 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -28,6 +28,8 @@ typedef struct VirtIOBlock
  char serial_str[BLOCK_SERIAL_STRLEN + 1];
  QEMUBH *bh;
  size_t config_size;
+unsigned int queue_depth;
+unsigned int requests;
  } VirtIOBlock;

  static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -87,6 +89,8 @@ typedef struct VirtIOBlockReq
  struct VirtIOBlockReq *next;
  } VirtIOBlockReq;

+static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue
*vq);
+
  static void virtio_blk_req_complete(VirtIOBlockReq *req, int status)
  {
  VirtIOBlock *s = req-dev;
@@ -95,6 +99,11 @@ static void virtio_blk_req_complete(VirtIOBlockReq
*req, int status)
  virtqueue_push(s-vq,req-elem, req-qiov.size +
sizeof(*req-in));
  virtio_notify(s-vdev, s-vq);

+virtio_blk_handle_output(s-vdev, s-vq);
+if (--s-requests == (s-queue_depth - 1)) {
+virtio_queue_set_notification(s-vq, 1);
+}
+
  qemu_free(req);
  }

@@ -340,6 +349,10 @@ static void virtio_blk_handle_output(VirtIODevice
*vdev, VirtQueue *vq)
  exit(1);
  }

+if (++s-requests == s-queue_depth) {
+virtio_queue_set_notification(s-vq, 0);
+}
+
  req-out = (void *)req-elem.out_sg[0].iov_base;
  req-in = (void *)req-elem.in_sg[req-elem.in_num -
1].iov_base;

@@ -502,6 +515,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev,
DriveInfo *dinfo)
  s-vdev.reset = virtio_blk_reset;
  s-bs = dinfo-bdrv;
  s-rq = NULL;
+s-queue_depth = drive_get_queue_depth(dinfo-bdrv);
  if (strlen(ps))
  strncpy(s-serial_str, ps, sizeof(s-serial_str));
  else
diff --git a/qemu-config.c b/qemu-config.c
index c3203c8..6fcf958 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -78,6 +78,10 @@ QemuOptsList qemu_drive_opts = {
  },{
  .name = readonly,
  .type = QEMU_OPT_BOOL,
+},{
+.name = x-queue-depth-suppress-notify,
+.type = QEMU_OPT_NUMBER,
+.help = number of requests in queueu before suppressing
notify,
  },
  { /* end if list */ }
  },
diff --git a/sysemu.h b/sysemu.h
index 9d80bb2..6eecbe1 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -183,6 +183,7 @@ typedef struct DriveInfo {
  BlockInterfaceErrorAction on_read_error;
  BlockInterfaceErrorAction on_write_error;
  char serial[BLOCK_SERIAL_STRLEN + 1];
+int queue_depth;
  QTAILQ_ENTRY(DriveInfo) next;
  } DriveInfo;

@@ -198,6 +199,7 @@ extern DriveInfo *drive_get_by_id(const char *id);
  extern int drive_get_max_bus(BlockInterfaceType type);
  extern void drive_uninit(DriveInfo *dinfo);
  extern const char *drive_get_serial(BlockDriverState *bdrv);
+extern int drive_get_queue_depth(BlockDriverState *bdrv);

  extern BlockInterfaceErrorAction drive_get_on_error(
  BlockDriverState *bdrv, int is_read);
diff --git a/vl.c b/vl.c
index b048e89..517d290 100644
--- a/vl.c
+++ b/vl.c
@@ -2043,6 +2043,18 @@ const char *drive_get_serial(BlockDriverState
*bdrv)
  return \0;
  }

+int drive_get_queue_depth(BlockDriverState *bdrv)
+{
+DriveInfo *dinfo;
+
+QTAILQ_FOREACH(dinfo,drives, next) {
+if (dinfo-bdrv == bdrv)
+return dinfo-queue_depth;
+}
+
+return 1;
+}
+
  BlockInterfaceErrorAction drive_get_on_error(
  BlockDriverState *bdrv, int is_read)
  {
@@ -2110,6 +2122,7 @@ DriveInfo *drive_init(QemuOpts *opts, void
*opaque,
  const char *devaddr;
  DriveInfo *dinfo;
  int snapshot = 0;
+int queue_depth = 1;
   


It should be disabled by default.  queue_depth=1 by default is going to 
be devastating for multi-spindle set-ups.


Regards,

Anthony Liguori




Re: [Qemu-devel] ppc softmmu build failure

2010-01-14 Thread Edgar E. Iglesias
On Thu, Jan 14, 2010 at 02:50:27PM +0100, Edgar E. Iglesias wrote:
 I pulled from master today and Can't build ppc-softmmu anymore:
 
 % make   
   LINK  ppcemb-softmmu/qemu-system-ppcemb
 ppce500_mpc8544ds.o: In function `mpc8544_copy_soc_cell':
 /home/edgar/src/c/qemu/git/qemu/hw/ppce500_mpc8544ds.c:57: undefined 
 reference to `kvmppc_read_host_property'
 collect2: ld returned 1 exit status
 make[1]: *** [qemu-system-ppcemb] Error 1
 make: *** [subdir-ppcemb-softmmu] Error 2


Seems it was not because of the update but because of enabling
'--enable-debug-tcg' '--enable-debug'...

Cheers




Re: [Qemu-devel] [PATCH 0/3] add disk/cdrom version properties

2010-01-14 Thread Anthony Liguori

On 01/14/2010 07:44 AM, Gerd Hoffmann wrote:

   Hi,

This patch series makes the version reported by ide and scsi drives
configurable using qdev properties.  Should help keeping the virtual
hardware more constant on qemu updates, so it becomes less likely
that Windows wants be re-activated.

Maybe 0.12 candidate?
   


Definitely, but is there really three patches?  I only see two.

Regards,

Anthony Liguori


cheers,
   Gerd




   






[Qemu-devel] [PATCH 0/3] Modifications to the drives' readonly attribute

2010-01-14 Thread Naphtali Sprei
Naphtali Sprei (3):
  Make CDROM a read-only drive
  Switch to bit-flags based read-only drive option implementation
  Disable fall-back to read-only when cannot open drive's file for
read-write

 block.c   |   29 +
 block.h   |4 ++--
 block/raw-posix.c |2 +-
 block/raw-win32.c |4 ++--
 block/vmdk.c  |9 +
 hw/xen_disk.c |2 +-
 monitor.c |2 +-
 qemu-img.c|   10 ++
 qemu-io.c |   14 ++
 qemu-nbd.c|2 +-
 qemu-options.hx   |2 +-
 vl.c  |   14 +++---
 12 files changed, 50 insertions(+), 44 deletions(-)





[Qemu-devel] [PATCH 1/3] Make CDROM a read-only drive

2010-01-14 Thread Naphtali Sprei

Signed-off-by: Naphtali Sprei nsp...@redhat.com
---
 vl.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 06cb40d..4f19505 100644
--- a/vl.c
+++ b/vl.c
@@ -2234,6 +2234,10 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque,
 (void)bdrv_set_read_only(dinfo-bdrv, 1);
 }
 
+if (media == MEDIA_CDROM) {
+(void)bdrv_set_read_only(dinfo-bdrv, 1);
+}
+
 if (bdrv_open2(dinfo-bdrv, file, bdrv_flags, drv)  0) {
 fprintf(stderr, qemu: could not open disk image %s: %s\n,
 file, strerror(errno));
-- 
1.6.3.3





[Qemu-devel] [PATCH 3/3] Disable fall-back to read-only when cannot open drive's file for read-write

2010-01-14 Thread Naphtali Sprei

Signed-off-by: Naphtali Sprei nsp...@redhat.com
---
 block.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index cbd72cc..0b4b9ad 100644
--- a/block.c
+++ b/block.c
@@ -457,10 +457,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 ret = -ENOTSUP;
 } else {
 ret = drv-bdrv_open(bs, filename, open_flags);
-if ((ret == -EACCES || ret == -EPERM)  !(flags  BDRV_O_FILE)) {
-ret = drv-bdrv_open(bs, filename, open_flags  ~BDRV_O_RDWR);
-bs-read_only = 1;
-}
 }
 if (ret  0) {
 qemu_free(bs-opaque);
-- 
1.6.3.3





Re: [Qemu-devel] [...@redhat.com: [PATCHv2 0/3] qemu: memory notifiers]

2010-01-14 Thread Avi Kivity

On 01/14/2010 04:27 PM, Anthony Liguori wrote:

On 01/14/2010 03:32 AM, Michael S. Tsirkin wrote:

Avi, Marcelo, if there are no objections, maybe this series can be
merged through the new shiny qemu-kvm upstream branch? Thanks!


FWIW, I've been waiting for at least a comment from Avi since that was 
requested in the original post.


Yeah, sorry, I'm behind due to my fpu patchset being subtly broken.  
Will flush the queue as I think I have things under control now.


--
error compiling committee.c: too many arguments to function





Re: [Qemu-devel] [PATCH 0/8] virtio-console: Move to qdev, multiple devices, generic ports

2010-01-14 Thread Anthony Liguori

On 01/14/2010 07:17 AM, Amit Shah wrote:

Hello people,

This iteration of the series removes the START and END flags (and
hence the header associated with each buffer). That's the major change
since the last submission.
   


I think the biggest issue remaining is the buffering.

I think this is a pretty fundamental issue to work out since it 
determines the very nature of the transport (stream vs. datagram).


Because you have to put a max buffer size on the transport, I think 
buffering is a really flawed approach provably equivalent to just 
increasing the message size within the transport.  In general, the later 
is a better approach because then the guest is using it's memory vs. 
using host memory.


Regards,

Anthony Liguori




Re: [Qemu-devel] [PATCH 0/3] add disk/cdrom version properties

2010-01-14 Thread Gerd Hoffmann

On 01/14/10 15:31, Anthony Liguori wrote:

On 01/14/2010 07:44 AM, Gerd Hoffmann wrote:

Hi,

This patch series makes the version reported by ide and scsi drives
configurable using qdev properties. Should help keeping the virtual
hardware more constant on qemu updates, so it becomes less likely
that Windows wants be re-activated.

Maybe 0.12 candidate?


Definitely, but is there really three patches? I only see two.


Yes.  I've got only 1+2 back via qemu-devel so far, guess the list is a 
bit slow today.


cheers,
  Gerd




[Qemu-devel] Re: Advise on updating SeaBIOS in stable

2010-01-14 Thread Anthony Liguori

On 01/14/2010 07:46 AM, Kevin O'Connor wrote:

On Wed, Jan 13, 2010 at 05:58:35PM -0600, Anthony Liguori wrote:
   

I actually need the compiler fix to build on my laptop (F12) so I've
included that too.  Care to take a look at
git://git.qemu.org/seabios.git stable-0.5.0?  It survives some light
testing and I'll be doing more thorough testing overnight.
 

I'd also include: 2ceeec9d, and 085debd9.  The first fixes a binutils
oddity, and the second is a straight-forward bug fix.

   

If you want to add some more and/or tag a release, I'll resync again
before cutting 0.12.2.

 

If you're looking to pull in 32bit pcibios support, then I don't think
it would be worthwhile to rebase to a stable branch, as the 32bit
pcibios support is easily the biggest user visible change in v0.5.1
(in the sense that Linux will call 32bit pcibios if it's available).
   

Unless there's a strong demand for it, I'd like to hold off on 32bit
pcibios support.
 

That makes sense.  I'll pull your branch into my tree as well.
However, I don't think I'll get time to look much closer at this until
tomorrow night.
   


Based on the importance of 32bit pcibios support, I think it makes sense 
for us to just go to 0.5.1.  Post 0.12.2, I think we'll want to be more 
restrictive but this looks to be an important feature.


Regards,

Anthony Liguori


-Kevin
   






Re: [Qemu-devel] [patch] Fix a typo in 'P' packet processing for M68K.

2010-01-14 Thread Aurelien Jarno
On Wed, Dec 23, 2009 at 04:33:24PM -0800, Kazu Hirata wrote:
 Hi,
 
 Attached is a patch to fix a typo in 'P' packet processing for M68K.
 
 Without this patch, QEMU fails to honor GDB's P packets from GDB
 (writing to registers) for the address registers (A0 - A7).
 
 The problem is because of an obvious typo.  Notice that the second
 if condition is meant to be n  16 in:
 
   if (n  8) {
 :
   } else if (n  8) {
 
 I don't have a write access to the repository.  Could someone apply
 this patch if it's OK?

This patch looks ok, but is missing a Signed-of-by:

 Thanks in advance,
 
 Kazu Hirata
 
 diff --git a/gdbstub.c b/gdbstub.c
 index 055093f..1a1640a 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -1014,7 +1014,7 @@ static int cpu_gdb_write_register(CPUState *env, 
 uint8_t *mem_buf, int n)
  if (n  8) {
  /* D0-D7 */
  env-dregs[n] = tmp;
 -} else if (n  8) {
 +} else if (n  16) {
  /* A0-A7 */
  env-aregs[n - 8] = tmp;
  } else {
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] [PATCH][STABLE] Don't set default monitor when there is a mux'ed one

2010-01-14 Thread Jan Kiszka
This fixes eg. -nographic -serial mon:stdio [-serial ...].

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---

 vl.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index 06cb40d..fa3e8ee 100644
--- a/vl.c
+++ b/vl.c
@@ -5171,6 +5171,9 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_serial:
 add_device_config(DEV_SERIAL, optarg);
 default_serial = 0;
+if (strncmp(optarg, mon:, 4) == 0) {
+default_monitor = 0;
+}
 break;
 case QEMU_OPTION_watchdog:
 if (watchdog) {
@@ -5189,10 +5192,16 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_virtiocon:
 add_device_config(DEV_VIRTCON, optarg);
 default_virtcon = 0;
+if (strncmp(optarg, mon:, 4) == 0) {
+default_monitor = 0;
+}
 break;
 case QEMU_OPTION_parallel:
 add_device_config(DEV_PARALLEL, optarg);
 default_parallel = 0;
+if (strncmp(optarg, mon:, 4) == 0) {
+default_monitor = 0;
+}
 break;
 case QEMU_OPTION_debugcon:
 add_device_config(DEV_DEBUGCON, optarg);




[Qemu-devel] Error message regression

2010-01-14 Thread Aurelien Jarno
Starting with commit 0e8c9214ba1d4128cf92442cd343bc3733478261, when
trying to run make without running configure first, the error message
has been changed from: 

| $ make
| Please call configure before running make!
| make: *** [config-host.mak] Erreur 1

to this more cryptic message:

| $ make
| Makefile:78: /Makefile.objs: No such file or directory
| make: *** No rule to make target `/Makefile.objs'.  Stop.

Any idea how to fix that properly?

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] [PATCH 3/3] pc: add driver version compat properties

2010-01-14 Thread Gerd Hoffmann
This patch adds compat property entries for ide-disk.ver and
scsi-disk.ver to pc-0.10 and pc-0.11.  With this patch applied
the scsi and ide disks report 0.10 and 0.11 as version when
you start qemu with -M pc-0.10 or -M pc-0.11.

Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
 hw/pc.c |   16 
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index a93c5f2..2548c14 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1083,6 +1083,14 @@ static QEMUMachine pc_machine_v0_11 = {
 .property = vectors,
 .value= stringify(0),
 },{
+.driver   = ide-drive,
+.property = ver,
+.value= 0.11,
+},{
+.driver   = scsi-disk,
+.property = ver,
+.value= 0.11,
+},{
 .driver   = PCI,
 .property = rombar,
 .value= stringify(0),
@@ -1114,6 +1122,14 @@ static QEMUMachine pc_machine_v0_10 = {
 .property = vectors,
 .value= stringify(0),
 },{
+.driver   = ide-drive,
+.property = ver,
+.value= 0.10,
+},{
+.driver   = scsi-disk,
+.property = ver,
+.value= 0.10,
+},{
 .driver   = PCI,
 .property = rombar,
 .value= stringify(0),
-- 
1.6.5.2





[Qemu-devel] Re: [PATCH][STABLE] Don't set default monitor when there is a mux'ed one

2010-01-14 Thread Gerd Hoffmann

On 01/14/10 15:49, Jan Kiszka wrote:

This fixes eg. -nographic -serial mon:stdio [-serial ...].


Looks good to me.

Acked-by: Gerd Hoffmann kra...@redhat.com

cheers,
  Gerd





Re: [Qemu-devel] [patch] alpha-linux-user stat64 issue

2010-01-14 Thread Aurelien Jarno
On Tue, Dec 29, 2009 at 12:01:22AM -0500, Vince Weaver wrote:
 Hello
 
 The stat64/fstat64 syscalls are broken for alpha linux-user.
 
 This is because Alpha, even though it is native 64-bits, has a stat64 
 syscall that is different than regular stat.  This means that the
 TARGET_LONG_BITS==64 check in syscall.c isn't enough.  Below is
 a patch that fixes things for me, although it might not be the cleanest 
 fix.
 
 This issue keeps sixtrack and fma3d spec2k benchmarks from running.

Thanks, applied.

 Vince
 
 Signed-off-by: Vince Weaver vi...@csl.cornell.edu
 
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 1acf1f5..f2dd39e 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -4004,7 +4004,7 @@ static inline abi_long host_to_target_stat64(void 
 *cpu_env,
  } else
  #endif
  {
 -#if TARGET_LONG_BITS == 64
 +#if (TARGET_LONG_BITS == 64)  (!defined(TARGET_ALPHA))
  struct target_stat *target_st;
  #else
  struct target_stat64 *target_st;
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] [PATCH] linux-user: adapt uname machine to emulated CPU

2010-01-14 Thread Aurelien Jarno
On Tue, Dec 29, 2009 at 10:39:21PM +0100, Loïc Minier wrote:
 Hey there
 
  This patch for linux-user adapts the output of the emulated uname()
  syscall to match the configured CPU.  Tested with x86, x86-64 and arm
  emulation.

This patch look ok, but is missing a Signed-off-by:

Aurelien

Thanks,
 
 
 ---
  Makefile.target|2 +-
  linux-user/cpu-uname.c |   72 
 
  linux-user/cpu-uname.h |1 +
  linux-user/syscall.c   |3 +-
  4 files changed, 76 insertions(+), 2 deletions(-)
  create mode 100644 linux-user/cpu-uname.c
  create mode 100644 linux-user/cpu-uname.h
 
 diff --git a/Makefile.target b/Makefile.target
 index 7c1f30c..6414472 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -90,7 +90,7 @@ ifdef CONFIG_LINUX_USER
  VPATH+=:$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
  QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user 
 -I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
  obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
 -  elfload.o linuxload.o uaccess.o gdbstub.o
 +  elfload.o linuxload.o uaccess.o gdbstub.o cpu-uname.o
  
  obj-$(TARGET_HAS_BFLT) += flatload.o
  obj-$(TARGET_HAS_ELFLOAD32) += elfload32.o
 diff --git a/linux-user/cpu-uname.c b/linux-user/cpu-uname.c
 new file mode 100644
 index 000..ddc37be
 --- /dev/null
 +++ b/linux-user/cpu-uname.c
 @@ -0,0 +1,72 @@
 +/*
 + *  cpu to uname machine name map
 + *
 + *  Copyright (c) 2009 Loïc Minier
 + *
 + *  This program is free software; you can redistribute it and/or modify
 + *  it under the terms of the GNU General Public License as published by
 + *  the Free Software Foundation; either version 2 of the License, or
 + *  (at your option) any later version.
 + *
 + *  This program is distributed in the hope that it will be useful,
 + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
 + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + *  GNU General Public License for more details.
 + *
 + *  You should have received a copy of the GNU General Public License
 + *  along with this program; if not, see http://www.gnu.org/licenses/.
 + */
 +
 +#include stdio.h
 +
 +#include qemu.h
 +//#include qemu-common.h
 +#include cpu-uname.h
 +
 +/* return highest utsname machine name for emulated instruction set
 + *
 + * NB: the default emulated CPU (any) might not match any existing CPU, 
 e.g.
 + * on ARM it has all features turned on, so there is no perfect arch string 
 to
 + * return here */
 +const char *cpu_to_uname_machine(void *cpu_env)
 +{
 +#ifdef TARGET_ARM
 +/* utsname machine name on linux arm is CPU arch name + endianness, e.g.
 + * armv7l; to get a list of CPU arch names from the linux source, use:
 + * grep arch_name: -A1 linux/arch/arm/mm/proc-*.S
 + * see arch/arm/kernel/setup.c: setup_processor()
 + *
 + * to test by CPU id, compare cpu_env-cp15.c0_cpuid to ARM_CPUID_*
 + * defines and to test by CPU feature, use arm_feature(cpu_env,
 + * ARM_FEATURE_*) */
 +
 +/* in theory, endianness is configurable on some ARM CPUs, but this isn't
 + * used in user mode emulation */
 +#ifdef TARGET_WORDS_BIGENDIAN
 +#define utsname_suffix b
 +#else
 +#define utsname_suffix l
 +#endif
 +if (arm_feature(cpu_env, ARM_FEATURE_V7))
 +return armv7 utsname_suffix;
 +if (arm_feature(cpu_env, ARM_FEATURE_V6))
 +return armv6 utsname_suffix;
 +/* earliest emulated CPU is ARMv5TE; qemu can emulate the 1026, but not 
 its
 + * Jazelle support */
 +return armv5te utsname_suffix;
 +#elif defined(TARGET_X86_64)
 +return x86-64;
 +#elif defined(TARGET_I386)
 +/* see arch/x86/kernel/cpu/bugs.c: check_bugs(), 386, 486, 586, 686 */
 +uint32_t cpuid_version = ((CPUX86State *)cpu_env)-cpuid_version;
 +int family = ((cpuid_version  8)  0x0f) + ((cpuid_version  20)  
 0xff);
 +if (family == 4)
 +return i486;
 +if (family == 5)
 +return i586;
 +return i686;
 +#else
 +/* default is #define-d in each arch/ subdir */
 +return UNAME_MACHINE;
 +#endif
 +}
 diff --git a/linux-user/cpu-uname.h b/linux-user/cpu-uname.h
 new file mode 100644
 index 000..32492de
 --- /dev/null
 +++ b/linux-user/cpu-uname.h
 @@ -0,0 +1 @@
 +const char *cpu_to_uname_machine(void *cpu_env);
 diff --git a/linux-user/syscall.c b/linux-user/syscall.c
 index 1acf1f5..9e2e89a 100644
 --- a/linux-user/syscall.c
 +++ b/linux-user/syscall.c
 @@ -82,6 +82,7 @@
  #include linux/fb.h
  #include linux/vt.h
  #include linux_loop.h
 +#include cpu-uname.h
  
  #include qemu.h
  #include qemu-common.h
 @@ -5739,7 +5740,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
 arg1,
  if (!is_error(ret)) {
  /* Overrite the native machine name with whatever is being
 emulated. */
 -strcpy (buf-machine, UNAME_MACHINE);
 +strcpy (buf-machine, 

Re: [Qemu-devel] [PATCH][STABLE] Don't set default monitor when there is a mux'ed one

2010-01-14 Thread Anthony Liguori

On 01/14/2010 08:49 AM, Jan Kiszka wrote:

This fixes eg. -nographic -serial mon:stdio [-serial ...].
   


Is this really an appropriate invocation though?

-nographic != mon:stdio so the semantics of how this is supposed to 
behave is at best ill-defined.


Regards,

Anthony Liguori




Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations

2010-01-14 Thread Aurelien Jarno
On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
 The recent transition to always have the DCR helper functions take 32 bit
 values broke the PPC64 target, as tlong became 64 bits there.
 
 This patch moves all translate.c callers to a _tl function that simply
 calls the uint32_t functions. That way we don't need to mess with TCG
 trying to pass registers as uint32_t variables to functions.
 
 Fixes PPC64 build with --enable-debug-tcg
 
 Signed-off-by: Alexander Graf ag...@suse.de
 Reported-by: Stefan Weil w...@mail.berlios.de
 ---
  target-ppc/cpu.h   |2 ++
  target-ppc/helper.h|4 ++--
  target-ppc/op_helper.c |   10 ++
  target-ppc/translate.c |   12 ++--
  4 files changed, 20 insertions(+), 8 deletions(-)
 
 diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
 index d15bba1..60a8b68 100644
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, 
 target_ulong rs);
  void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
  #endif /* !defined(CONFIG_USER_ONLY) */
  void ppc_store_msr (CPUPPCState *env, target_ulong value);
 +void helper_store_dcr (uint32_t dcrn, uint32_t val);
 +uint32_t helper_load_dcr (uint32_t dcrn);
  
  void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
 ...));
  
 diff --git a/target-ppc/helper.h b/target-ppc/helper.h
 index 40d4ced..86f0af7 100644
 --- a/target-ppc/helper.h
 +++ b/target-ppc/helper.h
 @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
  DEF_HELPER_2(divs, tl, tl, tl)
  DEF_HELPER_2(divso, tl, tl, tl)
  
 -DEF_HELPER_1(load_dcr, i32, i32);
 -DEF_HELPER_2(store_dcr, void, i32, i32)
 +DEF_HELPER_1(load_dcr_tl, tl, tl);
 +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
  
  DEF_HELPER_1(load_dump_spr, void, i32)
  DEF_HELPER_1(store_dump_spr, void, i32)
 diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
 index cea27f2..6c375d3 100644
 --- a/target-ppc/op_helper.c
 +++ b/target-ppc/op_helper.c
 @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
  return val;
  }
  
 +target_ulong helper_load_dcr_tl (target_ulong dcrn)
 +{
 +return (uint32_t)helper_load_dcr((uint32_t)dcrn);
 +}
 +
  void helper_store_dcr (uint32_t dcrn, uint32_t val)
  {
  if (unlikely(env-dcr_env == NULL)) {
 @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
  }
  }
  
 +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
 +{
 +helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
 +}
 +

I do wonder why we need to keep the old helper_load_dcr() and
helper_store_dcr() instead of modifying them directly. They doesn't seems
to be used elsewhere.

  #if !defined(CONFIG_USER_ONLY)
  void helper_40x_rfci (void)
  {
 diff --git a/target-ppc/translate.c b/target-ppc/translate.c
 index d4e81ce..d83d196 100644
 --- a/target-ppc/translate.c
 +++ b/target-ppc/translate.c
 @@ -5565,7 +5565,7 @@ static void gen_mfdcr(DisasContext *ctx)
  /* NIP cannot be restored if the memory exception comes from an helper */
  gen_update_nip(ctx, ctx-nip - 4);
  dcrn = tcg_const_tl(SPR(ctx-opcode));
 -gen_helper_load_dcr(cpu_gpr[rD(ctx-opcode)], dcrn);
 +gen_helper_load_dcr_tl(cpu_gpr[rD(ctx-opcode)], dcrn);
  tcg_temp_free(dcrn);
  #endif
  }
 @@ -5584,7 +5584,7 @@ static void gen_mtdcr(DisasContext *ctx)
  /* NIP cannot be restored if the memory exception comes from an helper */
  gen_update_nip(ctx, ctx-nip - 4);
  dcrn = tcg_const_tl(SPR(ctx-opcode));
 -gen_helper_store_dcr(dcrn, cpu_gpr[rS(ctx-opcode)]);
 +gen_helper_store_dcr_tl(dcrn, cpu_gpr[rS(ctx-opcode)]);
  tcg_temp_free(dcrn);
  #endif
  }
 @@ -5602,7 +5602,7 @@ static void gen_mfdcrx(DisasContext *ctx)
  }
  /* NIP cannot be restored if the memory exception comes from an helper */
  gen_update_nip(ctx, ctx-nip - 4);
 -gen_helper_load_dcr(cpu_gpr[rD(ctx-opcode)], cpu_gpr[rA(ctx-opcode)]);
 +gen_helper_load_dcr_tl(cpu_gpr[rD(ctx-opcode)], 
 cpu_gpr[rA(ctx-opcode)]);
  /* Note: Rc update flag set leads to undefined state of Rc0 */
  #endif
  }
 @@ -5620,7 +5620,7 @@ static void gen_mtdcrx(DisasContext *ctx)
  }
  /* NIP cannot be restored if the memory exception comes from an helper */
  gen_update_nip(ctx, ctx-nip - 4);
 -gen_helper_store_dcr(cpu_gpr[rA(ctx-opcode)], cpu_gpr[rS(ctx-opcode)]);
 +gen_helper_store_dcr_tl(cpu_gpr[rA(ctx-opcode)], 
 cpu_gpr[rS(ctx-opcode)]);
  /* Note: Rc update flag set leads to undefined state of Rc0 */
  #endif
  }
 @@ -5630,7 +5630,7 @@ static void gen_mfdcrux(DisasContext *ctx)
  {
  /* NIP cannot be restored if the memory exception comes from an helper */
  gen_update_nip(ctx, ctx-nip - 4);
 -gen_helper_load_dcr(cpu_gpr[rD(ctx-opcode)], cpu_gpr[rA(ctx-opcode)]);
 +gen_helper_load_dcr_tl(cpu_gpr[rD(ctx-opcode)], 
 cpu_gpr[rA(ctx-opcode)]);
  /* Note: Rc update flag set leads to undefined state of Rc0 */
  }
  

Re: [Qemu-devel] [PATCH] sh: sm501: Add hardware cursor feature

2010-01-14 Thread Aurelien Jarno
On Fri, Jan 01, 2010 at 03:59:39PM +0900, Shin-ichiro KAWASAKI wrote:
 This patch adds hardware cursor feature to SM501 graphics chip emulation,
 to make the graphic console more useful for QEMU SH4 users.
 
 
 Signed-off-by: Shin-ichiro KAWASAKI kawas...@juno.dti.ne.jp

Thanks, applied.

  hw/sm501.c  |  154 
 +++
  hw/sm501_template.h |   42 ++
  2 files changed, 185 insertions(+), 11 deletions(-)
 
 diff --git a/hw/sm501.c b/hw/sm501.c
 index 612a8e5..cd1f595 100644
 --- a/hw/sm501.c
 +++ b/hw/sm501.c
 @@ -434,6 +434,8 @@
  
  /* end of register definitions */
  
 +#define SM501_HWC_WIDTH   (64)
 +#define SM501_HWC_HEIGHT  (64)
  
  /* SM501 local memory size taken from linux/drivers/mfd/sm501.c */
  static const uint32_t sm501_mem_local_size[] = {
 @@ -526,6 +528,95 @@ static uint32_t get_local_mem_size_index(uint32_t size)
  return index;
  }
  
 +/**
 + * Check the availability of hardware cursor.
 + * @param crt  0 for PANEL, 1 for CRT.
 + */
 +static inline int is_hwc_enabled(SM501State *state, int crt)
 +{
 +uint32_t addr = crt ? state-dc_crt_hwc_addr : state-dc_panel_hwc_addr;
 +return addr  0x8000;
 +}
 +
 +/**
 + * Get the address which holds cursor pattern data.
 + * @param crt  0 for PANEL, 1 for CRT.
 + */
 +static inline uint32_t get_hwc_address(SM501State *state, int crt)
 +{
 +uint32_t addr = crt ? state-dc_crt_hwc_addr : state-dc_panel_hwc_addr;
 +return (addr  0x03F0)/*  4*/;
 +}
 +
 +/**
 + * Get the cursor position in y coordinate.
 + * @param crt  0 for PANEL, 1 for CRT.
 + */
 +static inline uint32_t get_hwc_y(SM501State *state, int crt)
 +{
 +uint32_t location = crt ? state-dc_crt_hwc_location
 +: state-dc_panel_hwc_location;
 +return (location  0x07FF)  16;
 +}
 +
 +/**
 + * Get the cursor position in x coordinate.
 + * @param crt  0 for PANEL, 1 for CRT.
 + */
 +static inline uint32_t get_hwc_x(SM501State *state, int crt)
 +{
 +uint32_t location = crt ? state-dc_crt_hwc_location
 +: state-dc_panel_hwc_location;
 +return location  0x07FF;
 +}
 +
 +/**
 + * Get the cursor position in x coordinate.
 + * @param crt  0 for PANEL, 1 for CRT.
 + * @param index  0, 1, 2 or 3 which specifies color of corsor dot.
 + */
 +static inline uint16_t get_hwc_color(SM501State *state, int crt, int index)
 +{
 +uint16_t color_reg = 0;
 +uint16_t color_565 = 0;
 +
 +if (index == 0) {
 +return 0;
 +}
 +
 +switch (index) {
 +case 1:
 +case 2:
 +color_reg = crt ? state-dc_crt_hwc_color_1_2
 +: state-dc_panel_hwc_color_1_2;
 +break;
 +case 3:
 +color_reg = crt ? state-dc_crt_hwc_color_3
 +: state-dc_panel_hwc_color_3;
 +break;
 +default:
 +printf(invalid hw cursor color.\n);
 +assert(0);
 +}
 +
 +switch (index) {
 +case 1:
 +case 3:
 +color_565 = (uint16_t)(color_reg  0x);
 +break;
 +case 2:
 +color_565 = (uint16_t)((color_reg  16)  0x);
 +break;
 +}
 +return color_565;
 +}
 +
 +static int within_hwc_y_range(SM501State *state, int y, int crt)
 +{
 +int hwc_y = get_hwc_y(state, crt);
 +return (hwc_y = y  y  hwc_y + SM501_HWC_HEIGHT);
 +}
 +
  static uint32_t sm501_system_config_read(void *opaque, target_phys_addr_t 
 addr)
  {
  SM501State * s = (SM501State *)opaque;
 @@ -736,13 +827,13 @@ static uint32_t sm501_disp_ctrl_read(void *opaque, 
 target_phys_addr_t addr)
   ret = s-dc_crt_hwc_addr;
   break;
  case SM501_DC_CRT_HWC_LOC:
 - ret = s-dc_crt_hwc_addr;
 + ret = s-dc_crt_hwc_location;
   break;
  case SM501_DC_CRT_HWC_COLOR_1_2:
 - ret = s-dc_crt_hwc_addr;
 + ret = s-dc_crt_hwc_color_1_2;
   break;
  case SM501_DC_CRT_HWC_COLOR_3:
 - ret = s-dc_crt_hwc_addr;
 + ret = s-dc_crt_hwc_color_3;
   break;
  
  case SM501_DC_PANEL_PALETTE ... SM501_DC_PANEL_PALETTE + 0x400*3 - 4:
 @@ -809,13 +900,13 @@ static void sm501_disp_ctrl_write(void *opaque,
   s-dc_panel_hwc_addr = value  0x8FF0;
   break;
  case SM501_DC_PANEL_HWC_LOC:
 - s-dc_panel_hwc_addr = value  0x0FFF0FFF;
 + s-dc_panel_hwc_location = value  0x0FFF0FFF;
   break;
  case SM501_DC_PANEL_HWC_COLOR_1_2:
 - s-dc_panel_hwc_addr = value;
 + s-dc_panel_hwc_color_1_2 = value;
   break;
  case SM501_DC_PANEL_HWC_COLOR_3:
 - s-dc_panel_hwc_addr = value  0x;
 + s-dc_panel_hwc_color_3 = value  0x;
   break;
  
  case SM501_DC_CRT_CONTROL:
 @@ -844,13 +935,13 @@ static void sm501_disp_ctrl_write(void *opaque,
   s-dc_crt_hwc_addr = value  0x8FF0;
   break;
  case SM501_DC_CRT_HWC_LOC:
 - s-dc_crt_hwc_addr = value  0x0FFF0FFF;
 + s-dc_crt_hwc_location = value  

Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations

2010-01-14 Thread Alexander Graf

On 14.01.2010, at 16:13, Aurelien Jarno wrote:

 On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
 The recent transition to always have the DCR helper functions take 32 bit
 values broke the PPC64 target, as tlong became 64 bits there.
 
 This patch moves all translate.c callers to a _tl function that simply
 calls the uint32_t functions. That way we don't need to mess with TCG
 trying to pass registers as uint32_t variables to functions.
 
 Fixes PPC64 build with --enable-debug-tcg
 
 Signed-off-by: Alexander Graf ag...@suse.de
 Reported-by: Stefan Weil w...@mail.berlios.de
 ---
 target-ppc/cpu.h   |2 ++
 target-ppc/helper.h|4 ++--
 target-ppc/op_helper.c |   10 ++
 target-ppc/translate.c |   12 ++--
 4 files changed, 20 insertions(+), 8 deletions(-)
 
 diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
 index d15bba1..60a8b68 100644
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, 
 target_ulong rs);
 void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
 #endif /* !defined(CONFIG_USER_ONLY) */
 void ppc_store_msr (CPUPPCState *env, target_ulong value);
 +void helper_store_dcr (uint32_t dcrn, uint32_t val);
 +uint32_t helper_load_dcr (uint32_t dcrn);
 
 void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
 ...));
 
 diff --git a/target-ppc/helper.h b/target-ppc/helper.h
 index 40d4ced..86f0af7 100644
 --- a/target-ppc/helper.h
 +++ b/target-ppc/helper.h
 @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
 DEF_HELPER_2(divs, tl, tl, tl)
 DEF_HELPER_2(divso, tl, tl, tl)
 
 -DEF_HELPER_1(load_dcr, i32, i32);
 -DEF_HELPER_2(store_dcr, void, i32, i32)
 +DEF_HELPER_1(load_dcr_tl, tl, tl);
 +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
 
 DEF_HELPER_1(load_dump_spr, void, i32)
 DEF_HELPER_1(store_dump_spr, void, i32)
 diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
 index cea27f2..6c375d3 100644
 --- a/target-ppc/op_helper.c
 +++ b/target-ppc/op_helper.c
 @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
 return val;
 }
 
 +target_ulong helper_load_dcr_tl (target_ulong dcrn)
 +{
 +return (uint32_t)helper_load_dcr((uint32_t)dcrn);
 +}
 +
 void helper_store_dcr (uint32_t dcrn, uint32_t val)
 {
 if (unlikely(env-dcr_env == NULL)) {
 @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
 }
 }
 
 +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
 +{
 +helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
 +}
 +
 
 I do wonder why we need to keep the old helper_load_dcr() and
 helper_store_dcr() instead of modifying them directly. They doesn't seems
 to be used elsewhere.

Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny 
preprocessor or function callback logic. But maybe I'm wrong :).


Alex



Re: [Qemu-devel] [PATCH][STABLE] Don't set default monitor when there is a mux'ed one

2010-01-14 Thread Jan Kiszka
Anthony Liguori wrote:
 On 01/14/2010 08:49 AM, Jan Kiszka wrote:
 This fixes eg. -nographic -serial mon:stdio [-serial ...].

 
 Is this really an appropriate invocation though?
 
 -nographic != mon:stdio so the semantics of how this is supposed to 
 behave is at best ill-defined.

Original -nographic implied mon:stdio unless you specified your own
-serial. If this was reasonable or not, changing behavior now breaks
tons of scripts.

Actually, there is more legacy breakage in the new default handling. The
missing translation of -serial stdio -monitor stdio = -serial
mon:stdio can already be harmful to some setups, though I guess they
are less common.

However, this patch addresses something that is a bug outside the scope
of -nographic's semantic.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux




[Qemu-devel] [PATCH] char: Remove redundant qemu_chr_generic_open() call.

2010-01-14 Thread Kusanagi Kouichi
qemu_chr_open_fd() calls qemu_chr_generic_open(),
so qemu_chr_open_tty() doesn't need to call it.

Signed-off-by: Kusanagi Kouichi sl...@ac.auone-net.jp
---
 qemu-char.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index b13f8d4..800ee6c 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -1187,7 +1187,6 @@ static CharDriverState *qemu_chr_open_tty(QemuOpts *opts)
 return NULL;
 }
 chr-chr_ioctl = tty_serial_ioctl;
-qemu_chr_generic_open(chr);
 return chr;
 }
 #else  /* ! __linux__  ! __sun__ */
-- 
1.6.6





Re: [Qemu-devel] [PATCH 1/2] tcg-x86_64: Special-case all 32-bit AND operands.

2010-01-14 Thread Aurelien Jarno
On Tue, Jan 05, 2010 at 04:03:00PM -0800, Richard Henderson wrote:
 This avoids an unnecessary REX.W prefix when dealing with AND
 operands that fit into a 32-bit quantity.  The most common change
 actually seen is movz[wb]q - movz[wb]l.
 
 Similarly, avoid REXW in ext{8,16}u_i64 tcg opcodes.
 
 Signed-off-by: Richard Henderson r...@twiddle.net
 ---
  tcg/x86_64/tcg-target.c |   26 --
  1 files changed, 8 insertions(+), 18 deletions(-)
 
 diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
 index 2339091..f584c94 100644
 --- a/tcg/x86_64/tcg-target.c
 +++ b/tcg/x86_64/tcg-target.c
 @@ -426,24 +426,18 @@ static inline void tgen_arithi64(TCGContext *s, int c, 
 int r0, int64_t val)
  } else if ((c == ARITH_ADD  val == -1) || (c == ARITH_SUB  val == 
 1)) {
  /* dec */
  tcg_out_modrm(s, 0xff | P_REXW, 1, r0);
 -} else if (val == (int8_t)val) {
 -tcg_out_modrm(s, 0x83 | P_REXW, c, r0);
 -tcg_out8(s, val);
 -} else if (c == ARITH_AND  val == 0xffu) {
 -/* movzbl */
 -tcg_out_modrm(s, 0xb6 | P_EXT | P_REXW, r0, r0);
 -} else if (c == ARITH_AND  val == 0xu) {
 -/* movzwl */
 -tcg_out_modrm(s, 0xb7 | P_EXT | P_REXW, r0, r0);
  } else if (c == ARITH_AND  val == 0xu) {
  /* 32-bit mov zero extends */
  tcg_out_modrm(s, 0x8b, r0, r0);
 +} else if (c == ARITH_AND  (uint64_t)val = 0xu) {
 +/* AND with no high bits set can use a 32-bit operation.  */
 +tgen_arithi32(s, c, r0, val);

Do we really want to call tgen_arithi32() here, that will redo part of
the above tests again? It might be better to simply remove the REX.W
prefix above instead.

 +} else if (val == (int8_t)val) {
 +tcg_out_modrm(s, 0x83 | P_REXW, c, r0);
 +tcg_out8(s, val);
  } else if (val == (int32_t)val) {
  tcg_out_modrm(s, 0x81 | P_REXW, c, r0);
  tcg_out32(s, val);
 -} else if (c == ARITH_AND  val == (uint32_t)val) {
 -tcg_out_modrm(s, 0x81, c, r0);
 -tcg_out32(s, val);
  } else {
  tcg_abort();
  }
 @@ -1182,16 +1176,12 @@ static inline void tcg_out_op(TCGContext *s, int opc, 
 const TCGArg *args,
  tcg_out_modrm(s, 0x63 | P_REXW, args[0], args[1]);
  break;
  case INDEX_op_ext8u_i32:
 +case INDEX_op_ext8u_i64:
  tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, args[0], args[1]);
  break;
  case INDEX_op_ext16u_i32:
 -tcg_out_modrm(s, 0xb7 | P_EXT, args[0], args[1]);
 -break;
 -case INDEX_op_ext8u_i64:
 -tcg_out_modrm(s, 0xb6 | P_EXT | P_REXW, args[0], args[1]);
 -break;
  case INDEX_op_ext16u_i64:
 -tcg_out_modrm(s, 0xb7 | P_EXT | P_REXW, args[0], args[1]);
 +tcg_out_modrm(s, 0xb7 | P_EXT, args[0], args[1]);
  break;
  case INDEX_op_ext32u_i64:
  tcg_out_modrm(s, 0x8b, args[0], args[1]);

This part looks fine.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] [PATCH 1/2] tcg-x86_64: Special-case all 32-bit AND operands.

2010-01-14 Thread Richard Henderson

On 01/14/2010 07:57 AM, Aurelien Jarno wrote:

On Tue, Jan 05, 2010 at 04:03:00PM -0800, Richard Henderson wrote:

This avoids an unnecessary REX.W prefix when dealing with AND
operands that fit into a 32-bit quantity.  The most common change
actually seen is movz[wb]q -  movz[wb]l.

Similarly, avoid REXW in ext{8,16}u_i64 tcg opcodes.

Signed-off-by: Richard Hendersonr...@twiddle.net
---
  tcg/x86_64/tcg-target.c |   26 --
  1 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
index 2339091..f584c94 100644
--- a/tcg/x86_64/tcg-target.c
+++ b/tcg/x86_64/tcg-target.c
@@ -426,24 +426,18 @@ static inline void tgen_arithi64(TCGContext *s, int c, 
int r0, int64_t val)
  } else if ((c == ARITH_ADD  val == -1) || (c == ARITH_SUB  val == 1)) 
{
  /* dec */
  tcg_out_modrm(s, 0xff | P_REXW, 1, r0);
-} else if (val == (int8_t)val) {
-tcg_out_modrm(s, 0x83 | P_REXW, c, r0);
-tcg_out8(s, val);
-} else if (c == ARITH_AND  val == 0xffu) {
-/* movzbl */
-tcg_out_modrm(s, 0xb6 | P_EXT | P_REXW, r0, r0);
-} else if (c == ARITH_AND  val == 0xu) {
-/* movzwl */
-tcg_out_modrm(s, 0xb7 | P_EXT | P_REXW, r0, r0);
  } else if (c == ARITH_AND  val == 0xu) {
  /* 32-bit mov zero extends */
  tcg_out_modrm(s, 0x8b, r0, r0);
+} else if (c == ARITH_AND  (uint64_t)val= 0xu) {
+/* AND with no high bits set can use a 32-bit operation.  */
+tgen_arithi32(s, c, r0, val);


Do we really want to call tgen_arithi32() here, that will redo part of
the above tests again? It might be better to simply remove the REX.W
prefix above instead.


Pardon?  Do you mean the inc/dec tests?  Otherwise I don't see what 
above tests again you're talking about.


I am looking to handle more than 0xff, 0x with the new test in 
gen_arithi64 -- 0x1234 is an appropriate mask to shorten to 32-bit AND 
as well.  I have no idea if that answers your question.



r~




Re: [Qemu-devel] [PATCH 2/2] tcg-x86_64: Avoid unnecessary REX.B prefixes.

2010-01-14 Thread Aurelien Jarno
On Tue, Jan 05, 2010 at 04:31:11PM -0800, Richard Henderson wrote:
 A while ago Laurent pointed out that the setcc opcode emitted by
 the setcond patch had unnecessary REX prefixes.
 
 The existing P_REXB internal opcode flag unconditionally emits
 the REX prefix.  Technically it's not needed if the register in
 question is %al, %bl, %cl, %dl.
 
 Eliding the prefix requires splitting the P_REXB flag into two,
 in order to indicate whether the byte register in question is
 in the REG or the R/M field.  Within TCG, the byte register is
 in the REG field only for stores.
 
 Signed-off-by: Richard Henderson r...@twiddle.net
 ---
  tcg/x86_64/tcg-target.c |   46 ++
  1 files changed, 30 insertions(+), 16 deletions(-)
 
 diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
 index f584c94..8b6b68c 100644
 --- a/tcg/x86_64/tcg-target.c
 +++ b/tcg/x86_64/tcg-target.c
 @@ -217,9 +217,10 @@ static inline int tcg_target_const_match(tcg_target_long 
 val,
  #define JCC_JLE 0xe
  #define JCC_JG  0xf
  
 -#define P_EXT   0x100 /* 0x0f opcode prefix */
 -#define P_REXW  0x200 /* set rex.w = 1 */
 -#define P_REXB  0x400 /* force rex use for byte registers */
 +#define P_EXT0x100   /* 0x0f opcode prefix */
 +#define P_REXW   0x200   /* set rex.w = 1 */
 +#define P_REXB_R 0x400   /* REG field as byte register */
 +#define P_REXB_RM0x800   /* R/M field as byte register */

  static const uint8_t tcg_cond_to_jcc[10] = {
  [TCG_COND_EQ] = JCC_JE,
 @@ -234,17 +235,30 @@ static const uint8_t tcg_cond_to_jcc[10] = {
  [TCG_COND_GTU] = JCC_JA,
  };
  
 -static inline void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
 +static void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
  {
 -int rex;
 -rex = ((opc  6)  0x8) | ((r  1)  0x4) | 
 -((x  2)  2) | ((rm  3)  1);
 -if (rex || (opc  P_REXB)) {
 +int rex = 0;
 +
 +rex |= (opc  P_REXW)  6;  /* REX.W */
 +rex |= (r  8)  1; /* REX.R */
 +rex |= (x  8)  2; /* REX.X */
 +rex |= (rm  8)  3;/* REX.B */
 +
 +/* P_REXB_{R,RM} indicates that the given register is the low byte.
 +   For %[abcd]l we need no REX prefix, but for %{si,di,bp,sp}l we do,
 +   as otherwise the encoding indicates %[abcd]h.  Note that the values
 +   that are ORed in merely indicate that the REX byte must be present;
 +   those bits get discarded in output.  */
 +rex |= (r = 4 ? opc  P_REXB_R : 0);
 +rex |= (rm = 4 ? opc  P_REXB_RM : 0);
 +
 +if (rex) {
  tcg_out8(s, rex | 0x40);
  }

With the above change, rex can be  0xff. Not sure it's not a good idea
to not have an explicit cast when calling tcg_out8(), even if it 
technically works.

 -if (opc  P_EXT)
 +if (opc  P_EXT) {
  tcg_out8(s, 0x0f);
 -tcg_out8(s, opc  0xff);
 +}
 +tcg_out8(s, opc);

What's the reason for removing the ' 0xff' part? tcg_out8() takes an uint8_t.

  }
  
  static inline void tcg_out_modrm(TCGContext *s, int opc, int r, int rm)
 @@ -408,7 +422,7 @@ static inline void tgen_arithi32(TCGContext *s, int c, 
 int r0, int32_t val)
  tcg_out8(s, val);
  } else if (c == ARITH_AND  val == 0xffu) {
  /* movzbl */
 -tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, r0, r0);
 +tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB_RM, r0, r0);
  } else if (c == ARITH_AND  val == 0xu) {
  /* movzwl */
  tcg_out_modrm(s, 0xb7 | P_EXT, r0, r0);
 @@ -776,7 +790,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
 *args,
  switch(opc) {
  case 0:
  /* movzbl */
 -tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, TCG_REG_RSI, data_reg);
 +tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB_RM, TCG_REG_RSI, data_reg);
  break;
  case 1:
  /* movzwl */
 @@ -829,7 +843,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
 *args,
  switch(opc) {
  case 0:
  /* movb */
 -tcg_out_modrm_offset(s, 0x88 | P_REXB, data_reg, r0, offset);
 +tcg_out_modrm_offset(s, 0x88 | P_REXB_R, data_reg, r0, offset);
  break;
  case 1:
  if (bswap) {
 @@ -964,7 +978,7 @@ static inline void tcg_out_op(TCGContext *s, int opc, 
 const TCGArg *args,
  case INDEX_op_st8_i32:
  case INDEX_op_st8_i64:
  /* movb */
 -tcg_out_modrm_offset(s, 0x88 | P_REXB, args[0], args[1], args[2]);
 +tcg_out_modrm_offset(s, 0x88 | P_REXB_R, args[0], args[1], args[2]);
  break;
  case INDEX_op_st16_i32:
  case INDEX_op_st16_i64:
 @@ -1161,7 +1175,7 @@ static inline void tcg_out_op(TCGContext *s, int opc, 
 const TCGArg *args,
  break;
  
  case INDEX_op_ext8s_i32:
 -tcg_out_modrm(s, 0xbe | P_EXT | P_REXB, args[0], args[1]);
 +tcg_out_modrm(s, 0xbe | P_EXT | 

Re: [Qemu-devel] [PATCH v3] raw-posix: Detect CDROM via ioctl

2010-01-14 Thread Cole Robinson
On 01/14/2010 05:45 AM, malc wrote:
 On Wed, 13 Jan 2010, Cole Robinson wrote:
 
 On 01/13/2010 07:11 PM, malc wrote:
 On Wed, 13 Jan 2010, Cole Robinson wrote:

 Current CDROM detection is hardcoded based on source file name.
 Make this smarter by attempting a CDROM specific ioctl.

 This makes '-cdrom /dev/sr0' succeed with no media present.

 v2:
 Give ioctl check higher priority than filename check,

 v3:
 Actually initialize 'prio' variable
 Check for ioctl success rather than absence of specific failure

 Does it even compile on BSDs, Darwin etc?


 The changed functions are all under #ifdef __linux__, so I assume its fine.
 Haven't tested though.
 
 Fine if so, it's not immediatelly aparent from the patches nor from
 the commit messages though.
 

True. I'll resend with a better commit message.

Thanks,
Cole




[Qemu-devel] [PATCH v4] raw-posix: Detect legacy floppy via ioctl on linux

2010-01-14 Thread Cole Robinson
Current legacy floppy detection is hardcoded based on source file
name. Make this smarter on linux by attempting a floppy specific
ioctl.

v2:
Give ioctl check higher priority than filename check
s/IDE/legacy/

v3:
Actually initialize 'prio' variable
Check for ioctl success rather than absence of specific failure

v4:
Explicitly mention that change is linux specific.

Signed-off-by: Cole Robinson crobi...@redhat.com
---
 block/raw-posix.c |   21 +++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index a2c7508..eea7e56 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1055,9 +1055,26 @@ static int floppy_open(BlockDriverState *bs, const char 
*filename, int flags)
 
 static int floppy_probe_device(const char *filename)
 {
+int fd, ret;
+int prio = 0;
+struct floppy_struct fdparam;
+
 if (strstart(filename, /dev/fd, NULL))
-return 100;
-return 0;
+prio = 50;
+
+fd = open(filename, O_RDONLY | O_NONBLOCK);
+if (fd  0) {
+goto out;
+}
+
+/* Attempt to detect via a floppy specific ioctl */
+ret = ioctl(fd, FDGETPRM, fdparam);
+if (ret = 0)
+prio = 100;
+
+close(fd);
+out:
+return prio;
 }
 
 
-- 
1.6.5.2





[Qemu-devel] [PATCH] qemu-iotests: Test qemu-img rebase

2010-01-14 Thread Kevin Wolf
Signed-off-by: Kevin Wolf kw...@redhat.com
---
 024 |  104 +++
 024.out |   81 +
 group   |1 +
 3 files changed, 186 insertions(+), 0 deletions(-)
 create mode 100755 024
 create mode 100644 024.out

diff --git a/024 b/024
new file mode 100755
index 000..c53831f
--- /dev/null
+++ b/024
@@ -0,0 +1,104 @@
+#!/bin/sh
+#
+# Rebasing COW images
+#
+# Copyright (C) 2009 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/.
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo QA output created by $seq
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+rm -f $TEST_DIR/t.$IMGFMT.base_old
+rm -f $TEST_DIR/t.$IMGFMT.base_new
+}
+trap _cleanup; exit \$status 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Currently only qcow2 supports rebasing
+_supported_fmt qcow2
+_supported_os Linux
+
+CLUSTER_SIZE=65536
+
+# Cluster allocations to be tested:
+#
+# Backing (old) 11  --  11  --  11  --  11  --
+# Backing (new) 22  22  --  --  22  22  --  --
+# COW image 33  33  33  33  --  --  --  --
+
+echo Creating backing file
+echo
+
+_make_test_img 1G
+io_pattern writev $((-2 * CLUSTER_SIZE)) $CLUSTER_SIZE $((2 * CLUSTER_SIZE)) 4 
0x11
+mv $TEST_IMG $TEST_IMG.base_old
+
+echo Creating new backing file
+echo
+
+_make_test_img 1G
+io_pattern writev $((-4 * CLUSTER_SIZE)) $((2 * CLUSTER_SIZE)) $((4 * 
CLUSTER_SIZE)) 2 0x22
+mv $TEST_IMG $TEST_IMG.base_new
+
+echo Creating COW image
+echo
+
+_make_test_img -b $TEST_IMG.base_old 1G
+io_pattern writev 0 $((4 * CLUSTER_SIZE)) 0 1 0x33
+
+echo Read before the rebase to make sure everything is set up correctly
+echo
+io_pattern readv $((0 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((1 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((2 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((3 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((4 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
+io_pattern readv $((5 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+io_pattern readv $((6 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
+io_pattern readv $((7 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+
+echo
+echo Rebase and test again
+echo
+$QEMU_IMG rebase -b $TEST_IMG.base_new $TEST_IMG
+io_pattern readv $((0 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((1 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((2 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((3 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x33
+io_pattern readv $((4 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
+io_pattern readv $((5 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+io_pattern readv $((6 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
+io_pattern readv $((7 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+
+
+# success, all done
+echo *** done
+rm -f $seq.full
+status=0
diff --git a/024.out b/024.out
new file mode 100644
index 000..f75b75e
--- /dev/null
+++ b/024.out
@@ -0,0 +1,81 @@
+QA output created by 024
+Creating backing file
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 cluster_size=65536 
+=== IO: pattern 0x11
+qemu-io wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io wrote 65536/65536 bytes at offset 131072
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io wrote 65536/65536 bytes at offset 262144
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io wrote 65536/65536 bytes at offset 393216
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io Creating new backing file
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 cluster_size=65536 
+=== IO: pattern 0x22
+qemu-io wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io wrote 131072/131072 bytes at offset 262144
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io Creating COW image
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
backing_file='TEST_DIR/t.IMGFMT.base_old' cluster_size=65536 
+=== IO: pattern 0x33
+qemu-io wrote 262144/262144 bytes at offset 0
+256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-io Read before the rebase to make sure 

[Qemu-devel] [PATCH 1/8] VNC: Use 'enabled' key instead of 'status'

2010-01-14 Thread Luiz Capitulino
Currently the 'status' key is a string whose value can be
disabled or enabled, change it to the QMP's standard
'enabled' key, which is a bool.

Note that 'status' in being dropped and this wouldn't be
allowed if QMP were stable.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 vnc.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/vnc.c b/vnc.c
index 58eac73..ef86ef7 100644
--- a/vnc.c
+++ b/vnc.c
@@ -254,7 +254,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
 QList *clients;
 
 server = qobject_to_qdict(data);
-if (strcmp(qdict_get_str(server, status), disabled) == 0) {
+if (qdict_get_bool(server, enabled) == 0) {
 monitor_printf(mon, Server: disabled\n);
 return;
 }
@@ -282,7 +282,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * The main QDict contains the following:
  *
- * - status: disabled or enabled
+ * - enabled: true or false
  * - host: server's IP address
  * - service: server's port number
  * - auth: authentication method (optional)
@@ -297,13 +297,13 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * Example:
  *
- * { status: enabled, host: 0.0.0.0, service: 50402, auth: vnc,
+ * { enabled: true, host: 0.0.0.0, service: 50402, auth: vnc,
  *   clients: [ { host: 127.0.0.1, service: 50401 } ] }
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
 if (vnc_display == NULL || vnc_display-display == NULL) {
-*ret_data = qobject_from_jsonf({ 'status': 'disabled' });
+*ret_data = qobject_from_jsonf({ 'enabled': false });
 } else {
 QDict *qdict;
 QList *clist;
@@ -319,7 +319,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
 }
 }
 
-*ret_data = qobject_from_jsonf({ 'status': 'enabled', 'clients': %p 
},
+*ret_data = qobject_from_jsonf({ 'enabled': true, 'clients': %p },
QOBJECT(clist));
 assert(*ret_data != NULL);
 
-- 
1.6.6





[Qemu-devel] [PATCH 2/8] VNC: Make 'auth' key mandatory

2010-01-14 Thread Luiz Capitulino
There is no reason to have it as optional and the code
in the server and client gets slightly simpler if the
key is mandatory.

While there also do some cleanup on how the server info is
collected.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 vnc.c |   26 ++
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/vnc.c b/vnc.c
index ef86ef7..b5f3b64 100644
--- a/vnc.c
+++ b/vnc.c
@@ -122,7 +122,7 @@ static int put_addr_qdict(QDict *qdict, struct 
sockaddr_storage *sa,
 return 0;
 }
 
-static int vnc_qdict_local_addr(QDict *qdict, int fd)
+static int vnc_server_addr_put(QDict *qdict, int fd)
 {
 struct sockaddr_storage sa;
 socklen_t salen;
@@ -199,6 +199,16 @@ static const char *vnc_auth_name(VncDisplay *vd) {
 return unknown;
 }
 
+static int vnc_server_info_put(QDict *qdict)
+{
+if (vnc_server_addr_put(qdict, vnc_display-lsock)  0) {
+return -1;
+}
+
+qdict_put(qdict, auth, qstring_from_str(vnc_auth_name(vnc_display)));
+return 0;
+}
+
 static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
 {
 QDict *qdict;
@@ -263,8 +273,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
 monitor_printf(mon,  address: %s:%s\n,
qdict_get_str(server, host),
qdict_get_str(server, service));
-monitor_printf(mon, auth: %s\n,
-qdict_haskey(server, auth) ? qdict_get_str(server, auth) : none);
+monitor_printf(mon, auth: %s\n, qdict_get_str(server, auth));
 
 clients = qdict_get_qlist(server, clients);
 if (qlist_empty(clients)) {
@@ -285,7 +294,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - enabled: true or false
  * - host: server's IP address
  * - service: server's port number
- * - auth: authentication method (optional)
+ * - auth: authentication method
  * - clients: a QList of all connected clients
  *
  * Clients are described by a QDict, with the following information:
@@ -323,14 +332,7 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
QOBJECT(clist));
 assert(*ret_data != NULL);
 
-qdict = qobject_to_qdict(*ret_data);
-
-if (vnc_display-auth != VNC_AUTH_NONE) {
-qdict_put(qdict, auth,
-  qstring_from_str(vnc_auth_name(vnc_display)));
-}
-
-if (vnc_qdict_local_addr(qdict, vnc_display-lsock)  0) {
+if (vnc_server_info_put(qobject_to_qdict(*ret_data))  0) {
 qobject_decref(*ret_data);
 *ret_data = NULL;
 }
-- 
1.6.6





[Qemu-devel] [PATCH 3/8] VNC: Rename client's 'username' key

2010-01-14 Thread Luiz Capitulino
It's the SASL username, so it's better to call it 'sasl_username'
to be consistent.

Note that this change wouldn't be allowed if QMP were stable.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 vnc.c |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/vnc.c b/vnc.c
index b5f3b64..e35cc36 100644
--- a/vnc.c
+++ b/vnc.c
@@ -228,7 +228,8 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState 
*client)
 #ifdef CONFIG_VNC_SASL
 if (client-sasl.conn 
 client-sasl.username) {
-qdict_put(qdict, username, qstring_from_str(client-sasl.username));
+qdict_put(qdict, sasl_username,
+  qstring_from_str(client-sasl.username));
 }
 #endif
 
@@ -253,8 +254,8 @@ static void info_vnc_iter(QObject *obj, void *opaque)
 #endif
 #ifdef CONFIG_VNC_SASL
 monitor_printf(mon, username: %s\n,
-qdict_haskey(client, username) ?
-qdict_get_str(client, username) : none);
+qdict_haskey(client, sasl_username) ?
+qdict_get_str(client, sasl_username) : none);
 #endif
 }
 
@@ -302,7 +303,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * - host: client's IP address
  * - service: client's port number
  * - x509_dname: TLS dname (optional)
- * - username: SASL username (optional)
+ * - sasl_username: SASL username (optional)
  *
  * Example:
  *
-- 
1.6.6





[Qemu-devel] [PATCH v0 0/8]: VNC events and cleanup

2010-01-14 Thread Luiz Capitulino
 Hi there,

 This series contains two VNC related changes. First a small cleanup
is done in the current 'query-vnc' command _response_, then the following
QMP events are introduced:

- VNC_CONNECTED: emitted when a VNC client establishes a connection
- VNC_INITIALIZED: emitted when the VNC session is made active
- VNC_DISCONNECTED: emitted when the conection is closed

 The only issue is the current events documentation. I'm using the
current format which is quite bad, things will improve when we have
proper documentation support (being worked out by Markus).

 Thanks.




[Qemu-devel] [PATCH 4/8] VNC: Add 'family' key

2010-01-14 Thread Luiz Capitulino
It contains the socket adress family name, like ipv4 or
ipv6.

This is useful for clients so that they can interpret the
'host' key reliably.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 vnc.c |   26 +-
 1 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/vnc.c b/vnc.c
index e35cc36..e023824 100644
--- a/vnc.c
+++ b/vnc.c
@@ -100,6 +100,26 @@ char *vnc_socket_remote_addr(const char *format, int fd) {
 return addr_to_string(format, sa, salen);
 }
 
+static QString *get_sock_family(const struct sockaddr_storage *sa)
+{
+const char *name;
+
+switch (sa-ss_family)
+{
+case AF_INET:
+name = ipv4;
+break;
+case AF_INET6:
+name = ipv6;
+break;
+default:
+name = unknown;
+break;
+}
+
+return qstring_from_str(name);
+}
+
 static int put_addr_qdict(QDict *qdict, struct sockaddr_storage *sa,
   socklen_t salen)
 {
@@ -118,6 +138,7 @@ static int put_addr_qdict(QDict *qdict, struct 
sockaddr_storage *sa,
 
 qdict_put(qdict, host, qstring_from_str(host));
 qdict_put(qdict, service, qstring_from_str(serv));
+qdict_put(qdict, family, get_sock_family(sa));
 
 return 0;
 }
@@ -294,6 +315,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  *
  * - enabled: true or false
  * - host: server's IP address
+ * - family: address family (ipv4 or ipv6)
  * - service: server's port number
  * - auth: authentication method
  * - clients: a QList of all connected clients
@@ -301,6 +323,7 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * Clients are described by a QDict, with the following information:
  *
  * - host: client's IP address
+ * - family: address family (ipv4 or ipv6)
  * - service: client's port number
  * - x509_dname: TLS dname (optional)
  * - sasl_username: SASL username (optional)
@@ -308,7 +331,8 @@ void do_info_vnc_print(Monitor *mon, const QObject *data)
  * Example:
  *
  * { enabled: true, host: 0.0.0.0, service: 50402, auth: vnc,
- *   clients: [ { host: 127.0.0.1, service: 50401 } ] }
+ *   family: ipv4,
+ *   clients: [{ host: 127.0.0.1, service: 50401, family: ipv4 
}]}
  */
 void do_info_vnc(Monitor *mon, QObject **ret_data)
 {
-- 
1.6.6





[Qemu-devel] [PATCH 5/8] VNC: Cache client info at connection time

2010-01-14 Thread Luiz Capitulino
When a disconnection happens the client's socket on QEMU
side may become invalid, this way it won't be possible
to query it to get client information, which is going to
be needed by the future QMP VNC_DISCONNECTED event.

To always have this information available we query the
socket at connection time and cache the client info in
struct VncState.

Two function are introduced to perform this job.

vnc_client_cache_addr() is called right when the connection
is made, however the authentication information is not
available at that moment so vnc_client_cache_auth() is
called from protocol_client_init() to get auth info.

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 vnc.c |   40 ++--
 vnc.h |2 ++
 2 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/vnc.c b/vnc.c
index e023824..d37fa60 100644
--- a/vnc.c
+++ b/vnc.c
@@ -230,16 +230,16 @@ static int vnc_server_info_put(QDict *qdict)
 return 0;
 }
 
-static QDict *do_info_vnc_client(Monitor *mon, VncState *client)
+static void vnc_client_cache_auth(VncState *client)
 {
 QDict *qdict;
 
-qdict = qdict_new();
-if (vnc_qdict_remote_addr(qdict, client-csock)  0) {
-QDECREF(qdict);
-return NULL;
+if (!client-info) {
+return;
 }
 
+qdict = qobject_to_qdict(client-info);
+
 #ifdef CONFIG_VNC_TLS
 if (client-tls.session 
 client-tls.dname) {
@@ -253,8 +253,20 @@ static QDict *do_info_vnc_client(Monitor *mon, VncState 
*client)
   qstring_from_str(client-sasl.username));
 }
 #endif
+}
 
-return qdict;
+static void vnc_client_cache_addr(VncState *client)
+{
+QDict *qdict;
+
+qdict = qdict_new();
+if (vnc_qdict_remote_addr(qdict, client-csock)  0) {
+QDECREF(qdict);
+/* XXX: how to report the error? */
+return;
+}
+
+client-info = QOBJECT(qdict);
 }
 
 static void info_vnc_iter(QObject *obj, void *opaque)
@@ -339,16 +351,17 @@ void do_info_vnc(Monitor *mon, QObject **ret_data)
 if (vnc_display == NULL || vnc_display-display == NULL) {
 *ret_data = qobject_from_jsonf({ 'enabled': false });
 } else {
-QDict *qdict;
 QList *clist;
 
 clist = qlist_new();
 if (vnc_display-clients) {
 VncState *client = vnc_display-clients;
 while (client) {
-qdict = do_info_vnc_client(mon, client);
-if (qdict)
-qlist_append(clist, qdict);
+if (client-info) {
+/* incref so that it's not freed by upper layers */
+qobject_incref(client-info);
+qlist_append_obj(clist, client-info);
+}
 client = client-next;
 }
 }
@@ -1079,6 +1092,9 @@ static void vnc_disconnect_finish(VncState *vs)
 qemu_free(vs-output.buffer);
 vs-output.buffer = NULL;
 }
+
+qobject_decref(vs-info);
+
 #ifdef CONFIG_VNC_TLS
 vnc_tls_client_cleanup(vs);
 #endif /* CONFIG_VNC_TLS */
@@ -2069,6 +2085,8 @@ static int protocol_client_init(VncState *vs, uint8_t 
*data, size_t len)
 vnc_write(vs, buf, size);
 vnc_flush(vs);
 
+vnc_client_cache_auth(vs);
+
 vnc_read_when(vs, protocol_client_msg, 1);
 
 return 0;
@@ -2377,6 +2395,8 @@ static void vnc_connect(VncDisplay *vd, int csock)
 socket_set_nonblock(vs-csock);
 qemu_set_fd_handler2(vs-csock, NULL, vnc_client_read, NULL, vs);
 
+vnc_client_cache_addr(vs);
+
 vs-vd = vd;
 vs-ds = vd-ds;
 vs-last_x = -1;
diff --git a/vnc.h b/vnc.h
index fcc6824..1210824 100644
--- a/vnc.h
+++ b/vnc.h
@@ -144,6 +144,8 @@ struct VncState
 VncStateSASL sasl;
 #endif
 
+QObject *info;
+
 Buffer output;
 Buffer input;
 /* current output mode information */
-- 
1.6.6





[Qemu-devel] [PATCH 6/8] QMP: Introduce VNC_CONNECTED event

2010-01-14 Thread Luiz Capitulino
It's emitted when a VNC client connects to QEMU, client's information
such as port and IP address are provided.

Note that this event is emitted right when the connection is
established. This means that it happens before authentication
procedure and session initialization.

Event example:

{ event: VNC_CONNECTED,
timestamp: { seconds: 1262976601, microseconds: 975795 },
data: {
server: { auth: sasl, family: ipv4,
service: 5901, host: 0.0.0.0 },
client: { family: ipv4, service: 58425,
host: 127.0.0.1 } } }

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp-events.txt |7 +++
 monitor.c  |3 +++
 monitor.h  |1 +
 vnc.c  |   25 +
 4 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 682a5e5..d36da46 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -24,3 +24,10 @@ Data: None.
 
 Description: Issued when the Virtual Machine enters debug mode.
 Data: None.
+
+4 VNC_CONNECTED
+---
+
+Description: Issued when a VNC client establishes a connection.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc',
+except that authentication ID is not provided.
diff --git a/monitor.c b/monitor.c
index 8e00f68..4a6af42 100644
--- a/monitor.c
+++ b/monitor.c
@@ -357,6 +357,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 case QEVENT_STOP:
 event_name = STOP;
 break;
+case QEVENT_VNC_CONNECTED:
+event_name = VNC_CONNECTED;
+break;
 default:
 abort();
 break;
diff --git a/monitor.h b/monitor.h
index 6ed117a..4d57679 100644
--- a/monitor.h
+++ b/monitor.h
@@ -20,6 +20,7 @@ typedef enum MonitorEvent {
 QEVENT_RESET,
 QEVENT_POWERDOWN,
 QEVENT_STOP,
+QEVENT_VNC_CONNECTED,
 QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vnc.c b/vnc.c
index d37fa60..6d488e5 100644
--- a/vnc.c
+++ b/vnc.c
@@ -269,6 +269,30 @@ static void vnc_client_cache_addr(VncState *client)
 client-info = QOBJECT(qdict);
 }
 
+static void vnc_qmp_event(VncState *vs, MonitorEvent event)
+{
+QDict *server;
+QObject *data;
+
+if (!vs-info) {
+return;
+}
+
+server = qdict_new();
+if (vnc_server_info_put(server)  0) {
+QDECREF(server);
+return;
+}
+
+data = qobject_from_jsonf({ 'client': %p, 'server': %p },
+  vs-info, QOBJECT(server));
+
+monitor_protocol_event(event, data);
+
+qobject_incref(vs-info);
+qobject_decref(data);
+}
+
 static void info_vnc_iter(QObject *obj, void *opaque)
 {
 QDict *client;
@@ -2396,6 +2420,7 @@ static void vnc_connect(VncDisplay *vd, int csock)
 qemu_set_fd_handler2(vs-csock, NULL, vnc_client_read, NULL, vs);
 
 vnc_client_cache_addr(vs);
+vnc_qmp_event(vs, QEVENT_VNC_CONNECTED);
 
 vs-vd = vd;
 vs-ds = vd-ds;
-- 
1.6.6





[Qemu-devel] [PATCH 7/8] QMP: Introduce VNC_DISCONNECTED event

2010-01-14 Thread Luiz Capitulino
It's emitted when a VNC client disconnects from QEMU, client's
information such as port and IP address are provided.

Event example:

{ event: VNC_DISCONNECTED,
timestamp: { seconds: 1262976601, microseconds: 975795 },
data: {
server: { auth: sasl, family: ipv4,
service: 5901, host: 0.0.0.0 },
client: { family: ipv4, service: 58425,
host: 127.0.0.1, sasl_username: foo } } }

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp-events.txt |6 ++
 monitor.c  |3 +++
 monitor.h  |1 +
 vnc.c  |2 ++
 4 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index d36da46..1e87eb1 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -31,3 +31,9 @@ Data: None.
 Description: Issued when a VNC client establishes a connection.
 Data: 'server' and 'client' keys with the same keys as 'query-vnc',
 except that authentication ID is not provided.
+
+5 VNC_DISCONNECTED
+--
+
+Description: Issued when the conection is closed.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
diff --git a/monitor.c b/monitor.c
index 4a6af42..5ac1c5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -360,6 +360,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 case QEVENT_VNC_CONNECTED:
 event_name = VNC_CONNECTED;
 break;
+case QEVENT_VNC_DISCONNECTED:
+event_name = VNC_DISCONNECTED;
+break;
 default:
 abort();
 break;
diff --git a/monitor.h b/monitor.h
index 4d57679..42386de 100644
--- a/monitor.h
+++ b/monitor.h
@@ -21,6 +21,7 @@ typedef enum MonitorEvent {
 QEVENT_POWERDOWN,
 QEVENT_STOP,
 QEVENT_VNC_CONNECTED,
+QEVENT_VNC_DISCONNECTED,
 QEVENT_MAX,
 } MonitorEvent;
 
diff --git a/vnc.c b/vnc.c
index 6d488e5..a590bb9 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1108,6 +1108,8 @@ static void vnc_disconnect_start(VncState *vs)
 
 static void vnc_disconnect_finish(VncState *vs)
 {
+vnc_qmp_event(vs, QEVENT_VNC_DISCONNECTED);
+
 if (vs-input.buffer) {
 qemu_free(vs-input.buffer);
 vs-input.buffer = NULL;
-- 
1.6.6





[Qemu-devel] [PATCH 8/8] QMP: Introduce VNC_INITIALIZED event

2010-01-14 Thread Luiz Capitulino
It's emitted when a VNC client session is activated by QEMU,
client's information such as port, IP and auth ID (if the
session is authenticated) are provided.

Event example:

{ event: VNC_INITIALIZED,
timestamp: {seconds: 1263475302, microseconds: 150772},
data: {
server: { auth: sasl, family: ipv4,
service: 5901, host: 0.0.0.0},
client: { family: ipv4, service: 46089,
host: 127.0.0.1, sasl_username: lcapitulino } } }

Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 QMP/qmp-events.txt |6 ++
 monitor.c  |3 +++
 monitor.h  |1 +
 vnc.c  |1 +
 4 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 1e87eb1..dc48ccc 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -37,3 +37,9 @@ except that authentication ID is not provided.
 
 Description: Issued when the conection is closed.
 Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
+
+6 VNC_INITIALIZED
+-
+
+Description: Issued when the VNC session is made active.
+Data: 'server' and 'client' keys with the same keys as 'query-vnc'.
diff --git a/monitor.c b/monitor.c
index 5ac1c5c..f2abd3c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -360,6 +360,9 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 case QEVENT_VNC_CONNECTED:
 event_name = VNC_CONNECTED;
 break;
+case QEVENT_VNC_INITIALIZED:
+event_name = VNC_INITIALIZED;
+break;
 case QEVENT_VNC_DISCONNECTED:
 event_name = VNC_DISCONNECTED;
 break;
diff --git a/monitor.h b/monitor.h
index 42386de..2da30e8 100644
--- a/monitor.h
+++ b/monitor.h
@@ -21,6 +21,7 @@ typedef enum MonitorEvent {
 QEVENT_POWERDOWN,
 QEVENT_STOP,
 QEVENT_VNC_CONNECTED,
+QEVENT_VNC_INITIALIZED,
 QEVENT_VNC_DISCONNECTED,
 QEVENT_MAX,
 } MonitorEvent;
diff --git a/vnc.c b/vnc.c
index a590bb9..c7d6652 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2112,6 +2112,7 @@ static int protocol_client_init(VncState *vs, uint8_t 
*data, size_t len)
 vnc_flush(vs);
 
 vnc_client_cache_auth(vs);
+vnc_qmp_event(vs, QEVENT_VNC_INITIALIZED);
 
 vnc_read_when(vs, protocol_client_msg, 1);
 
-- 
1.6.6





Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations

2010-01-14 Thread Aurelien Jarno
On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
 
 On 14.01.2010, at 16:13, Aurelien Jarno wrote:
 
  On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
  The recent transition to always have the DCR helper functions take 32 bit
  values broke the PPC64 target, as tlong became 64 bits there.
  
  This patch moves all translate.c callers to a _tl function that simply
  calls the uint32_t functions. That way we don't need to mess with TCG
  trying to pass registers as uint32_t variables to functions.
  
  Fixes PPC64 build with --enable-debug-tcg
  
  Signed-off-by: Alexander Graf ag...@suse.de
  Reported-by: Stefan Weil w...@mail.berlios.de
  ---
  target-ppc/cpu.h   |2 ++
  target-ppc/helper.h|4 ++--
  target-ppc/op_helper.c |   10 ++
  target-ppc/translate.c |   12 ++--
  4 files changed, 20 insertions(+), 8 deletions(-)
  
  diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
  index d15bba1..60a8b68 100644
  --- a/target-ppc/cpu.h
  +++ b/target-ppc/cpu.h
  @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, 
  target_ulong rs);
  void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
  #endif /* !defined(CONFIG_USER_ONLY) */
  void ppc_store_msr (CPUPPCState *env, target_ulong value);
  +void helper_store_dcr (uint32_t dcrn, uint32_t val);
  +uint32_t helper_load_dcr (uint32_t dcrn);
  
  void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
  ...));
  
  diff --git a/target-ppc/helper.h b/target-ppc/helper.h
  index 40d4ced..86f0af7 100644
  --- a/target-ppc/helper.h
  +++ b/target-ppc/helper.h
  @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
  DEF_HELPER_2(divs, tl, tl, tl)
  DEF_HELPER_2(divso, tl, tl, tl)
  
  -DEF_HELPER_1(load_dcr, i32, i32);
  -DEF_HELPER_2(store_dcr, void, i32, i32)
  +DEF_HELPER_1(load_dcr_tl, tl, tl);
  +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
  
  DEF_HELPER_1(load_dump_spr, void, i32)
  DEF_HELPER_1(store_dump_spr, void, i32)
  diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
  index cea27f2..6c375d3 100644
  --- a/target-ppc/op_helper.c
  +++ b/target-ppc/op_helper.c
  @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
  return val;
  }
  
  +target_ulong helper_load_dcr_tl (target_ulong dcrn)
  +{
  +return (uint32_t)helper_load_dcr((uint32_t)dcrn);
  +}
  +
  void helper_store_dcr (uint32_t dcrn, uint32_t val)
  {
  if (unlikely(env-dcr_env == NULL)) {
  @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
  }
  }
  
  +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
  +{
  +helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
  +}
  +
  
  I do wonder why we need to keep the old helper_load_dcr() and
  helper_store_dcr() instead of modifying them directly. They doesn't seems
  to be used elsewhere.
 
 Last time I checked they were used in hw/*ppc*.c. Maybe mangled through funny 
 preprocessor or function callback logic. But maybe I'm wrong :).
 

I am not able to find them. I have tried to remove helper_load_dcr() and
helper_store_dcr() from target-ppc/cpu.h and the code still compiles.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] [PATCH] PPC: Add wrapper for target long DCR operations

2010-01-14 Thread Alexander Graf

On 14.01.2010, at 18:02, Aurelien Jarno wrote:

 On Thu, Jan 14, 2010 at 04:19:31PM +0100, Alexander Graf wrote:
 
 On 14.01.2010, at 16:13, Aurelien Jarno wrote:
 
 On Fri, Jan 01, 2010 at 04:41:06PM +0100, Alexander Graf wrote:
 The recent transition to always have the DCR helper functions take 32 bit
 values broke the PPC64 target, as tlong became 64 bits there.
 
 This patch moves all translate.c callers to a _tl function that simply
 calls the uint32_t functions. That way we don't need to mess with TCG
 trying to pass registers as uint32_t variables to functions.
 
 Fixes PPC64 build with --enable-debug-tcg
 
 Signed-off-by: Alexander Graf ag...@suse.de
 Reported-by: Stefan Weil w...@mail.berlios.de
 ---
 target-ppc/cpu.h   |2 ++
 target-ppc/helper.h|4 ++--
 target-ppc/op_helper.c |   10 ++
 target-ppc/translate.c |   12 ++--
 4 files changed, 20 insertions(+), 8 deletions(-)
 
 diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
 index d15bba1..60a8b68 100644
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -733,6 +733,8 @@ void ppc_store_slb (CPUPPCState *env, target_ulong rb, 
 target_ulong rs);
 void ppc_store_sr (CPUPPCState *env, int srnum, target_ulong value);
 #endif /* !defined(CONFIG_USER_ONLY) */
 void ppc_store_msr (CPUPPCState *env, target_ulong value);
 +void helper_store_dcr (uint32_t dcrn, uint32_t val);
 +uint32_t helper_load_dcr (uint32_t dcrn);
 
 void ppc_cpu_list (FILE *f, int (*cpu_fprintf)(FILE *f, const char *fmt, 
 ...));
 
 diff --git a/target-ppc/helper.h b/target-ppc/helper.h
 index 40d4ced..86f0af7 100644
 --- a/target-ppc/helper.h
 +++ b/target-ppc/helper.h
 @@ -359,8 +359,8 @@ DEF_HELPER_2(divo, tl, tl, tl)
 DEF_HELPER_2(divs, tl, tl, tl)
 DEF_HELPER_2(divso, tl, tl, tl)
 
 -DEF_HELPER_1(load_dcr, i32, i32);
 -DEF_HELPER_2(store_dcr, void, i32, i32)
 +DEF_HELPER_1(load_dcr_tl, tl, tl);
 +DEF_HELPER_2(store_dcr_tl, void, tl, tl)
 
 DEF_HELPER_1(load_dump_spr, void, i32)
 DEF_HELPER_1(store_dump_spr, void, i32)
 diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
 index cea27f2..6c375d3 100644
 --- a/target-ppc/op_helper.c
 +++ b/target-ppc/op_helper.c
 @@ -1844,6 +1844,11 @@ uint32_t helper_load_dcr (uint32_t dcrn)
return val;
 }
 
 +target_ulong helper_load_dcr_tl (target_ulong dcrn)
 +{
 +return (uint32_t)helper_load_dcr((uint32_t)dcrn);
 +}
 +
 void helper_store_dcr (uint32_t dcrn, uint32_t val)
 {
if (unlikely(env-dcr_env == NULL)) {
 @@ -1857,6 +1862,11 @@ void helper_store_dcr (uint32_t dcrn, uint32_t val)
}
 }
 
 +void helper_store_dcr_tl (target_ulong dcrn, target_ulong val)
 +{
 +helper_store_dcr((uint32_t)dcrn, (uint32_t)val);
 +}
 +
 
 I do wonder why we need to keep the old helper_load_dcr() and
 helper_store_dcr() instead of modifying them directly. They doesn't seems
 to be used elsewhere.
 
 Last time I checked they were used in hw/*ppc*.c. Maybe mangled through 
 funny preprocessor or function callback logic. But maybe I'm wrong :).
 
 
 I am not able to find them. I have tried to remove helper_load_dcr() and
 helper_store_dcr() from target-ppc/cpu.h and the code still compiles.

Hm, right. The only references are to ppc_dcr_read and ppc_dcr_write. I guess 
the other helpers are superfluous then.

Alex



Re: [Qemu-devel] [patch] Fix a typo in 'P' packet processing for M68K.

2010-01-14 Thread Kazu Hirata

Hi Aurelien,


I don't have a write access to the repository.  Could someone apply
this patch if it's OK?


This patch looks ok, but is missing a Signed-of-by:


Thank you for a review.  I'm going to resubmit the patch in the git style.

Kazu Hirata




[Qemu-devel] [PATCH] Fix a typo in 'P' packet processing for M68K.

2010-01-14 Thread Kazu Hirata
Hi,

Attached is a patch to fix a typo in 'P' packet processing for M68K.

Without this patch, QEMU fails to honor GDB's P packets from GDB
(writing to registers) for the address registers (A0 - A7).

The problem is because of an obvious typo.  Notice that the second
if condition is meant to be n  16 in:

  if (n  8) {
:
  } else if (n  8) {

Signed-off-by: Kazu Hirata k...@codesourcery.com
---
 gdbstub.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 6180171..80477be 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1014,7 +1014,7 @@ static int cpu_gdb_write_register(CPUState *env, uint8_t 
*mem_buf, int n)
 if (n  8) {
 /* D0-D7 */
 env-dregs[n] = tmp;
-} else if (n  8) {
+} else if (n  16) {
 /* A0-A7 */
 env-aregs[n - 8] = tmp;
 } else {
-- 
1.6.2.4





[Qemu-devel] [PATCH] Makefile: Fix message for missing configure

2010-01-14 Thread Stefan Weil
When make is called without a valid configuration,
it should tell the user what to do.

Revision 0e8c9214ba1d4128cf92442cd343bc3733478261
was a regression which resulted in a message
which was no longer user friendly
(reported by Aurelien Jarno).

This patch restores the old behaviour.

Cc: Aurelien Jarno aurel...@aurel32.net
Signed-off-by: Stefan Weil w...@mail.berlios.de
---
 Makefile |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index fa7f851..b1bbe6d 100644
--- a/Makefile
+++ b/Makefile
@@ -75,7 +75,9 @@ SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
 subdir-%: $(GENERATED_HEADERS)
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V=$(V) 
TARGET_DIR=$*/ all,)
 
+ifneq ($(wildcard config-host.mak),)
 include $(SRC_PATH)/Makefile.objs
+endif
 
 $(common-obj-y): $(GENERATED_HEADERS)
 $(filter %-softmmu,$(SUBDIR_RULES)): $(common-obj-y)
-- 
1.6.5





Re: [Qemu-devel] Error message regression

2010-01-14 Thread Andreas Färber


Am 14.01.2010 um 15:50 schrieb Aurelien Jarno:


Starting with commit 0e8c9214ba1d4128cf92442cd343bc3733478261, when
trying to run make without running configure first, the error message
has been changed from:

| $ make
| Please call configure before running make!
| make: *** [config-host.mak] Erreur 1

to this more cryptic message:

| $ make
| Makefile:78: /Makefile.objs: No such file or directory
| make: *** No rule to make target `/Makefile.objs'.  Stop.


Sorry, didn't test that.


Any idea how to fix that properly?


I'd assume this is due to $SRC_PATH being undefined. Probably an  
ifneq(...) around the include would do the trick, I'll have a look  
later today.


Andreas




[Qemu-devel] Re: [PATCH v0 0/8]: VNC events and cleanup

2010-01-14 Thread Daniel P. Berrange
On Thu, Jan 14, 2010 at 02:50:51PM -0200, Luiz Capitulino wrote:
  Hi there,
 
  This series contains two VNC related changes. First a small cleanup
 is done in the current 'query-vnc' command _response_, then the following
 QMP events are introduced:
 
 - VNC_CONNECTED: emitted when a VNC client establishes a connection
 - VNC_INITIALIZED: emitted when the VNC session is made active
 - VNC_DISCONNECTED: emitted when the conection is closed
 
  The only issue is the current events documentation. I'm using the
 current format which is quite bad, things will improve when we have
 proper documentation support (being worked out by Markus).

This series looks reasonable to me wrt VNC.

Should we try to think about what we'll do with other network services ?
eg will we add SPICE_CONNECTED / SPICE_DISCONNECTED in the future ?
Similar question for chardevs using networking ?


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|




[Qemu-devel] Re: [PATCH] Makefile: Fix message for missing configure

2010-01-14 Thread Andreas Färber


Am 14.01.2010 um 18:11 schrieb Stefan Weil:


When make is called without a valid configuration,
it should tell the user what to do.

Revision 0e8c9214ba1d4128cf92442cd343bc3733478261
was a regression which resulted in a message
which was no longer user friendly
(reported by Aurelien Jarno).

This patch restores the old behaviour.

Cc: Aurelien Jarno aurel...@aurel32.net
Signed-off-by: Stefan Weil w...@mail.berlios.de


Reviewed-by: Andreas Färber afaer...@opensolaris.org

Looks okay, thanks.

Andreas


---
Makefile |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index fa7f851..b1bbe6d 100644
--- a/Makefile
+++ b/Makefile
@@ -75,7 +75,9 @@ SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
subdir-%: $(GENERATED_HEADERS)
	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V=$(V)  
TARGET_DIR=$*/ all,)


+ifneq ($(wildcard config-host.mak),)
include $(SRC_PATH)/Makefile.objs
+endif

$(common-obj-y): $(GENERATED_HEADERS)
$(filter %-softmmu,$(SUBDIR_RULES)): $(common-obj-y)
--
1.6.5







Re: [Qemu-devel] [PATCH 2/2] tcg-x86_64: Avoid unnecessary REX.B prefixes.

2010-01-14 Thread Richard Henderson

On 01/14/2010 08:10 AM, Aurelien Jarno wrote:

On Tue, Jan 05, 2010 at 04:31:11PM -0800, Richard Henderson wrote:

A while ago Laurent pointed out that the setcc opcode emitted by
the setcond patch had unnecessary REX prefixes.

The existing P_REXB internal opcode flag unconditionally emits
the REX prefix.  Technically it's not needed if the register in
question is %al, %bl, %cl, %dl.

Eliding the prefix requires splitting the P_REXB flag into two,
in order to indicate whether the byte register in question is
in the REG or the R/M field.  Within TCG, the byte register is
in the REG field only for stores.

Signed-off-by: Richard Hendersonr...@twiddle.net
---
  tcg/x86_64/tcg-target.c |   46 ++
  1 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
index f584c94..8b6b68c 100644
--- a/tcg/x86_64/tcg-target.c
+++ b/tcg/x86_64/tcg-target.c
@@ -217,9 +217,10 @@ static inline int tcg_target_const_match(tcg_target_long 
val,
  #define JCC_JLE 0xe
  #define JCC_JG  0xf

-#define P_EXT   0x100 /* 0x0f opcode prefix */
-#define P_REXW  0x200 /* set rex.w = 1 */
-#define P_REXB  0x400 /* force rex use for byte registers */
+#define P_EXT  0x100   /* 0x0f opcode prefix */
+#define P_REXW 0x200   /* set rex.w = 1 */
+#define P_REXB_R   0x400   /* REG field as byte register */
+#define P_REXB_RM  0x800   /* R/M field as byte register */

  static const uint8_t tcg_cond_to_jcc[10] = {
  [TCG_COND_EQ] = JCC_JE,
@@ -234,17 +235,30 @@ static const uint8_t tcg_cond_to_jcc[10] = {
  [TCG_COND_GTU] = JCC_JA,
  };

-static inline void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
+static void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
  {
-int rex;
-rex = ((opc  6)  0x8) | ((r  1)  0x4) |
-((x  2)  2) | ((rm  3)  1);
-if (rex || (opc  P_REXB)) {
+int rex = 0;
+
+rex |= (opc  P_REXW)  6;  /* REX.W */
+rex |= (r  8)  1; /* REX.R */
+rex |= (x  8)  2; /* REX.X */
+rex |= (rm  8)  3;/* REX.B */
+
+/* P_REXB_{R,RM} indicates that the given register is the low byte.
+   For %[abcd]l we need no REX prefix, but for %{si,di,bp,sp}l we do,
+   as otherwise the encoding indicates %[abcd]h.  Note that the values
+   that are ORed in merely indicate that the REX byte must be present;
+   those bits get discarded in output.  */
+rex |= (r= 4 ? opc  P_REXB_R : 0);
+rex |= (rm= 4 ? opc  P_REXB_RM : 0);
+
+if (rex) {
  tcg_out8(s, rex | 0x40);
  }


With the above change, rex can be  0xff. Not sure it's not a good idea
to not have an explicit cast when calling tcg_out8(), even if it
technically works.


Same as below.




-if (opc  P_EXT)
+if (opc  P_EXT) {
  tcg_out8(s, 0x0f);
-tcg_out8(s, opc  0xff);
+}
+tcg_out8(s, opc);


What's the reason for removing the '  0xff' part? tcg_out8() takes an uint8_t.


Yes, and the uint8_t truncates the value just fine.  Is there any 
particular reason you want to clutter the code with a duplicate 
truncation?  It might have been reasonable if the function name didn't 
quite clearly indicate that a single byte was going to be output...



r~




Re: [Qemu-devel] [PATCH 2/2] tcg-x86_64: Avoid unnecessary REX.B prefixes.

2010-01-14 Thread Aurelien Jarno
On Thu, Jan 14, 2010 at 10:09:48AM -0800, Richard Henderson wrote:
 On 01/14/2010 08:10 AM, Aurelien Jarno wrote:
 On Tue, Jan 05, 2010 at 04:31:11PM -0800, Richard Henderson wrote:
 A while ago Laurent pointed out that the setcc opcode emitted by
 the setcond patch had unnecessary REX prefixes.

 The existing P_REXB internal opcode flag unconditionally emits
 the REX prefix.  Technically it's not needed if the register in
 question is %al, %bl, %cl, %dl.

 Eliding the prefix requires splitting the P_REXB flag into two,
 in order to indicate whether the byte register in question is
 in the REG or the R/M field.  Within TCG, the byte register is
 in the REG field only for stores.

 Signed-off-by: Richard Hendersonr...@twiddle.net
 ---
   tcg/x86_64/tcg-target.c |   46 
 ++
   1 files changed, 30 insertions(+), 16 deletions(-)

 diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
 index f584c94..8b6b68c 100644
 --- a/tcg/x86_64/tcg-target.c
 +++ b/tcg/x86_64/tcg-target.c
 @@ -217,9 +217,10 @@ static inline int 
 tcg_target_const_match(tcg_target_long val,
   #define JCC_JLE 0xe
   #define JCC_JG  0xf

 -#define P_EXT   0x100 /* 0x0f opcode prefix */
 -#define P_REXW  0x200 /* set rex.w = 1 */
 -#define P_REXB  0x400 /* force rex use for byte registers */
 +#define P_EXT  0x100   /* 0x0f opcode prefix */
 +#define P_REXW 0x200   /* set rex.w = 1 */
 +#define P_REXB_R   0x400   /* REG field as byte register */
 +#define P_REXB_RM  0x800   /* R/M field as byte register */

   static const uint8_t tcg_cond_to_jcc[10] = {
   [TCG_COND_EQ] = JCC_JE,
 @@ -234,17 +235,30 @@ static const uint8_t tcg_cond_to_jcc[10] = {
   [TCG_COND_GTU] = JCC_JA,
   };

 -static inline void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int 
 x)
 +static void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
   {
 -int rex;
 -rex = ((opc  6)  0x8) | ((r  1)  0x4) |
 -((x  2)  2) | ((rm  3)  1);
 -if (rex || (opc  P_REXB)) {
 +int rex = 0;
 +
 +rex |= (opc  P_REXW)  6;/* REX.W */
 +rex |= (r  8)  1;   /* REX.R */
 +rex |= (x  8)  2;   /* REX.X */
 +rex |= (rm  8)  3;  /* REX.B */
 +
 +/* P_REXB_{R,RM} indicates that the given register is the low byte.
 +   For %[abcd]l we need no REX prefix, but for %{si,di,bp,sp}l we do,
 +   as otherwise the encoding indicates %[abcd]h.  Note that the values
 +   that are ORed in merely indicate that the REX byte must be present;
 +   those bits get discarded in output.  */
 +rex |= (r= 4 ? opc  P_REXB_R : 0);
 +rex |= (rm= 4 ? opc  P_REXB_RM : 0);
 +
 +if (rex) {
   tcg_out8(s, rex | 0x40);
   }

 With the above change, rex can be  0xff. Not sure it's not a good idea
 to not have an explicit cast when calling tcg_out8(), even if it
 technically works.

 Same as below.


 -if (opc  P_EXT)
 +if (opc  P_EXT) {
   tcg_out8(s, 0x0f);
 -tcg_out8(s, opc  0xff);
 +}
 +tcg_out8(s, opc);

 What's the reason for removing the '  0xff' part? tcg_out8() takes an 
 uint8_t.

 Yes, and the uint8_t truncates the value just fine.  Is there any  
 particular reason you want to clutter the code with a duplicate  
 truncation?  It might have been reasonable if the function name didn't  
 quite clearly indicate that a single byte was going to be output...


I have to say I don't really like this way of programming. It seems I
agree with gcc people, as they have created the -Wconversion option (ok,
it emits tons of warning within QEMU).

Moreover here, the second truncation removal is totally unreleated to 
this patch.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] [PATCH 1/2] tcg-x86_64: Special-case all 32-bit AND operands.

2010-01-14 Thread Aurelien Jarno
On Thu, Jan 14, 2010 at 08:05:58AM -0800, Richard Henderson wrote:
 On 01/14/2010 07:57 AM, Aurelien Jarno wrote:
 On Tue, Jan 05, 2010 at 04:03:00PM -0800, Richard Henderson wrote:
 This avoids an unnecessary REX.W prefix when dealing with AND
 operands that fit into a 32-bit quantity.  The most common change
 actually seen is movz[wb]q -  movz[wb]l.
 
 Similarly, avoid REXW in ext{8,16}u_i64 tcg opcodes.
 
 Signed-off-by: Richard Hendersonr...@twiddle.net
 ---
   tcg/x86_64/tcg-target.c |   26 --
   1 files changed, 8 insertions(+), 18 deletions(-)
 
 diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
 index 2339091..f584c94 100644
 --- a/tcg/x86_64/tcg-target.c
 +++ b/tcg/x86_64/tcg-target.c
 @@ -426,24 +426,18 @@ static inline void tgen_arithi64(TCGContext *s, int 
 c, int r0, int64_t val)
   } else if ((c == ARITH_ADD  val == -1) || (c == ARITH_SUB  val == 
  1)) {
   /* dec */
   tcg_out_modrm(s, 0xff | P_REXW, 1, r0);
 -} else if (val == (int8_t)val) {
 -tcg_out_modrm(s, 0x83 | P_REXW, c, r0);
 -tcg_out8(s, val);
 -} else if (c == ARITH_AND  val == 0xffu) {
 -/* movzbl */
 -tcg_out_modrm(s, 0xb6 | P_EXT | P_REXW, r0, r0);
 -} else if (c == ARITH_AND  val == 0xu) {
 -/* movzwl */
 -tcg_out_modrm(s, 0xb7 | P_EXT | P_REXW, r0, r0);
   } else if (c == ARITH_AND  val == 0xu) {
   /* 32-bit mov zero extends */
   tcg_out_modrm(s, 0x8b, r0, r0);
 +} else if (c == ARITH_AND  (uint64_t)val= 0xu) {
 +/* AND with no high bits set can use a 32-bit operation.  */
 +tgen_arithi32(s, c, r0, val);
 
 Do we really want to call tgen_arithi32() here, that will redo part of
 the above tests again? It might be better to simply remove the REX.W
 prefix above instead.
 
 Pardon?  Do you mean the inc/dec tests?  Otherwise I don't see what
 above tests again you're talking about.
 
 I am looking to handle more than 0xff, 0x with the new test in
 gen_arithi64 -- 0x1234 is an appropriate mask to shorten to 32-bit
 AND as well.  I have no idea if that answers your question.
 

Yes it does, I missed that fact. Viewing your patch on this side gives
it more sense. I have applied it.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




[Qemu-devel] Re: [PATCH v0 0/8]: VNC events and cleanup

2010-01-14 Thread Luiz Capitulino
On Thu, 14 Jan 2010 17:32:14 +
Daniel P. Berrange berra...@redhat.com wrote:

 On Thu, Jan 14, 2010 at 02:50:51PM -0200, Luiz Capitulino wrote:
   Hi there,
  
   This series contains two VNC related changes. First a small cleanup
  is done in the current 'query-vnc' command _response_, then the following
  QMP events are introduced:
  
  - VNC_CONNECTED: emitted when a VNC client establishes a connection
  - VNC_INITIALIZED: emitted when the VNC session is made active
  - VNC_DISCONNECTED: emitted when the conection is closed
  
   The only issue is the current events documentation. I'm using the
  current format which is quite bad, things will improve when we have
  proper documentation support (being worked out by Markus).
 
 This series looks reasonable to me wrt VNC.
 
 Should we try to think about what we'll do with other network services ?
 eg will we add SPICE_CONNECTED / SPICE_DISCONNECTED in the future ?
 Similar question for chardevs using networking ?

 That's a good question, you think they should converge some way?

 If the chardev API can tell that a fd belongs to a certain subsystem,
then maybe we could move the machinery of connected/disconnected events
down there.

 But that's only an internal refactoring, at first I think we'll have
to add different events for each subsystem we're interested in.




Re: [Qemu-devel] [PATCH] Makefile: Fix message for missing configure

2010-01-14 Thread Aurelien Jarno
On Thu, Jan 14, 2010 at 06:11:43PM +0100, Stefan Weil wrote:
 When make is called without a valid configuration,
 it should tell the user what to do.
 
 Revision 0e8c9214ba1d4128cf92442cd343bc3733478261
 was a regression which resulted in a message
 which was no longer user friendly
 (reported by Aurelien Jarno).
 
 This patch restores the old behaviour.

Thanks for the quick reaction, applied.

 Cc: Aurelien Jarno aurel...@aurel32.net
 Signed-off-by: Stefan Weil w...@mail.berlios.de
 ---
  Makefile |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/Makefile b/Makefile
 index fa7f851..b1bbe6d 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -75,7 +75,9 @@ SUBDIR_RULES=$(patsubst %,subdir-%, $(TARGET_DIRS))
  subdir-%: $(GENERATED_HEADERS)
   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $* V=$(V) 
 TARGET_DIR=$*/ all,)
  
 +ifneq ($(wildcard config-host.mak),)
  include $(SRC_PATH)/Makefile.objs
 +endif
  
  $(common-obj-y): $(GENERATED_HEADERS)
  $(filter %-softmmu,$(SUBDIR_RULES)): $(common-obj-y)
 -- 
 1.6.5
 
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] [PATCH] Fix a typo in 'P' packet processing for M68K.

2010-01-14 Thread Aurelien Jarno
On Thu, Jan 14, 2010 at 09:08:00AM -0800, Kazu Hirata wrote:
 Hi,
 
 Attached is a patch to fix a typo in 'P' packet processing for M68K.
 
 Without this patch, QEMU fails to honor GDB's P packets from GDB
 (writing to registers) for the address registers (A0 - A7).
 
 The problem is because of an obvious typo.  Notice that the second
 if condition is meant to be n  16 in:
 
   if (n  8) {
 :
   } else if (n  8) {
 
 Signed-off-by: Kazu Hirata k...@codesourcery.com

Thanks, applied.

 ---
  gdbstub.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/gdbstub.c b/gdbstub.c
 index 6180171..80477be 100644
 --- a/gdbstub.c
 +++ b/gdbstub.c
 @@ -1014,7 +1014,7 @@ static int cpu_gdb_write_register(CPUState *env, 
 uint8_t *mem_buf, int n)
  if (n  8) {
  /* D0-D7 */
  env-dregs[n] = tmp;
 -} else if (n  8) {
 +} else if (n  16) {
  /* A0-A7 */
  env-aregs[n - 8] = tmp;
  } else {
 -- 
 1.6.2.4
 
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net




Re: [Qemu-devel] Static analysis using clang on the x86_64 target

2010-01-14 Thread Blue Swirl
On Thu, Jan 14, 2010 at 3:09 AM, Amit Shah amit.s...@redhat.com wrote:
 On (Wed) Jan 13 2010 [19:08:11], Blue Swirl wrote:

 Thanks. I fixed the warnings related to Sparc32. Were there really no
 new warnings for Sparc64?

 Looks like it; vl.c gets reported three times at the same locations so 3
 arches have been compiled.

 My test machine is down ATM, I can confirm later when it's up.

 BTW for the patch

 commit 884a0c7677cf8431d2a632673914994c2e01673d

    pcnet: remove dead nested assignment, spotted by clang

 diff --git a/hw/pcnet.c b/hw/pcnet.c
 index 91d106d..44b5b31 100644
 --- a/hw/pcnet.c
 +++ b/hw/pcnet.c
 @@ -1608,7 +1608,7 @@ static void pcnet_aprom_writeb(void *opaque,
 uint32_t addr, uint32_t val)
  static uint32_t pcnet_aprom_readb(void *opaque, uint32_t addr)
  {
     PCNetState *s = opaque;
 -    uint32_t val = s-prom[addr = 15];
 +    uint32_t val = s-prom[addr  15];
  #ifdef PCNET_DEBUG
     printf(pcnet_aprom_readb addr=0x%08x val=0x%02x\n, addr, val);
  #endif


 if debugging is enabled, addr will now print a different value than
 earlier.

It should be harmless, pcnet_aprom_readb doesn't do addr  15 either.
It's probably a relic from the time when the devices were passed the
MMIO address instead of offset.




[Qemu-devel] [PATCH] QMP: Save default control monitor for emitting async events

2010-01-14 Thread Adam Litke
When using a control/QMP monitor in tandem with a regular monitor, asynchronous
messages can get lost depending on the order of the QEMU program arguments.
QEMU events issued by monitor_protocol_event() always go to cur_mon.  If the
user monitor was specified on the command line first (or it has ,default), the
message will be directed to the user monitor (not the QMP monitor).

One solution is to save the default QMP session in another monitor pointer (ala
cur_mon) and always direct asynchronous events to that monitor...

Signed-off-by: Adam Litke a...@us.ibm.com

diff --git a/monitor.c b/monitor.c
index 134ed15..794f6ba 100644
--- a/monitor.c
+++ b/monitor.c
@@ -128,6 +128,7 @@ static const mon_cmd_t mon_cmds[];
 static const mon_cmd_t info_cmds[];
 
 Monitor *cur_mon = NULL;
+Monitor *cur_mon_control = NULL;
 
 static void monitor_command_cb(Monitor *mon, const char *cmdline,
void *opaque);
@@ -334,11 +335,11 @@ void monitor_protocol_event(MonitorEvent event, QObject 
*data)
 {
 QDict *qmp;
 const char *event_name;
-Monitor *mon = cur_mon;
+Monitor *mon = cur_mon_control;
 
 assert(event  QEVENT_MAX);
 
-if (!monitor_ctrl_mode(mon))
+if (!mon)
 return;
 
 switch (event) {
@@ -4283,6 +4284,9 @@ void monitor_init(CharDriverState *chr, int flags)
 QLIST_INSERT_HEAD(mon_list, mon, entry);
 if (!cur_mon || (flags  MONITOR_IS_DEFAULT))
 cur_mon = mon;
+if (!cur_mon_control || (flags  MONITOR_IS_DEFAULT))
+if (flags  MONITOR_USE_CONTROL)
+cur_mon_control = mon;
 }
 
 static void bdrv_password_cb(Monitor *mon, const char *password, void *opaque)
diff --git a/monitor.h b/monitor.h
index 556239a..a343589 100644
--- a/monitor.h
+++ b/monitor.h
@@ -7,6 +7,7 @@
 #include block.h
 
 extern Monitor *cur_mon;
+extern Monitor *cur_mon_control;
 
 /* flags for monitor_init */
 #define MONITOR_IS_DEFAULT0x01
diff --git a/qemu-tool.c b/qemu-tool.c
index 18b48af..cfe03d6 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -34,6 +34,7 @@ void qemu_service_io(void)
 }
 
 Monitor *cur_mon;
+Monitor *cur_mon_control;
 
 void monitor_printf(Monitor *mon, const char *fmt, ...)
 {


-- 
Thanks,
Adam





[Qemu-devel] [PATCH] tcg-x86_64: Avoid unnecessary REX.B prefixes.

2010-01-14 Thread Richard Henderson
The existing P_REXB internal opcode flag unconditionally emits
the REX prefix.  Technically it's not needed if the register in
question is %al, %bl, %cl, %dl.

Eliding the prefix requires splitting the P_REXB flag into two,
in order to indicate whether the byte register in question is
in the REG or the R/M field.  Within TCG, the byte register is
in the REG field only for stores.

Signed-off-by: Richard Henderson r...@twiddle.net
---
 tcg/x86_64/tcg-target.c |   46 ++
 1 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/tcg/x86_64/tcg-target.c b/tcg/x86_64/tcg-target.c
index 8c7e738..cbaabef 100644
--- a/tcg/x86_64/tcg-target.c
+++ b/tcg/x86_64/tcg-target.c
@@ -217,9 +217,10 @@ static inline int tcg_target_const_match(tcg_target_long 
val,
 #define JCC_JLE 0xe
 #define JCC_JG  0xf
 
-#define P_EXT   0x100 /* 0x0f opcode prefix */
-#define P_REXW  0x200 /* set rex.w = 1 */
-#define P_REXB  0x400 /* force rex use for byte registers */
+#define P_EXT  0x100   /* 0x0f opcode prefix */
+#define P_REXW 0x200   /* set rex.w = 1 */
+#define P_REXB_R   0x400   /* REG field as byte register */
+#define P_REXB_RM  0x800   /* R/M field as byte register */
   
 static const uint8_t tcg_cond_to_jcc[10] = {
 [TCG_COND_EQ] = JCC_JE,
@@ -234,16 +235,29 @@ static const uint8_t tcg_cond_to_jcc[10] = {
 [TCG_COND_GTU] = JCC_JA,
 };
 
-static inline void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
+static void tcg_out_opc(TCGContext *s, int opc, int r, int rm, int x)
 {
-int rex;
-rex = ((opc  6)  0x8) | ((r  1)  0x4) | 
-((x  2)  2) | ((rm  3)  1);
-if (rex || (opc  P_REXB)) {
-tcg_out8(s, rex | 0x40);
+int rex = 0;
+
+rex |= (opc  P_REXW)  6;/* REX.W */
+rex |= (r  8)  1;   /* REX.R */
+rex |= (x  8)  2;   /* REX.X */
+rex |= (rm  8)  3;  /* REX.B */
+
+/* P_REXB_{R,RM} indicates that the given register is the low byte.
+   For %[abcd]l we need no REX prefix, but for %{si,di,bp,sp}l we do,
+   as otherwise the encoding indicates %[abcd]h.  Note that the values
+   that are ORed in merely indicate that the REX byte must be present;
+   those bits get discarded in output.  */
+rex |= opc  (r = 4 ? P_REXB_R : 0);
+rex |= opc  (rm = 4 ? P_REXB_RM : 0);
+
+if (rex) {
+tcg_out8(s, (uint8_t)(rex | 0x40));
 }
-if (opc  P_EXT)
+if (opc  P_EXT) {
 tcg_out8(s, 0x0f);
+}
 tcg_out8(s, opc  0xff);
 }
 
@@ -408,7 +422,7 @@ static inline void tgen_arithi32(TCGContext *s, int c, int 
r0, int32_t val)
 tcg_out8(s, val);
 } else if (c == ARITH_AND  val == 0xffu) {
 /* movzbl */
-tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, r0, r0);
+tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB_RM, r0, r0);
 } else if (c == ARITH_AND  val == 0xu) {
 /* movzwl */
 tcg_out_modrm(s, 0xb7 | P_EXT, r0, r0);
@@ -776,7 +790,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args,
 switch(opc) {
 case 0:
 /* movzbl */
-tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, TCG_REG_RSI, data_reg);
+tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB_RM, TCG_REG_RSI, data_reg);
 break;
 case 1:
 /* movzwl */
@@ -829,7 +843,7 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg 
*args,
 switch(opc) {
 case 0:
 /* movb */
-tcg_out_modrm_offset(s, 0x88 | P_REXB, data_reg, r0, offset);
+tcg_out_modrm_offset(s, 0x88 | P_REXB_R, data_reg, r0, offset);
 break;
 case 1:
 if (bswap) {
@@ -964,7 +978,7 @@ static inline void tcg_out_op(TCGContext *s, int opc, const 
TCGArg *args,
 case INDEX_op_st8_i32:
 case INDEX_op_st8_i64:
 /* movb */
-tcg_out_modrm_offset(s, 0x88 | P_REXB, args[0], args[1], args[2]);
+tcg_out_modrm_offset(s, 0x88 | P_REXB_R, args[0], args[1], args[2]);
 break;
 case INDEX_op_st16_i32:
 case INDEX_op_st16_i64:
@@ -1161,7 +1175,7 @@ static inline void tcg_out_op(TCGContext *s, int opc, 
const TCGArg *args,
 break;
 
 case INDEX_op_ext8s_i32:
-tcg_out_modrm(s, 0xbe | P_EXT | P_REXB, args[0], args[1]);
+tcg_out_modrm(s, 0xbe | P_EXT | P_REXB_RM, args[0], args[1]);
 break;
 case INDEX_op_ext16s_i32:
 tcg_out_modrm(s, 0xbf | P_EXT, args[0], args[1]);
@@ -1177,7 +1191,7 @@ static inline void tcg_out_op(TCGContext *s, int opc, 
const TCGArg *args,
 break;
 case INDEX_op_ext8u_i32:
 case INDEX_op_ext8u_i64:
-tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB, args[0], args[1]);
+tcg_out_modrm(s, 0xb6 | P_EXT | P_REXB_RM, args[0], args[1]);
 break;
 case INDEX_op_ext16u_i32:
 case INDEX_op_ext16u_i64:
-- 
1.6.5.2





[Qemu-devel] [PATCH] linux-user: Align mmap memory to the target page size.

2010-01-14 Thread Richard Henderson
Previously, mmap_find_vma could return addresses not properly aligned
to the target page size.  This of course led to all sorts of odd
problems down the road.

The trivial fix, to simply reject the unaligned address and continue
searching the address space by increments of one page, is not a good
idea when there's a 64-bit address space involved.  The kernel may well
continue to return the last available address which we've already
rejected while we search upward from e.g. 2**42 from 2**64.

This patch uses a more complex search algorithm that takes the result
of the previous allocation into account.  We normally search upward,
but notice 2 consecutive results and start searching downward instead.

Signed-off-by: Richard Henderson r...@twiddle.net
---
 linux-user/main.c |7 +---
 linux-user/mmap.c |   71 ++--
 2 files changed, 64 insertions(+), 14 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index a0d8ce7..7db9fc3 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2725,12 +2725,9 @@ int main(int argc, char **argv, char **envp)
 /*
  * Read in mmap_min_addr kernel parameter.  This value is used
  * When loading the ELF image to determine whether guest_base
- * is needed.
- *
- * When user has explicitly set the quest base, we skip this
- * test.
+ * is needed.  It is also used in mmap_find_vma.
  */
-if (!have_guest_base) {
+{
 FILE *fp;
 
 if ((fp = fopen(/proc/sys/vm/mmap_min_addr, r)) != NULL) {
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 144fb7c..b92fdc4 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -281,8 +281,9 @@ unsigned long last_brk;
  */
 abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 {
-void *ptr;
+void *ptr, *prev;
 abi_ulong addr;
+int wrapped, repeat;
 
 size = HOST_PAGE_ALIGN(size);
 start = qemu_host_page_mask;
@@ -292,8 +293,11 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 start = mmap_next_start;
 
 addr = start;
+wrapped = (start == 0);
+repeat = 0;
+prev = 0;
 
-for(;;) {
+for (;; prev = ptr) {
 /*
  * Reserve needed memory area to avoid a race.
  * It should be discarded using:
@@ -305,20 +309,69 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, -1, 0);
 
 /* ENOMEM, if host address space has no memory */
-if (ptr == MAP_FAILED)
+if (ptr == MAP_FAILED) {
 return (abi_ulong)-1;
+}
 
-/* If address fits target address space we've found what we need */
-if ((unsigned long)ptr + size - 1 = (abi_ulong)-1)
+/* Count the number of sequential returns of the same address.
+   This is used to modify the search algorithm below.  */
+repeat = (ptr == prev ? repeat + 1 : 0);
+
+if ((unsigned long)ptr  ~TARGET_PAGE_MASK) {
+/* The address is not properly aligned for the target.  */
+switch (repeat) {
+case 0:
+/* Assume the result that the kernel gave us is the
+   first with enough free space, so start again at the
+   next higher target page.  */
+addr = TARGET_PAGE_ALIGN((unsigned long)ptr);
+break;
+case 1:
+/* Sometimes the kernel decides to perform the allocation
+   at the top end of memory instead.  Notice this via
+   sequential allocations that result in the same address.  */
+/* ??? This can be exacerbated by a successful allocation
+   at the top of memory on one round, and storing that
+   result in mmap_next_start.  The next allocation is sure
+   to start at an address that's going to fail.  */
+addr = (unsigned long)ptr  TARGET_PAGE_MASK;
+break;
+case 2:
+/* Start over at low memory.  */
+addr = 0;
+break;
+default:
+/* Fail.  This unaligned block must be the only one left.  */
+addr = -1;
+break;
+}
+} else if ((unsigned long)ptr + size - 1 = (abi_ulong)-1) {
 break;
+} else {
+/* Since the result the kernel gave didn't fit, start
+   again at low memory.  If any repetition, fail.  */
+addr = (repeat ? -1 : 0);
+}
 
-/* Unmap and try again with new page */
+/* Unmap and try again.  */
 munmap(ptr, size);
-addr += qemu_host_page_size;
 
-/* ENOMEM if we check whole of target address space */
-if (addr == start)
+/* ENOMEM if we checked the whole of the target address space.  */
+if (addr == -1) {
 return 

Re: [Qemu-devel] [PATCH 2/2] tcg-x86_64: Avoid unnecessary REX.B prefixes.

2010-01-14 Thread Jamie Lokier
Richard Henderson wrote:
 On 01/14/2010 08:10 AM, Aurelien Jarno wrote:
 With the above change, rex can be  0xff. Not sure it's not a good idea
 to not have an explicit cast when calling tcg_out8(), even if it
 technically works.

 What's the reason for removing the '  0xff' part? tcg_out8() takes an 
 uint8_t.
 
 Yes, and the uint8_t truncates the value just fine.  Is there any 
 particular reason you want to clutter the code with a duplicate 
 truncation?  It might have been reasonable if the function name didn't 
 quite clearly indicate that a single byte was going to be output...

The  0xff makes it clear that rex  0xff is intentional; that you
have thought about it.

Otherwise it looks like rex  0xff might be unintentional.  Anyone can
check the code isn't mistaken, but it's better if it doesn't *look*
like a mistake.  After all, there have been mistakes in this sort of
code elsewhere many times.

In this sense, I think it's not cluttering; it's removing excessive
subtlety.

I would hope that GCC optimises the  0xff away.

-- Jamie




[Qemu-devel] Re: [PATCH] rtl8139: fix clang reporting unused assignment of VLAN tagging data

2010-01-14 Thread Paolo Bonzini



diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 1f4f585..f04dd54 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -1909,6 +1909,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)

  cpu_physical_memory_read(cplus_tx_ring_desc,(uint8_t *)val, 4);
  txdw0 = le32_to_cpu(val);
+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
  cpu_physical_memory_read(cplus_tx_ring_desc+4,  (uint8_t *)val, 4);
  txdw1 = le32_to_cpu(val);
  cpu_physical_memory_read(cplus_tx_ring_desc+8,  (uint8_t *)val, 4);
@@ -1920,6 +1921,9 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
 descriptor,
 txdw0, txdw1, txbufLO, txbufHI));

+/* TODO: the following discard cast should clean clang analyzer output */
+(void)txdw1;


I don't like this, why not comment it out like here:


+/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
  //val = cpu_to_le32(txdw1);
  //cpu_physical_memory_write(cplus_tx_ring_desc+4,val, 4);


(and maybe change this one as well to #if 0...#endif, I don't know).

Paolo




Re: [Qemu-devel] [PATCH 4/8] VNC: Add 'family' key

2010-01-14 Thread Gerd Hoffmann

+static QString *get_sock_family(const struct sockaddr_storage *sa)
+{
+const char *name;
+
+switch (sa-ss_family)
+{
+case AF_INET:
+name = ipv4;
+break;
+case AF_INET6:
+name = ipv6;
+break;
+default:
+name = unknown;
+break;
+}
+
+return qstring_from_str(name);
+}


qemu-socket has inet_strfamily() already.  You might want to un-static 
that one, then simply do


  return qstring_from_str(inet_strfamily(sa-ss_family));

cheers,
  Gerd