[Qemu-devel] [PATCH] MIPS Initial support of Godson-3a multicore CPU

2010-12-07 Thread Jin Guojie
Signed-off-by: Jin Guojie jinguo...@loongson.cn
Reviewed-by: Gao Xiang gaoxi...@ict.ac.cn
Reviewed-by: Chen Huacai zltjiang...@gmail.com

 A patch for Godson-3a CPU simulation.
 Godson-3a is a newly developed MIPS-III like, multicore CPU by ICT, China.
 We believe this patch could be helpful for other Godson developers.
 For you review. Any comment is welcomed.

Jin Guojie
www.loongson.cn
---
 Makefile.target  |2 +-
 hw/mips_godson3a.c   |  507 ++
 target-mips/mips-defs.h  |4 +-
 target-mips/translate_init.c |   26 +++
 4 files changed, 536 insertions(+), 3 deletions(-)
 create mode 100755 hw/mips_godson3a.c

diff --git a/Makefile.target b/Makefile.target
index 91e6e74..8f29aeb 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -230,7 +230,7 @@ obj-ppc-y += xilinx_timer.o
 obj-ppc-y += xilinx_uartlite.o
 obj-ppc-y += xilinx_ethlite.o

-obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
+obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o mips_godson3a.o
 obj-mips-y += mips_addr.o mips_timer.o mips_int.o
 obj-mips-y += vga.o i8259.o
 obj-mips-y += g364fb.o jazz_led.o
diff --git a/hw/mips_godson3a.c b/hw/mips_godson3a.c
new file mode 100755
index 000..4085db2
--- /dev/null
+++ b/hw/mips_godson3a.c
@@ -0,0 +1,507 @@
+/*
+ * QEMU godson 3a developing board support
+ *
+ * Copyright (c) 2009 Gao Xiang (gaoxi...@ict.ac.cn)
+ * Copyright (c) 2010 Jin Guojie (jinguo...@loongson.cn)
+ * This code is licensed under the GNU GPL v2.
+ */
+
+/*
+ * Godson 3a developing board is based on ICT/ST Godson-3a.
+ * Godson-3a CPU is a MIPS-III like, multicore processor.
+ * It can be configured to contain 4 or 8 cores. Every 4
+ * cores are grouped into one on-chip 'node'. SMP mechanism
+ * is supported by Godson IPI(inter-processors interrupt)
+ * specification.
+ *
+ * Godson 3a CPU intro:
+ *   http://en.wikipedia.org/wiki/Loongson
+ *
+ * Godson 3a user manual:
+ *   http://www.loongsondeveloper.com/doc/Loongson3AUserGuide.pdf
+ */
+#include hw.h
+#include mips.h
+#include pc.h
+#include isa.h
+#include net.h
+#include sysemu.h
+#include boards.h
+#include ide.h
+#include mips-bios.h
+#include elf.h
+#include loader.h
+#include blockdev.h
+#include mips_cpudevs.h
+#include mc146818rtc.h
+
+static target_ulong PHYS_TO_VIRT(target_ulong phys)
+{
+if (smp_cpus  1)
+return ((phys) | 0x9800ULL);
+else
+return ((phys) | ~(target_ulong)0x7fff);
+}
+
+#define VIRT_TO_PHYS_ADDEND (-((int64_t)(int32_t)0x8000))
+
+#define MAX_IDE_BUS 2
+
+static const int ide_iobase[2] = { 0x1f0, 0x170 };
+static const int ide_iobase2[2] = { 0x3f6, 0x376 };
+static const int ide_irq[2] = { 14, 15 };
+
+static int serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
+
+static PITState *pit; /* PIT i8254 */
+
+/* i8254 PIT is attached to the IRQ0 at PIC i8259 */
+
+static struct _loaderparams {
+int ram_size;
+const char *kernel_filename;
+const char *kernel_cmdline;
+const char *initrd_filename;
+} loaderparams;
+
+static void mips_qemu_writel (void *opaque, target_phys_addr_t addr,
+ uint32_t val)
+{
+if ((addr  0x) == 0  val == 42)
+qemu_system_reset_request();
+else if ((addr  0x) == 4  val == 42)
+qemu_system_shutdown_request();
+}
+
+static uint32_t mips_qemu_readl (void *opaque, target_phys_addr_t addr)
+{
+return 0;
+}
+
+static CPUWriteMemoryFunc *mips_qemu_write[] = {
+mips_qemu_writel,
+mips_qemu_writel,
+mips_qemu_writel,
+};
+
+static CPUReadMemoryFunc *mips_qemu_read[] = {
+mips_qemu_readl,
+mips_qemu_readl,
+mips_qemu_readl,
+};
+
+static int mips_qemu_iomemtype = 0;
+
+typedef struct ResetData {
+CPUState *env;
+uint64_t vector;
+} ResetData;
+
+static int64_t load_kernel (CPUState *env)
+{
+int64_t entry, kernel_high;
+long kernel_size, initrd_size, params_size;
+ram_addr_t initrd_offset;
+uint32_t *params_buf;
+int big_endian;
+
+#ifdef TARGET_WORDS_BIGENDIAN
+big_endian = 1;
+#else
+big_endian = 0;
+#endif
+
+kernel_size = load_elf(loaderparams.kernel_filename,
cpu_mips_kseg0_to_phys, NULL,
+   (uint64_t *)entry, NULL, (uint64_t *)kernel_high,
+  big_endian, ELF_MACHINE, 1);
+if (kernel_size = 0) {
+if ((entry  ~0x7fffULL) == 0x8000)
+entry = (int32_t)entry;
+env-active_tc.PC = entry;
+env = first_cpu;
+} else {
+fprintf(stderr, qemu: could not load kernel '%s'\n,
+loaderparams.kernel_filename);
+exit(1);
+}
+
+/* load initrd */
+initrd_size = 0;
+initrd_offset = 0;
+if (loaderparams.initrd_filename) {
+initrd_size = get_image_size (loaderparams.initrd_filename);
+   if (initrd_size  0) {
+   if(initrd_size  0x1000)
+   initrd_offset = 0x100;
+  

[Qemu-devel] [PATCH] fix do_socket(): type conversion for all targets

2010-12-07 Thread Jin Guojie
Signed-off-by: Jin Guojie jinguo...@loongson.cn
Signed-off-by: Yin Yixiao yinyix...@ict.ac.cn
Reviewed-by: Gao Xiang gaoxi...@ict.ac.cn

 A patch for do_socket() in Linux user mode.
 The original code only handles socket constants conversion for target MIPS.
 We encountered this problem when emulating X86 apps on MIPS machines.
 This patch extends the scope to all targets and fix the problem.
 For your review, any comment is strongly welcomed.

Jin Guojie
www.loongson.cn
---
 linux-user/socket.h  |   10 ++
 linux-user/syscall.c |3 +--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/linux-user/socket.h b/linux-user/socket.h
index 93d4782..f24b44c 100644
--- a/linux-user/socket.h
+++ b/linux-user/socket.h
@@ -144,4 +144,14 @@

#define TARGET_SO_PEERSEC  31

+   enum sock_type {
+  TARGET_SOCK_DGRAM   = 2,
+  TARGET_SOCK_STREAM  = 1,
+  TARGET_SOCK_RAW = 3,
+  TARGET_SOCK_RDM = 4,
+  TARGET_SOCK_SEQPACKET   = 5,
+  TARGET_SOCK_DCCP= 6,
+  TARGET_SOCK_PACKET  = 10,
+   };
+
 #endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c3e8706..544698a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1568,7 +1568,6 @@ static abi_long unlock_iovec(struct iovec *vec,
abi_ulong target_addr,
 /* do_socket() Must return target values and target errnos. */
 static abi_long do_socket(int domain, int type, int protocol)
 {
-#if defined(TARGET_MIPS)
 switch(type) {
 case TARGET_SOCK_DGRAM:
 type = SOCK_DGRAM;
@@ -1589,7 +1588,7 @@ static abi_long do_socket(int domain, int type,
int protocol)
 type = SOCK_PACKET;
 break;
 }
-#endif
+
 if (domain == PF_NETLINK)
 return -EAFNOSUPPORT; /* do not NETLINK socket connections possible */
 return get_errno(socket(domain, type, protocol));
-- 
1.5.2.3



[Qemu-devel] [PATCH 1/4] qemu-option: Don't reinvent append_option_parameters()

2010-12-07 Thread Stefan Hajnoczi
parse_option_parameters() may need to create a new option parameter list
from a template list.  Use append_option_parameters() instead of
duplicating the code.

Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
 qemu-option.c |9 +
 1 files changed, 1 insertions(+), 8 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index 1f8f41a..e380fc1 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -416,20 +416,13 @@ QEMUOptionParameter *parse_option_parameters(const char 
*param,
 char value[256];
 char *param_delim, *value_delim;
 char next_delim;
-size_t num_options;
 
 if (list == NULL) {
 return NULL;
 }
 
 if (dest == NULL) {
-// Count valid options
-num_options = count_option_parameters(list);
-
-// Create a copy of the option list to fill in values
-dest = qemu_mallocz((num_options + 1) * sizeof(QEMUOptionParameter));
-allocated = dest;
-memcpy(dest, list, (num_options + 1) * sizeof(QEMUOptionParameter));
+dest = allocated = append_option_parameters(NULL, list);
 }
 
 while (*param) {
-- 
1.7.2.3




[Qemu-devel] [PATCH 0/4] qemu-img: Fail creation if backing format is invalid

2010-12-07 Thread Stefan Hajnoczi
This patch series adds a check to validate the backing format before creating
an image file.  This ensures we provide a clear error message as early as
possible when an unsupported format is used.

The first three patches clean up code on the way and the last patch makes the
actual backing format validation change.




[Qemu-devel] [PATCH 3/4] qemu-img: Free option parameter lists in img_create()

2010-12-07 Thread Stefan Hajnoczi
Free option parameter lists in the img_create() error return path.

Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
 qemu-img.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 5b6e648..23bb7dc 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -439,8 +439,6 @@ static int img_create(int argc, char **argv)
 puts();
 
 ret = bdrv_create(drv, filename, param);
-free_option_parameters(create_options);
-free_option_parameters(param);
 
 if (ret  0) {
 if (ret == -ENOTSUP) {
@@ -452,6 +450,8 @@ static int img_create(int argc, char **argv)
 }
 }
 out:
+free_option_parameters(create_options);
+free_option_parameters(param);
 if (ret) {
 return 1;
 }
-- 
1.7.2.3




[Qemu-devel] [PATCH 4/4] qemu-img: Fail creation if backing format is invalid

2010-12-07 Thread Stefan Hajnoczi
The qemu-img create command should check the backing format to ensure
only image files with valid backing formats are created.  By checking in
qemu-img.c we can print a useful error message.

Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
 qemu-img.c |   22 --
 1 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 23bb7dc..b10f363 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -300,6 +300,7 @@ static int img_create(int argc, char **argv)
 const char *base_filename = NULL;
 BlockDriver *drv, *proto_drv;
 QEMUOptionParameter *param = NULL, *create_options = NULL;
+QEMUOptionParameter *backing_fmt = NULL;
 char *options = NULL;
 
 flags = 0;
@@ -390,14 +391,22 @@ static int img_create(int argc, char **argv)
 goto out;
 }
 
+backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
+if (backing_fmt  backing_fmt-value.s) {
+if (!bdrv_find_format(backing_fmt-value.s)) {
+error(Unknown backing file format '%s',
+  backing_fmt-value.s);
+ret = -1;
+goto out;
+}
+}
+
 // The size for the image must always be specified, with one exception:
 // If we are using a backing file, we can obtain the size from there
 if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == -1) {
 
 QEMUOptionParameter *backing_file =
 get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
-QEMUOptionParameter *backing_fmt =
-get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
 
 if (backing_file  backing_file-value.s) {
 BlockDriverState *bs;
@@ -406,14 +415,7 @@ static int img_create(int argc, char **argv)
 char buf[32];
 
 if (backing_fmt  backing_fmt-value.s) {
- if (bdrv_find_format(backing_fmt-value.s)) {
- fmt = backing_fmt-value.s;
-} else {
- error(Unknown backing file format '%s',
-backing_fmt-value.s);
- ret = -1;
- goto out;
-}
+fmt = backing_fmt-value.s;
 }
 
 bs = bdrv_new_open(backing_file-value.s, fmt, BDRV_O_FLAGS);
-- 
1.7.2.3




[Qemu-devel] [PATCH 2/4] qemu-option: Fix parse_option_parameters() documentation typo

2010-12-07 Thread Stefan Hajnoczi
Yoda said, list is the templace is.  Fix this.

Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
 qemu-option.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index e380fc1..65db542 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -394,8 +394,8 @@ QEMUOptionParameter 
*append_option_parameters(QEMUOptionParameter *dest,
 /*
  * Parses a parameter string (param) into an option list (dest).
  *
- * list is the templace is. If dest is NULL, a new copy of list is created for
- * it. If list is NULL, this function fails.
+ * list is the template option list. If dest is NULL, a new copy of list is
+ * created. If list is NULL, this function fails.
  *
  * A parameter string consists of one or more parameters, separated by commas.
  * Each parameter consists of its name and possibly of a value. In the latter
-- 
1.7.2.3




Re: [Qemu-devel] [PATCH 1/6] [RFC] Emulation of GRLIB GPTimer as defined in GRLIB IP Core User's Manual.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 06:12 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  hw/grlib_gptimer.c |  448 
  1 files changed, 448 insertions(+), 0 deletions(-)

diff --git a/hw/grlib_gptimer.c b/hw/grlib_gptimer.c
new file mode 100644
index 000..41edbe4
--- /dev/null
+++ b/hw/grlib_gptimer.c
@@ -0,0 +1,448 @@
+/*
+ * QEMU GRLIB GPTimer Emulator
+ *
+ * Copyright (c) 2010 AdaCore
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include sysbus.h
+#include qemu-timer.h
+
+#include grlib.h
+
+/* #define DEBUG_TIMER */


The usual convention is
//#define DEBUG_TIMER
for easy editing.



Actually, it's easier for me with the /* */, but OK.


However, very often the much more powerful tracepoints can replace
debug statements.


+
+#ifdef DEBUG_TIMER
+#define DPRINTF(fmt, ...)   \
+do { printf(GPTIMER:  fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...)
+#endif
+
+#define UNIT_REG_SIZE16 /* Size of memory mapped regs for the unit */
+#define GPTIMER_REG_SIZE 16 /* Size of memory mapped regs for a GPTimer */
+
+#define GPTIMER_MAX_TIMERS 8
+
+/* GPTimer Config register fields */
+#define GPTIMER_ENABLE  (1  0)
+#define GPTIMER_RESTART (1  1)
+#define GPTIMER_LOAD(1  2)
+#define GPTIMER_INT_ENABLE  (1  3)
+#define GPTIMER_INT_PENDING (1  4)
+#define GPTIMER_CHAIN   (1  5) /* Not supported */
+#define GPTIMER_DEBUG_HALT  (1  6) /* Not supported */
+
+/* Memory mapped register offsets */
+#define SCALER_OFFSET 0x00
+#define SCALER_RELOAD_OFFSET  0x04
+#define CONFIG_OFFSET 0x08
+#define COUNTER_OFFSET0x00
+#define COUNTER_RELOAD_OFFSET 0x04
+#define TIMER_BASE0x10
+
+typedef struct GPTimer GPTimer;
+typedef struct GPTimerUnit GPTimerUnit;
+
+struct GPTimer
+{
+QEMUBH *bh;
+struct ptimer_state *ptimer;
+
+qemu_irq irq;
+int  id;
+GPTimerUnit *unit;
+
+/* registers */
+uint32_t counter;
+uint32_t reload;
+uint32_t config;
+};
+
+struct GPTimerUnit
+{
+SysBusDevice  busdev;
+
+uint32_t nr_timers; /* Number of timers available */
+uint32_t freq_hz;   /* System frequency */
+uint32_t irq_line;  /* Base irq line */
+
+GPTimer *timers;
+
+/* registers */
+uint32_t scaler;
+uint32_t reload;
+uint32_t config;
+};
+
+DeviceState *grlib_gptimer_create(target_phys_addr_t  base,
+  uint32_tnr_timers,
+  uint32_tfreq,
+  qemu_irq   *cpu_irqs,
+  int base_irq)


This function belongs to leon3.c.


I don't see why. GPTimer is a peripheral and you may want to use it in 
an other system.



+{
+DeviceState *dev;
+int i;
+_ir
+dev = qdev_create(NULL, grlib,gptimer);
+qdev_prop_set_uint32(dev, nr-timers, nr_timers);
+qdev_prop_set_uint32(dev, frequency, freq);
+qdev_prop_set_uint32(dev, irq-line, base_irq);


Base irq is not device property, but part of board configuration. Thus
leon3.c should just passcpu_irqs[base_irq] to this function.



I need this property to put the IRQ line in the configuration register. 
Is there a way to get this number from a qemu_irq structure?



+
+if (qdev_init(dev)) {
+return NULL;
+}
+
+sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
+
+for (i = 0; i  nr_timers; i++)
+sysbus_connect_irq(sysbus_from_qdev(dev), i, cpu_irqs[base_irq + i]);
+
+return dev;
+}
+
+static void grlib_gptimer_enable(GPTimer *timer)
+{
+assert(timer != NULL);
+
+DPRINTF(%s id:%d\n, __func__, timer-id);
+
+ptimer_stop(timer-ptimer);
+
+if (!(timer-config  

Re: [Qemu-devel] [PATCH, RFT] monitor: implement x86 info tlb for PAE and long modes

2010-12-07 Thread Ian Campbell
On Mon, 2010-12-06 at 18:41 +, Blue Swirl wrote:
 On Mon, Dec 6, 2010 at 1:05 PM, Ian Campbell i...@hellion.org.uk wrote:
  On Mon, 2010-12-06 at 12:12 +0100, Alexander Graf wrote:
  On 05.12.2010, at 17:25, Blue Swirl wrote:
 
   'info tlb' didn't show correct information for PAE mode and
   x86_64 long mode.
  
   Implement the missing modes. Also print NX bit for PAE and long modes.
   Fix off-by-one error in 32 bit mode mask.
  
   Signed-off-by: Blue Swirl blauwir...@gmail.com
   ---
  
   I didn't find an OS that enabled PAE, please test and report.
 
  Xen does. Just take a random recent xen kernel and run it with -kernel :).
 
  In addition AFAIK recent 32 bit Fedora is PAE enabled by default (so is
  RHEL6?). Debian also supplies a -686-bigmem kernel flavour which is
  their name for PAE enabled.
 
 At least Fedora 14 installation CD kernel does not enable PAE with 5G
 of memory (enabled for i386 by adjusting target_phys_bits in
 configure).

Hm, seems like I was mistaken and it was just RHEL6 which changed the
default.

FWIW Fedora-14-i386-DVD.iso contains /images/pxeboot/vmlinuz-PAE and
initrd-PAE.img which might be useful to you, see also
http://download.fedora.redhat.com/pub/fedora/linux/releases/14/Fedora/i386/os/images/pxeboot/
 .

Ian.

-- 
Ian Campbell
Current Noise: Sabbat - Behind The Crooked Cross (Live)

Lighten up, while you still can,
Don't even try to understand,
Just find a place to make your stand,
And take it easy.
-- The Eagles, Take It Easy




Re: [Qemu-devel] [PATCH, RFT] monitor: implement x86 info tlb for PAE and long modes

2010-12-07 Thread Alexander Graf

On 06.12.2010, at 19:38, Blue Swirl wrote:

 On Mon, Dec 6, 2010 at 11:12 AM, Alexander Graf ag...@suse.de wrote:
 
 On 05.12.2010, at 17:25, Blue Swirl wrote:
 
 'info tlb' didn't show correct information for PAE mode and
 x86_64 long mode.
 
 Implement the missing modes. Also print NX bit for PAE and long modes.
 Fix off-by-one error in 32 bit mode mask.
 
 Signed-off-by: Blue Swirl blauwir...@gmail.com
 ---
 
 I didn't find an OS that enabled PAE, please test and report.
 
 Xen does. Just take a random recent xen kernel and run it with -kernel :).
 
 Thanks, it does (CR4 is 0xb0). I'm not sure all is correct, after
 0x100 the dump looks messy:
 00c0: 00c0 --PDA---W
 00e0: 00e0 --PDA---W
 0100: f000ff53f000f000 XG-D-C--W
 01001000: f000ff53f000e000 X--DW
 01002000: f000ff53f000f000 XG-D-C--W
 01003000: f000ff53f000f000 XG-D-C--W
 01004000: f000e987f000f000 X---A--U-

Good question XD. I guess the best way to find out if those numbers are right 
would be to trace the page tables manually. info tlb shows PA : VA, right?


Alex




Re: [Qemu-devel] [PATCH 0/6] usb-ccid (v8)

2010-12-07 Thread Alon Levy
ping.

Blue Swirl - one patch I forgot is in a later message titled ..v8.1..
with the removal of the libcaccard build.

On Sun, Nov 28, 2010 at 04:46:41PM +0200, Alon Levy wrote:
 This patchset adds three new devices, usb-ccid, ccid-card-passthru and
 ccid-card-emulated, providing a CCID bus, a simple passthru protocol
 implementing card requiring a client, and a standalone emulated card.
 
 It also introduces a new directory libcaccard with CAC card emulation,
 CAC is a type of ISO 7816 smart card.
 
 v7-v8 changes:
  * Blue Swirl comments:
   * usb-ccid: deannonymize some structs
   * usb-ccid: coding style change - answer_t and bulk_in_t fixed
   * usb-ccid: handle endianess conversion between guest and host
  * usb-ccid: s/ccid_bulk_in_copy_out/ccid_bulk_in_copy_to_guest/
  * ccid-card-emulated: fix segfault if backend not specified
  * ccid-card-emulated: let last reader inserted win
  * libcaccard: remove double vscard_common.h
 
 v6-v7 changes:
  * external libcaccard became internal directory libcaccard
   * statically link object files into qemu
   * produce libcaccard.so for usage by external projects
   * applied coding style to new code (please check me)
   - did not use the qemu options parsing for libcaccard, since
it seems to draw large amounts of qemu code (monitor for instance).
 
 v5-v6 changes:
  * really remove static debug (I apologize for claiming to have done so 
 before)
 
 v4-v5 changes:
  * rebased to latest
  * remove static debug in card devices
  * fix --enable-smartcard to link
  * stall instead of assert when exceeding BULK_OUT_DATA_SIZE
  * make ccid_reserve_recv_buf for too large len discard message, not exit
  * make ccid_reserve_recv_buf return void*
  * fix typo
  * remove commented code in VMState
 
 v3-v4:
  * remove ccid field in CCIDBus
  * remove static debug in bus
  * add back docs
 
 v2-v3:
  * split into bus (usb-ccid.c, uses ccid.h) and card (ccid-card-passthru.c).
  * removed documentation (being revised).
 
 v1-v2:
  * all QSIMPLEQ turned into fixed sized rings
  * all allocated buffers turned into fixed size buffers
  * added migration support
  * added a message to tell client qemu has migrated to ip:port
   * for lack of monitor commands ip:port are 0:0, which causes the updated
vscclient to connect to one port higher on the same host. will add monitor
commands in a separate patch. tested with current setup.
 
 Alon Levy (5):
   usb-ccid: add CCID bus
   ccid: add passthru card device
   libcaccard: update configure to build and use internal libcaccard
   ccid: add ccid-card-emulated device (v2)
   ccid: add docs
 
 Robert Relyea (1):
   libcaccard: initial commit after coding style fixes
 
  Makefile |6 +-
  Makefile.objs|6 +
  Makefile.target  |2 +
  configure|   36 ++
  docs/ccid.txt|  125 
  docs/libcaccard.txt  |  482 +++
  hw/ccid-card-emulated.c  |  501 
  hw/ccid-card-passthru.c  |  277 +
  hw/ccid.h|   34 ++
  hw/usb-ccid.c| 1345 
 ++
  libcaccard/Makefile  |   18 +
  libcaccard/cac.c |  411 +
  libcaccard/cac.h |   20 +
  libcaccard/card_7816.c   |  780 
  libcaccard/card_7816.h   |   60 ++
  libcaccard/card_7816t.h  |  163 +
  libcaccard/config.h  |   81 +++
  libcaccard/event.c   |  112 
  libcaccard/eventt.h  |   28 +
  libcaccard/link_test.c   |   20 +
  libcaccard/mutex.h   |   59 ++
  libcaccard/passthru.c|  608 +++
  libcaccard/passthru.h|   50 ++
  libcaccard/vcard.c   |  350 +++
  libcaccard/vcard.h   |   85 +++
  libcaccard/vcard_emul.h  |   59 ++
  libcaccard/vcard_emul_nss.c  | 1147 +++
  libcaccard/vcard_emul_type.c |   60 ++
  libcaccard/vcard_emul_type.h |   29 +
  libcaccard/vcardt.h  |   66 ++
  libcaccard/vevent.h  |   26 +
  libcaccard/vreader.c |  515 
  libcaccard/vreader.h |   53 ++
  libcaccard/vreadert.h|   23 +
  libcaccard/vscard_common.h   |  130 
  libcaccard/vscclient.c   |  710 ++
  36 files changed, 8475 insertions(+), 2 deletions(-)
  create mode 100644 docs/ccid.txt
  create mode 100644 docs/libcaccard.txt
  create mode 100644 hw/ccid-card-emulated.c
  create mode 100644 hw/ccid-card-passthru.c
  create mode 100644 hw/ccid.h
  create mode 100644 hw/usb-ccid.c
  create mode 100644 libcaccard/Makefile
  create mode 100644 libcaccard/cac.c
  create mode 100644 libcaccard/cac.h
  create mode 100644 libcaccard/card_7816.c
  create mode 100644 libcaccard/card_7816.h
  create mode 100644 libcaccard/card_7816t.h
  create mode 100644 libcaccard/config.h
  create mode 100644 libcaccard/event.c

[Qemu-devel] Re: [RFC][PATCH v5 00/21] virtagent: host/guest RPC communication agent

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 These patches apply to master, and can also be obtained from:
 git://repo.or.cz/qemu/mdroth.git virtagent_v5
 
 CHANGES IN V5:
 
  - Dependency on virtproxy dropped, virtagent now handles transport and 
 multiplexing of bi-directional RPCs internally
  - Removed duplification of qemu_set_fd_handler()-centered i/o code. Support 
 for interacting with objects that use qemu_set_fd_handler() now available to 
 tools via qemu-tools.c and a set of generalized utility functions
  - Fixed memory leaks in client/monitor functions
  - Various cleanups

Hi Michael,

Does this mean that virtproxy is now obsolete, or does it just mean
using virtproxy is optional?

I would still prefer to have virtagent a separate package, rather than
part of the QEMU tree though.

Thanks,
Jes




[Qemu-devel] QEMU 91c111 emulation bug?

2010-12-07 Thread Daniel Drubin
Greetings,

I am writing my own application to run on QEMU emulated ARM versatile
board. I observe some strange behavior when trying to emply the 91c11
LAN. Initialization passes OK, and I can receive and send ethernet
frames (verified with network sniffer). However, after the first
transmit the TX interrupt is not reported and the system stops
reporting ANY interrupts. I can transmit more frames, but no further
interrupts arrive (not even timer and UART interrupts). Reception is
interrupt-based and it works OK.

Is it something known?

I set up TX for auto-releasing transmitted frames from Tx FIFO and use
CPU to copy data to and from FIFOs.

I use QEMU 0.11.1 on Windows.

I can post the image that I use and/or NIC interface code.

Thanks,
Daniel



Re: [Qemu-devel] [PATCH 2/6] [RFC] Emulation of GRLIB IRQMP as defined in GRLIB IP Core User's Manual.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 06:25 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  hw/grlib_irqmp.c |  416 ++
  1 files changed, 416 insertions(+), 0 deletions(-)

diff --git a/hw/grlib_irqmp.c b/hw/grlib_irqmp.c
new file mode 100644
index 000..69e1553
--- /dev/null
+++ b/hw/grlib_irqmp.c
@@ -0,0 +1,416 @@
+/*
+ * QEMU GRLIB IRQMP Emulator
+ *
+ * (Multiprocessor and extended interrupt not supported)
+ *
+ * Copyright (c) 2010 AdaCore
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include sysbus.h
+#include cpu.h
+
+#include grlib.h
+
+/* #define DEBUG_IRQ */
+
+#ifdef DEBUG_IRQ
+#define DPRINTF(fmt, ...)   \
+do { printf(IRQMP:  fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...)
+#endif
+
+#define IRQMP_MAX_CPU 16
+#define IRQMP_REG_SIZE 256  /* Size of memory mapped registers */
+
+/* Memory mapped register offsets */
+#define LEVEL_OFFSET 0x00
+#define PENDING_OFFSET   0x04
+#define FORCE0_OFFSET0x08
+#define CLEAR_OFFSET 0x0C
+#define MP_STATUS_OFFSET 0x10
+#define BROADCAST_OFFSET 0x14
+#define MASK_OFFSET  0x40
+#define FORCE_OFFSET 0x80
+#define EXTENDED_OFFSET  0xC0
+
+typedef struct IRQMP
+{
+SysBusDevice busdev;
+
+CPUSPARCState *env;


Devices should never access CPUState directly. Instead, board level
should create CPU irqs and these should then be passed here.



This case is special, Leon3 is a System-On-Chip and some of the 
components are very close to the processor.

IRQMP is not really a peripheral nor a part of the CPU, it's both...


+} IRQMP;
+
+typedef struct IRQMPState
+{
+uint32_t level;
+uint32_t pending;
+uint32_t clear;
+uint32_t broadcast;
+
+uint32_t mask[IRQMP_MAX_CPU];
+uint32_t force[IRQMP_MAX_CPU];
+uint32_t extended[IRQMP_MAX_CPU];
+
+IRQMP*parent;
+} IRQMPState;
+
+IRQMPState grlib_irqmp_state;


Global state indicates poor design. Why separate IRQMP and IRQMPState?


I have to access IRQMPState in grlib_irqmp_ack and 
grlib_irqmp_check_irqs, but I don't see how I can do it without a global 
variable.

Again, I think that it's related to the special case of IRQMP.


+
+void grlib_irqmp_set_irq(void *opaque, int irq, int level);


This should not be global. Again, creating qemu_irqs or moving some of
the code to board level should help.


This one should be static indeed.


+
+DeviceState *grlib_irqmp_create(target_phys_addr_t   base,
+CPUState*env,
+qemu_irq   **cpu_irqs,
+uint32_t nr_irqs)
+{
+DeviceState *dev;
+
+assert(cpu_irqs != NULL);
+
+dev = qdev_create(NULL, grlib,irqmp);
+qdev_prop_set_ptr(dev, cpustate, env);
+
+if (qdev_init(dev)) {
+return NULL;
+}
+
+sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
+
+*cpu_irqs = qemu_allocate_irqs(grlib_irqmp_set_irq,
+grlib_irqmp_state,
+   nr_irqs);
+
+return dev;
+}
+
+static void grlib_irqmp_check_irqs(CPUState *env)
+{
+uint32_t pend   = 0;
+uint32_t level0 = 0;
+uint32_t level1 = 0;
+
+assert(env != NULL);
+
+/* IRQ for CPU 0 (no SMP support) */
+pend = (grlib_irqmp_state.pending | grlib_irqmp_state.force[0])
+  grlib_irqmp_state.mask[0];
+
+
+level0 = pend  ~grlib_irqmp_state.level;
+level1 = pendgrlib_irqmp_state.level;
+
+DPRINTF(pend:0x%04x force:0x%04x mask:0x%04x lvl1:0x%04x lvl0:0x%04x\n,
+grlib_irqmp_state.pending, grlib_irqmp_state.force[0],
+grlib_irqmp_state.mask[0], level1, level0);


The above should stay here, but code below should to go to board level
(leon3.c). Then you need to separate device IRQ handling from CPU PIL
handling.


If 

Re: [Qemu-devel] [PATCH 3/6] [RFC] Emulation of GRLIB APB UART as defined in GRLIB IP Core User's Manual.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 06:29 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  hw/grlib_apbuart.c |  231 
  1 files changed, 231 insertions(+), 0 deletions(-)

diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
new file mode 100644
index 000..32a5362
--- /dev/null
+++ b/hw/grlib_apbuart.c
@@ -0,0 +1,231 @@
+/*
+ * QEMU GRLIB APB UART Emulator
+ *
+ * Copyright (c) 2010 AdaCore
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include sysbus.h
+#include qemu-char.h
+
+#include grlib.h
+
+/* #define DEBUG_UART */
+
+#ifdef DEBUG_UART
+#define DPRINTF(fmt, ...)   \
+do { printf(APBUART:  fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...)
+#endif
+
+#define UART_REG_SIZE 20 /* Size of memory mapped registers */
+
+/* UART status register fields */
+#define UART_DATA_READY   (10)
+#define UART_TRANSMIT_SHIFT_EMPTY (11)
+#define UART_TRANSMIT_FIFO_EMPTY  (12)
+#define UART_BREAK_RECEIVED   (13)
+#define UART_OVERRUN  (14)
+#define UART_PARITY_ERROR (15)
+#define UART_FRAMING_ERROR(16)
+#define UART_TRANSMIT_FIFO_HALF   (17)
+#define UART_RECEIV_FIFO_HALF (18)


RECEIVE


Fixed.




+#define UART_TRANSMIT_FIFO_FULL   (19)
+#define UART_RECEIV_FIFO_FULL (1  10)
+
+/* UART control register fields */
+#define UART_RECEIV_ENABLE   (10)
+#define UART_TRANSMIT_ENABLE (11)
+#define UART_RECEIV_INTERRUPT(12)
+#define UART_TRANSMIT_INTERRUPT  (13)
+#define UART_PARITY_SELECT   (14)
+#define UART_PARITY_ENABLE   (15)
+#define UART_FLOW_CONTROL(16)
+#define UART_LOOPBACK(17)
+#define UART_EXTERNAL_CLOCK  (18)
+#define UART_RECEIV_FIFO_INTERRUPT   (19)
+#define UART_TRANSMIT_FIFO_INTERRUPT (1  10)
+#define UART_FIFO_DEBUG_MODE (1  11)
+#define UART_OUTPUT_ENABLE   (1  12)
+#define UART_FIFO_AVAILABLE  (1  31)
+
+/* Memory mapped register offsets */
+#define DATA_OFFSET   0x00
+#define STATUS_OFFSET 0x04
+#define CONTROL_OFFSET0x08
+#define SCALER_OFFSET 0x0C  /* not supported */
+#define FIFO_DEBUG_OFFSET 0x10  /* not supported */
+
+typedef struct UART
+{
+SysBusDevice busdev;
+
+qemu_irq irq;
+
+CharDriverState *chr;
+
+/* registers */
+uint32_t receive;
+uint32_t status;
+uint32_t control;
+} UART;
+
+
+DeviceState *grlib_apbuart_create(target_phys_addr_t  base,
+  CharDriverState*serial,
+  qemu_irqirq)
+{
+DeviceState *dev;
+
+dev = qdev_create(NULL, grlib,apbuart);
+qdev_prop_set_ptr(dev, chrdev, serial);
+
+if (qdev_init(dev)) {
+return NULL;
+}
+
+sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
+
+sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
+
+return dev;
+}
+
+static int grlib_apbuart_can_receive(void *opaque)
+{
+UART *uart = opaque;
+assert(uart != NULL);
+
+return !!(uart-status  UART_DATA_READY);
+}
+
+static void grlib_apbuart_receive(void *opaque, const uint8_t *buf, int size)
+{
+UART *uart = opaque;
+assert(uart != NULL);
+
+uart-receive  = *buf;
+uart-status  |= UART_DATA_READY;
+
+if (uart-control  UART_RECEIV_INTERRUPT)
+qemu_set_irq(uart-irq, 1);
+}
+
+static void grlib_apbuart_event(void *opaque, int event)
+{
+#ifdef DEBUG_UART
+printf(uart: event %x\n, event);
+#endif


DPRINTF or tracepoint.


Fixed.




+}
+
+static void
+grlib_apbuart_writel (void *opaque, target_phys_addr_t addr, uint32_t value)
+{
+UART  *uart = opaque;
+unsigned char  c= 0;
+
+addr= 0xff;
+
+assert(uart != NULL);
+

Re: [Qemu-devel] [PATCH 4/6] [RFC] Header file for the GRLIB components.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 06:31 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  hw/grlib.h |   27 +++
  1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/hw/grlib.h b/hw/grlib.h
new file mode 100644
index 000..eab7974
--- /dev/null
+++ b/hw/grlib.h
@@ -0,0 +1,27 @@
+#ifndef _GRLIB_H_
+#define _GRLIB_H_
+
+#include qdev.h
+
+/* IRQMP */
+
+DeviceState *grlib_irqmp_create(target_phys_addr_t   base,
+CPUState*env,
+qemu_irq   **cpu_irqs,
+uint32_t nr_irqs);
+
+/* GPTimer */
+
+DeviceState *grlib_gptimer_create(target_phys_addr_t  base,
+  uint32_tnr_timers,
+  uint32_tfreq,
+  qemu_irq   *cpu_irqs,
+  int base_irq);
+
+/* APB UART */
+
+DeviceState *grlib_apbuart_create(target_phys_addr_t  base,
+  CharDriverState*serial,
+  qemu_irqirq);
+


The file should not be needed when you move the functions to leon3.c.


These functions can be used to create more machines with GrLib 
components, that's why they are implemented in the grlib_* files.


--
Fabien Chouteau




[Qemu-devel] [PATCH 1/1] qemu-img: Deprecate obsolete -6 and -e options

2010-12-07 Thread Jes . Sorensen
From: Jes Sorensen jes.soren...@redhat.com

If -6 or -e is specified, an error message is printed and we exit. It
does not print help() to avoid the error message getting lost in the
noise.

Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
 block_int.h |1 -
 qemu-img.c  |   53 ++---
 2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/block_int.h b/block_int.h
index 3c3adb5..3ceed47 100644
--- a/block_int.h
+++ b/block_int.h
@@ -29,7 +29,6 @@
 #include qemu-queue.h
 
 #define BLOCK_FLAG_ENCRYPT 1
-#define BLOCK_FLAG_COMPRESS2
 #define BLOCK_FLAG_COMPAT6 4
 
 #define BLOCK_OPT_SIZE  size
diff --git a/qemu-img.c b/qemu-img.c
index 5b6e648..16fec40 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -261,21 +261,9 @@ fail:
 }
 
 static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
-int flags, const char *base_filename, const char *base_fmt)
+ const char *base_filename,
+ const char *base_fmt)
 {
-if (flags  BLOCK_FLAG_ENCRYPT) {
-if (set_option_parameter(list, BLOCK_OPT_ENCRYPT, on)) {
-error(Encryption not supported for file format '%s', fmt);
-return -1;
-}
-}
-if (flags  BLOCK_FLAG_COMPAT6) {
-if (set_option_parameter(list, BLOCK_OPT_COMPAT6, on)) {
-error(VMDK version 6 not supported for file format '%s', fmt);
-return -1;
-}
-}
-
 if (base_filename) {
 if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, base_filename)) 
{
 error(Backing file not supported for file format '%s', fmt);
@@ -293,7 +281,7 @@ static int add_old_style_options(const char *fmt, 
QEMUOptionParameter *list,
 
 static int img_create(int argc, char **argv)
 {
-int c, ret = 0, flags;
+int c, ret = 0;
 const char *fmt = raw;
 const char *base_fmt = NULL;
 const char *filename;
@@ -302,7 +290,6 @@ static int img_create(int argc, char **argv)
 QEMUOptionParameter *param = NULL, *create_options = NULL;
 char *options = NULL;
 
-flags = 0;
 for(;;) {
 c = getopt(argc, argv, F:b:f:he6o:);
 if (c == -1) {
@@ -323,11 +310,13 @@ static int img_create(int argc, char **argv)
 fmt = optarg;
 break;
 case 'e':
-flags |= BLOCK_FLAG_ENCRYPT;
-break;
+printf(qemu-img: option -e is deprecated, please use \'-o 
+   encryption\' instead!\n);
+return -1;
 case '6':
-flags |= BLOCK_FLAG_COMPAT6;
-break;
+printf(qemu-img: option -6 is deprecated, please use \'-o 
+   compat6\' instead!\n);
+return -1;
 case 'o':
 options = optarg;
 break;
@@ -385,7 +374,7 @@ static int img_create(int argc, char **argv)
 }
 
 /* Add old-style options to parameters */
-ret = add_old_style_options(fmt, param, flags, base_filename, base_fmt);
+ret = add_old_style_options(fmt, param, base_filename, base_fmt);
 if (ret  0) {
 goto out;
 }
@@ -674,7 +663,7 @@ static int compare_sectors(const uint8_t *buf1, const 
uint8_t *buf2, int n,
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
 const char *fmt, *out_fmt, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
 BlockDriverState **bs = NULL, *out_bs = NULL;
@@ -691,7 +680,7 @@ static int img_convert(int argc, char **argv)
 fmt = NULL;
 out_fmt = raw;
 out_baseimg = NULL;
-flags = 0;
+compress = 0;
 for(;;) {
 c = getopt(argc, argv, f:O:B:s:hce6o:);
 if (c == -1) {
@@ -712,14 +701,16 @@ static int img_convert(int argc, char **argv)
 out_baseimg = optarg;
 break;
 case 'c':
-flags |= BLOCK_FLAG_COMPRESS;
+compress = 1;
 break;
 case 'e':
-flags |= BLOCK_FLAG_ENCRYPT;
-break;
+printf(qemu-img: option -e is deprecated, please use \'-o 
+   encryption\' instead!\n);
+return -1;
 case '6':
-flags |= BLOCK_FLAG_COMPAT6;
-break;
+printf(qemu-img: option -6 is deprecated, please use \'-o 
+   compat6\' instead!\n);
+return -1;
 case 'o':
 options = optarg;
 break;
@@ -806,7 +797,7 @@ static int img_convert(int argc, char **argv)
 }
 
 set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
-ret = add_old_style_options(out_fmt, param, flags, out_baseimg, NULL);
+ret = add_old_style_options(out_fmt, param, out_baseimg, NULL);
 if (ret  0) {
 goto out;
 }
@@ -818,7 

Re: [Qemu-devel] [PATCH 5/6] [RFC] Emulation of Leon3.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 06:53 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  Makefile.target  |5 +-
  hw/leon3.c   |  310 ++
  target-sparc/cpu.h   |   10 ++
  target-sparc/helper.c|2 +-
  target-sparc/op_helper.c |   30 -
  5 files changed, 353 insertions(+), 4 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 2800f47..f40e04f 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -290,7 +290,10 @@ obj-sparc-y += cirrus_vga.o
  else
  obj-sparc-y = sun4m.o lance.o tcx.o sun4m_iommu.o slavio_intctl.o
  obj-sparc-y += slavio_timer.o slavio_misc.o sparc32_dma.o
-obj-sparc-y += cs4231.o eccmemctl.o sbi.o sun4c_intctl.o
+obj-sparc-y += cs4231.o eccmemctl.o sbi.o sun4c_intctl.o leon3.o
+
+# GRLIB
+obj-sparc-y += grlib_gptimer.o grlib_irqmp.o grlib_apbuart.o
  endif

  obj-arm-y = integratorcp.o versatilepb.o arm_pic.o arm_timer.o
diff --git a/hw/leon3.c b/hw/leon3.c
new file mode 100644
index 000..ba61081
--- /dev/null
+++ b/hw/leon3.c
@@ -0,0 +1,310 @@
+/*
+ * QEMU Leon3 System Emulator
+ *
+ * Copyright (c) 2010 AdaCore
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+#include hw.h
+#include qemu-timer.h
+#include qemu-char.h
+#include sysemu.h
+#include boards.h
+#include loader.h
+#include elf.h
+
+#include grlib.h
+
+/* #define DEBUG_LEON3 */
+
+#ifdef DEBUG_LEON3
+#define DPRINTF(fmt, ...)   \
+do { printf(Leon3:  fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...)
+#endif
+
+/* Default system clock.  */
+#define CPU_CLK (40 * 1000 * 1000)
+
+#define PROM_FILENAMEu-boot.bin
+
+#define MAX_PILS 16
+
+typedef struct Leon3State
+{
+uint32_t cache_control;
+uint32_t inst_cache_conf;
+uint32_t data_cache_conf;
+
+uint64_t entry; /* save kernel entry in case of reset */
+} Leon3State;
+
+Leon3State leon3_state;


Again global state, please refactor. Perhaps most of the cache
handling code belong to target-sparc/op_helper.c and this structure to
CPUSPARCState.


I will try to find a solution for that.
Is it OK to add some Leon3 specific stuff in the CPUSPARCState?


+
+/* Cache control: emulate the behavior of cache control registers but without
+   any effect on the emulated CPU */
+
+#define CACHE_DISABLED 0x0
+#define CACHE_FROZEN   0x1
+#define CACHE_ENABLED  0x3
+
+/* Cache Control register fields */
+
+#define CACHE_CTRL_IF (14)  /* Instruction Cache Freeze on Interrupt */
+#define CACHE_CTRL_DF (15)  /* Data Cache Freeze on Interrupt */
+#define CACHE_CTRL_DP (1  14)  /* Data cache flush pending */
+#define CACHE_CTRL_IP (1  15)  /* Instruction cache flush pending */
+#define CACHE_CTRL_IB (1  16)  /* Instruction burst fetch */
+#define CACHE_CTRL_FI (1  21)  /* Flush Instruction cache (Write only) */
+#define CACHE_CTRL_FD (1  22)  /* Flush Data cache (Write only) */
+#define CACHE_CTRL_DS (1  23)  /* Data cache snoop enable */
+
+void leon3_cache_control_int(void)
+{
+uint32_t state = 0;
+
+if (leon3_state.cache_control  CACHE_CTRL_IF) {
+/* Instruction cache state */
+state = leon3_state.cache_control  0x3;


Please add a new define CACHE_CTRL_xxx to replace 0x3.



Done.


+if (state == CACHE_ENABLED) {
+state = CACHE_FROZEN;
+DPRINTF(Instruction cache: freeze\n);
+}
+
+leon3_state.cache_control= ~0x3;
+leon3_state.cache_control |= state;
+}
+
+if (leon3_state.cache_control  CACHE_CTRL_DF) {
+/* Data cache state */
+state = (leon3_state.cache_control  2)  0x3;
+if (state == CACHE_ENABLED) {
+state = CACHE_FROZEN;
+DPRINTF(Data cache: freeze\n);
+}
+
+leon3_state.cache_control= ~(0x3  2);
+leon3_state.cache_control 

[Qemu-devel] User mode restart execution

2010-12-07 Thread John Vele
Grettings,

I am using user mode qemu for running stand-alone i386 binaries in linux.
What I want to do is this:
I want to run a binary until it ends,  but I don't want qemu-i386 to exit
(that is what
it does normally) but restart the execution of the same binary from the
beginning
without exiting.
I tried to modify the source code (where it starts to execute a loaded
binary) but
I don't know what variables or structs must be reinitialized in order to
start the
execution from the beginning of binary without exiting.
Any comment will be very very helpuf.

Thanx in advance


Re: [Qemu-devel] [PATCH 6/6] [RFC] SPARCV8 asr17 register support.

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 07:01 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 9:26 AM, Fabien Chouteauchout...@adacore.com  wrote:


Signed-off-by: Fabien Chouteauchout...@adacore.com
---
  hw/leon3.c   |6 ++
  target-sparc/cpu.h   |1 +
  target-sparc/machine.c   |2 ++
  target-sparc/translate.c |   10 ++
  4 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/hw/leon3.c b/hw/leon3.c
index ba61081..9605ce8 100644
--- a/hw/leon3.c
+++ b/hw/leon3.c
@@ -187,6 +187,12 @@ static void main_cpu_reset(void *opaque)
values */
 leon3_state.inst_cache_conf = 0x1022;
 leon3_state.data_cache_conf = 0x1822;
+
+/* Asr17 for Leon3 mono-processor */
+env-asr17= 0  28;  /* CPU id */
+env-asr17= 1  8;   /* SPARC V8 multiply and divide available */
+env-asr17= env-nwindows -1; /* Number of implemented registers
+   windows */


This is constant...


  }

  static void leon3_generic_hw_init(ram_addr_t  ram_size,
diff --git a/target-sparc/cpu.h b/target-sparc/cpu.h
index 6020ffd..36d49fc 100644
--- a/target-sparc/cpu.h
+++ b/target-sparc/cpu.h
@@ -341,6 +341,7 @@ typedef struct CPUSPARCState {
   from PSR) */
  #if !defined(TARGET_SPARC64) || defined(TARGET_ABI32)
 uint32_t wim;  /* window invalid mask */
+uint32_t asr17;/* asr17 */


... so no new env fields are needed...


  #endif
 target_ulong tbr;  /* trap base register */
  #if !defined(TARGET_SPARC64)
diff --git a/target-sparc/machine.c b/target-sparc/machine.c
index 752e431..c530bd3 100644
--- a/target-sparc/machine.c
+++ b/target-sparc/machine.c
@@ -42,6 +42,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 qemu_put_be32s(f,env-pil_in);
  #ifndef TARGET_SPARC64
 qemu_put_be32s(f,env-wim);
+qemu_put_be32s(f,env-asr17);


... there's also nothing to save/load...


 /* MMU */
 for (i = 0; i  32; i++)
 qemu_put_be32s(f,env-mmuregs[i]);
@@ -138,6 +139,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
 qemu_get_be32s(f,env-pil_in);
  #ifndef TARGET_SPARC64
 qemu_get_be32s(f,env-wim);
+qemu_get_be32s(f,env-asr17);
 /* MMU */
 for (i = 0; i  32; i++)
 qemu_get_be32s(f,env-mmuregs[i]);
diff --git a/target-sparc/translate.c b/target-sparc/translate.c
index 23f9519..65de614 100644
--- a/target-sparc/translate.c
+++ b/target-sparc/translate.c
@@ -58,6 +58,7 @@ static TCGv cpu_hintp, cpu_htba, cpu_hver, cpu_ssr, cpu_ver;
  static TCGv_i32 cpu_softint;
  #else
  static TCGv cpu_wim;
+static TCGv cpu_asr17;
  #endif
  /* local register indexes (only used inside old micro ops) */
  static TCGv cpu_tmp0;
@@ -2049,6 +2050,8 @@ static void disas_sparc_insn(DisasContext * dc)
 rs1 = GET_FIELD(insn, 13, 17);
 switch(rs1) {
 case 0: /* rdy */
+gen_movl_TN_reg(rd, cpu_y);
+break;
  #ifndef TARGET_SPARC64
 case 0x01 ... 0x0e: /* undefined in the SPARCv8
manual, rdy on the microSPARC
@@ -2058,6 +2061,11 @@ static void disas_sparc_insn(DisasContext * dc)
 case 0x10 ... 0x1f: /* implementation-dependent in the
SPARCv8 manual, rdy on the
microSPARC II */
+
+if (rs1 == 0x11) { /* Read %asr17 */
+gen_movl_TN_reg(rd, cpu_asr17);


Instead:
r_const = tcg_const_tl(asr constants  | dc-def-nwindows - 1);
gen_movl_TN_reg(rd, r_const);
tcg_temp_free(r_const);


OK for me, if it is acceptable to have this Leon3's specific behavior 
for all the SPARC32 CPUs.


--
Fabien Chouteau




Re: [Qemu-devel] [PATCH 01/25] QMP: Rename query handlers

2010-12-07 Thread Miguel Di Ciurcio Filho
On Mon, Dec 6, 2010 at 4:23 PM, Luiz Capitulino lcapitul...@redhat.com wrote:
 Query handlers still carry their human monitor name. This commit
 renames all of them to a more QMP-like name.

 For example, do_info_version() is renamed to qmp_query_version().
 - * do_info_balloon(): Balloon information
 + * qmp_query_balloon(): Balloon information
  *
  * Make an asynchronous request for balloon info.  When the request completes
  * a QDict will be returned according to the following specification:
 @@ -106,7 +106,7 @@ void monitor_print_balloon(Monitor *mon, const QObject 
 *data)
  *   major_page_faults: 142, minor_page_faults: 239245,
  *   free_mem: 1014185984, total_mem: 1044668416 }
  */
 -int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
 +int qmp_query_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
  {

If the idea is to detach the human monitor from the QMP API, is there
any reason to keep passing `Monitor *mon` to qmp_* functions? In some
parts of the code NULL is being passed.

Regards,

Miguel



[Qemu-devel] Re: [RFC][PATCH v5 01/21] Move code related to fd handlers into utility functions

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 This allows us to implement an i/o loop outside of vl.c that can
 interact with objects that use qemu_set_fd_handler()
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com

This commit message really tells us nothing. Please be more specific
about what is in the commit.

 diff --git a/qemu-ioh.c b/qemu-ioh.c
 new file mode 100644
 index 000..cc71470
 --- /dev/null
 +++ b/qemu-ioh.c
 @@ -0,0 +1,115 @@
 +/*
 + * QEMU System Emulator
 + *
 + * Copyright (c) 2003-2008 Fabrice Bellard
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining a 
 copy
 + * of this software and associated documentation files (the Software), to 
 deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:

Is this moved or new code? If the former, fine, but if it is new code,
you might want to leave your own name on the (c). I presume at least
some of the changes are (c) 2010?

 +/* XXX: fd_read_poll should be suppressed, but an API change is
 +   necessary in the character devices to suppress fd_can_read(). */

XXX in the comment isn't really of much use. Please make it more
explicit, or put your name in if it is a statement you wish to make.

 +int qemu_set_fd_handler3(void *ioh_record_list,
 + int fd,
 + IOCanReadHandler *fd_read_poll,
 + IOHandler *fd_read,
 + IOHandler *fd_write,
 + void *opaque)

I am not happy with this addition of numbers to these functions, it
doesn't tell us why we have a 3 and how it differs from 2. If 3 is
really the backend for implementing 2, maybe it would be better to name
it __qemu_set_fd_handler2() and then have macros/wrappers calling into it.

Cheers,
Jes



Re: [Qemu-devel] [PATCH 01/25] QMP: Rename query handlers

2010-12-07 Thread Luiz Capitulino
On Tue, 7 Dec 2010 10:28:25 -0200
Miguel Di Ciurcio Filho miguel.fi...@gmail.com wrote:

 On Mon, Dec 6, 2010 at 4:23 PM, Luiz Capitulino lcapitul...@redhat.com 
 wrote:
  Query handlers still carry their human monitor name. This commit
  renames all of them to a more QMP-like name.
 
  For example, do_info_version() is renamed to qmp_query_version().
  - * do_info_balloon(): Balloon information
  + * qmp_query_balloon(): Balloon information
   *
   * Make an asynchronous request for balloon info.  When the request 
  completes
   * a QDict will be returned according to the following specification:
  @@ -106,7 +106,7 @@ void monitor_print_balloon(Monitor *mon, const QObject 
  *data)
   *   major_page_faults: 142, minor_page_faults: 239245,
   *   free_mem: 1014185984, total_mem: 1044668416 }
   */
  -int do_info_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
  +int qmp_query_balloon(Monitor *mon, MonitorCompletion cb, void *opaque)
   {
 
 If the idea is to detach the human monitor from the QMP API, is there
 any reason to keep passing `Monitor *mon` to qmp_* functions?

No, I should drop it soon.

 In some parts of the code NULL is being passed.

Yes, this series is just a first step. We have lots of work to do and sometimes
I don't know what to do next, but the plan looks more or less like this:

1. Change info handlers to make directly QMP calls

   This series.
   
2. Move all qmp_query_* functions from the qmp_query_cmds[] table to the
   qmp-commands.hx one

   I have this almost ready, but I'm wondering if I should take the
   opportunity to drop the monitor object in this series...

3. Split all monitor commands into a human monitor part and a QMP part,
   the QMP part becomes the QMP API and the human monitor just calls it

   I have started doing this (maybe it's in an RFC state), but trust me,
   it's very difficult to split some handlers.

   One example is the do_change() one. Yes, a clearly human targeted
   command. So the question is: should we split such a handler or should
   we just add a good replacement?

   If we choose to add a replacement, then it's going to take a lot of
   time to have the QMP vs. HMP separation done. If we split the handler,
   then we'll have to live with complex, ugly code for a while.

4. Make handlers return an error object

   This is the new error infrastructure work, Markus will work on this.

5. Drop all Monitor object usage by QMP by introducing a QMP object

   We won't pass the QMP object to handlers, it's just our interface
   with the chardev layer that will change.

6. Add async command infrastructure

   Not necessary step 6, we'll probably be able to do it sooner.

7. Split the monitor code inti different files

   Possibly moving it to its own directory:

   - monitor/monitor.c: common code
   - monitor/hmp.c: human monitor
   - monitor/qmp.c: (guess what)

8. Make the QMP C API a real API

   Ie. do all cleanup needed to make it consumable, maybe even outside
   of QEMU

9. Conquer the world



[Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managing client/server rpc jobs

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 This implements a simple state machine to manage client/server rpc
 jobs being multiplexed over a single channel.
 
 A client job consists of sending an rpc request, reading an
 rpc response, then making the appropriate callbacks. We allow one
 client job to be processed at a time, which will make the following
 state transitions:
 
 VA_CLIENT_IDLE - VA_CLIENT_SEND (job queued, send channel open)
 VA_CLIENT_SEND - VA_CLIENT_WAIT (request sent, awaiting response)
 VA_CLIENT_WAIT - VA_CLIENT_IDLE (response recieved, callbacks made)
 
 A server job consists of recieving an rpc request, generating a
 response, then sending the response. We expect to receive one server
 request at a time due to the 1 at a time restriction for client jobs.
 Server jobs make the following transitions:
 
 VA_SERVER_IDLE - VA_SERVER_WAIT (recieved/executed request, send
 channel busy, response deferred)
 VA_SERVER_IDLE - VA_SERVER_SEND (recieved/executed request, send
 channel open, sending response)
 VA_SERVER_WAIT - VA_SERVER_SEND (send channel now open, sending
 response)
 VA_SERVER_SEND - VA_SERVER_IDLE (response sent)
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com

As mentioned before, I really don't understand why this is part of QEMU,
the guest agent really should be able to run totally outside of QEMU.

 +
 +#define DEBUG_VA
 +
 +#ifdef DEBUG_VA
 +#define TRACE(msg, ...) do { \
 +fprintf(stderr, %s:%s():L%d:  msg \n, \
 +__FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
 +} while(0)
 +#else
 +#define TRACE(msg, ...) \
 +do { } while (0)
 +#endif
 +
 +#define LOG(msg, ...) do { \
 +fprintf(stderr, %s:%s():  msg \n, \
 +__FILE__, __FUNCTION__, ## __VA_ARGS__); \
 +} while(0)

This must be like the 217th copy of these functions, could you please
use some of the code that is already in the tree, and make it generic if
needed.

 +
 +#define VERSION 1.0
 +#define EOL \r\n
 +
 +#define VA_HDR_LEN_MAX 4096 /* http header limit */
 +#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */
 +#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */
 +#define VA_SERVER_JOBS_MAX 1 /* max server rpcs we can queue */

As mentioned last time, please make this stuff configurable and not hard
coded.

Cheers,
Jes



[Qemu-devel] Re: [RFC][PATCH v5 04/21] virtagent: transport definitions and job callbacks

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 +static void va_server_read_cb(const char *content, size_t content_len)
 +{
 +xmlrpc_mem_block *resp_xml;
 +VAServerData *server_data = va_state-server_data;
 +int ret;
 +
 +TRACE(called);
 +resp_xml = xmlrpc_registry_process_call(server_data-env,
 +server_data-registry,
 +NULL, content, content_len);
 +if (resp_xml == NULL) {
 +LOG(error processing RPC request);
 +goto out_bad;
 +}
 +
 +ret = va_server_job_add(resp_xml);
 +if (ret != 0) {
 +LOG(error adding server job: %s, strerror(ret));
 +}
 +
 +return;
 +out_bad:
 +/* TODO: should reset state here */
 +return;

Looks like some missing error handling is needed here?

 +static void va_rpc_parse_hdr(VAHTState *s)
 +{
 +int i, line_pos = 0;
 +bool first_line = true;
 +char line_buf[4096];

In 03/21 you defined VA_HDR_LEN_MAX to 4096, here you hard code the
value  sounds like something begging to go wrong.

 +static int va_end_of_header(char *buf, int end_pos)
 +{
 +return !strncmp(buf+(end_pos-2), \n\r\n, 3);
 +}

Maybe I am missing something here, but it looks like you do a strncmp to
a char that is one past the end of the buffer, or? If this is
intentional, please document it.

All this http parsing code leaves the question open why you do it
manually, instead of relying on a library?

Cheers,
Jes



[Qemu-devel] [[PATCH][RESEND]] virtio-serial-bus: Bump up control vq descriptors to min. required

2010-12-07 Thread Amit Shah
The current default of 16 buffers for the control vq is too small for
the default max_nr_ports of 32.  We can get more entries in there,
example when asking the guest to add max. allowed ports.

Default to using the minimum required (next power of 2) of the
max_nr_ports in use.

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

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 74ba5ec..eb7b362 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -736,7 +736,7 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, uint32_t 
max_nr_ports)
 {
 VirtIOSerial *vser;
 VirtIODevice *vdev;
-uint32_t i, max_supported_ports;
+uint32_t i, max_supported_ports, cvq_len;
 
 if (!max_nr_ports)
 return NULL;
@@ -769,10 +769,17 @@ VirtIODevice *virtio_serial_init(DeviceState *dev, 
uint32_t max_nr_ports)
 /* Add a queue for guest to host transfers for port 0 (backward compat) */
 vser-ovqs[0] = virtio_add_queue(vdev, 128, handle_output);
 
+/*
+ * The number of descriptors to use should always be a power of
+ * two.  Use the next power of 2 of max_nr_ports to keep the
+ * entries to a minimum.
+ */
+cvq_len = 1  qemu_fls(max_nr_ports - 1);
+
 /* control queue: host to guest */
-vser-c_ivq = virtio_add_queue(vdev, 16, control_in);
+vser-c_ivq = virtio_add_queue(vdev, cvq_len, control_in);
 /* control queue: guest to host */
-vser-c_ovq = virtio_add_queue(vdev, 16, control_out);
+vser-c_ovq = virtio_add_queue(vdev, cvq_len, control_out);
 
 for (i = 1; i  vser-bus-max_nr_ports; i++) {
 /* Add a per-port queue for host to guest transfers */
-- 
1.7.3.2




[Qemu-devel] Re: [RFC][PATCH v5 05/21] virtagent: base client definitions

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 +#ifndef VIRTAGENT_H
 +#define VIRTAGENT_H
 +
 +#include monitor.h
 +
 +#define GUEST_AGENT_PATH_CLIENT /tmp/virtagent-guest-client.sock
 +#define HOST_AGENT_PATH_CLIENT /tmp/virtagent-host-client.sock

As mentioned last time, this belongs in a config file.

Jes



[Qemu-devel] Re: [RFC][PATCH v5 06/21] virtagent: base server definitions

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 +#define GUEST_AGENT_SERVICE_ID virtagent
 +#define GUEST_AGENT_PATH /tmp/virtagent-guest.sock
 +#define HOST_AGENT_SERVICE_ID virtagent-host
 +#define HOST_AGENT_PATH /tmp/virtagent-host.sock
 +#define VA_GETFILE_MAX 1  30
 +#define VA_FILEBUF_LEN 16384
 +#define VA_DMESG_LEN 16384

Config file please!

Jes



Re: [Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command

2010-12-07 Thread Michael Roth

On 12/06/2010 05:20 PM, Michael Roth wrote:

On 12/06/2010 04:08 PM, Adam Litke wrote:

On Fri, 2010-12-03 at 12:03 -0600, Michael Roth wrote:

Utilize the getfile RPC to provide a means to view text files in the
guest. Getfile can handle binary files as well but we don't advertise
that here due to the special handling requiring to store it and provide
it back to the user (base64 encoding it for instance). Hence the
otherwise confusing viewfile as opposed to getfile.


What happens to the monitor if you use this to view a binary file?


At the very least we probably get a lot of truncated files from the
binary-string conversion via monitor_printf(). Im not sure how the
qobject/json layer would deal with things.


Retrieving binary files progmatically using the QMP interface is a valid
use case right?


For getfile (the RPC), but not for viewfile (HMP/QMP). It's doable, but
we'd *have to* pass this data to the user as base64-encoded data at the
QMP level. At the HMP level I think we're good either way, since we
could just base64 decode in the print function.

So in the case of QMP we'd be pushing complexity to the user in exchange
for not having a seperate plain-text-only interface.

Either way seems reasonable, but I'd been planning on adding a seperate
`agent_copyfile remote_path local_path` command for dealing with
binary data, and making viewfile quick and easy for plain text (both for
HMP and QMP).



Although, agent_copyfile doesn't seem like the right approach looking at 
things like future libvirt integration. So we will most likely end up 
with a QMP command that passes base64-encoded binary data to the 
end-user for binary data, which we can provide a pretty-printing HMP 
function to decode. We'd need to take care to differentiate the HMP 
command from the QMP one however, else we'd have users tempted to do 
something like:


echo agent_getfile /remotepath/rand.bin | socat stdin 
unix-connect:monitor.sock  /localpath/rand.bin


to avoid having to decode the data. Would documenting the HMP 
counterpart as being reliable only for plain-text be sufficient? Or 
Should be have QMP:agent_getfile() and HMP:agent_viewfile()?





Re: [Qemu-devel] [PATCH] MIPS Initial support of Godson-3a multicore CPU

2010-12-07 Thread chen huacai
1, signed-off-by and reviewed-by should put after the introduction and
before the code, not at first.
2, You'd better split the big patch to 2~3 small patches,  E.g. CPU
definition and board emulation should be split. The format of email
title will be [Patch 0/3], [Patch 1/3] and so on. [Patch 0/3] give a
general introduction and others are real patches.
3, You should tell us how to test you code. E.g., you should provide
PMON/BIOS, OS kernel or tell us how to build them in [patch 0/3].

For more information, you can search for my patch series about Loongson-2E.
Good luck!

Huacai Chen

On Tue, Dec 7, 2010 at 5:32 PM, Jin Guojie jinguo...@loongson.cn wrote:
 Signed-off-by: Jin Guojie jinguo...@loongson.cn
 Reviewed-by: Gao Xiang gaoxi...@ict.ac.cn
 Reviewed-by: Chen Huacai zltjiang...@gmail.com

  A patch for Godson-3a CPU simulation.
  Godson-3a is a newly developed MIPS-III like, multicore CPU by ICT, China.
  We believe this patch could be helpful for other Godson developers.
  For you review. Any comment is welcomed.

 Jin Guojie
 www.loongson.cn
 ---
  Makefile.target              |    2 +-
  hw/mips_godson3a.c           |  507 
 ++
  target-mips/mips-defs.h      |    4 +-
  target-mips/translate_init.c |   26 +++
  4 files changed, 536 insertions(+), 3 deletions(-)
  create mode 100755 hw/mips_godson3a.c

 diff --git a/Makefile.target b/Makefile.target
 index 91e6e74..8f29aeb 100644
 --- a/Makefile.target
 +++ b/Makefile.target
 @@ -230,7 +230,7 @@ obj-ppc-y += xilinx_timer.o
  obj-ppc-y += xilinx_uartlite.o
  obj-ppc-y += xilinx_ethlite.o

 -obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
 +obj-mips-y = mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o 
 mips_godson3a.o
  obj-mips-y += mips_addr.o mips_timer.o mips_int.o
  obj-mips-y += vga.o i8259.o
  obj-mips-y += g364fb.o jazz_led.o
 diff --git a/hw/mips_godson3a.c b/hw/mips_godson3a.c
 new file mode 100755
 index 000..4085db2
 --- /dev/null
 +++ b/hw/mips_godson3a.c
 @@ -0,0 +1,507 @@
 +/*
 + * QEMU godson 3a developing board support
 + *
 + * Copyright (c) 2009 Gao Xiang (gaoxi...@ict.ac.cn)
 + * Copyright (c) 2010 Jin Guojie (jinguo...@loongson.cn)
 + * This code is licensed under the GNU GPL v2.
 + */
 +
 +/*
 + * Godson 3a developing board is based on ICT/ST Godson-3a.
 + * Godson-3a CPU is a MIPS-III like, multicore processor.
 + * It can be configured to contain 4 or 8 cores. Every 4
 + * cores are grouped into one on-chip 'node'. SMP mechanism
 + * is supported by Godson IPI(inter-processors interrupt)
 + * specification.
 + *
 + * Godson 3a CPU intro:
 + *   http://en.wikipedia.org/wiki/Loongson
 + *
 + * Godson 3a user manual:
 + *   http://www.loongsondeveloper.com/doc/Loongson3AUserGuide.pdf
 + */
 +#include hw.h
 +#include mips.h
 +#include pc.h
 +#include isa.h
 +#include net.h
 +#include sysemu.h
 +#include boards.h
 +#include ide.h
 +#include mips-bios.h
 +#include elf.h
 +#include loader.h
 +#include blockdev.h
 +#include mips_cpudevs.h
 +#include mc146818rtc.h
 +
 +static target_ulong PHYS_TO_VIRT(target_ulong phys)
 +{
 +    if (smp_cpus  1)
 +        return ((phys) | 0x9800ULL);
 +    else
 +        return ((phys) | ~(target_ulong)0x7fff);
 +}
 +
 +#define VIRT_TO_PHYS_ADDEND (-((int64_t)(int32_t)0x8000))
 +
 +#define MAX_IDE_BUS 2
 +
 +static const int ide_iobase[2] = { 0x1f0, 0x170 };
 +static const int ide_iobase2[2] = { 0x3f6, 0x376 };
 +static const int ide_irq[2] = { 14, 15 };
 +
 +static int serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
 +
 +static PITState *pit; /* PIT i8254 */
 +
 +/* i8254 PIT is attached to the IRQ0 at PIC i8259 */
 +
 +static struct _loaderparams {
 +    int ram_size;
 +    const char *kernel_filename;
 +    const char *kernel_cmdline;
 +    const char *initrd_filename;
 +} loaderparams;
 +
 +static void mips_qemu_writel (void *opaque, target_phys_addr_t addr,
 +                             uint32_t val)
 +{
 +    if ((addr  0x) == 0  val == 42)
 +        qemu_system_reset_request();
 +    else if ((addr  0x) == 4  val == 42)
 +        qemu_system_shutdown_request();
 +}
 +
 +static uint32_t mips_qemu_readl (void *opaque, target_phys_addr_t addr)
 +{
 +    return 0;
 +}
 +
 +static CPUWriteMemoryFunc *mips_qemu_write[] = {
 +    mips_qemu_writel,
 +    mips_qemu_writel,
 +    mips_qemu_writel,
 +};
 +
 +static CPUReadMemoryFunc *mips_qemu_read[] = {
 +    mips_qemu_readl,
 +    mips_qemu_readl,
 +    mips_qemu_readl,
 +};
 +
 +static int mips_qemu_iomemtype = 0;
 +
 +typedef struct ResetData {
 +    CPUState *env;
 +    uint64_t vector;
 +} ResetData;
 +
 +static int64_t load_kernel (CPUState *env)
 +{
 +    int64_t entry, kernel_high;
 +    long kernel_size, initrd_size, params_size;
 +    ram_addr_t initrd_offset;
 +    uint32_t *params_buf;
 +    int big_endian;
 +
 +#ifdef TARGET_WORDS_BIGENDIAN
 +    big_endian = 1;
 +#else
 +    big_endian = 0;
 +#endif
 +
 +    kernel_size = 

[Qemu-devel] [PATCH 0/5] ARM: fix VQSHL (register)

2010-12-07 Thread Peter Maydell
This patchset fixes bugs in the decode and implementation
of the ARM Neon VQSHL (register) instruction form. It is
a split out version of those parts of the maemo-qemu tree
commit 03a2445a which deal with this instruction form;
the remainder which address VQSHL (immediate) I'll send
in a separate patchset.

The 'Fix VQSHL of signed 64 bit values by shift counts = 64'
patch is a bug fix which does not appear in the maemo-qemu
patch. 'Correct result in saturating cases for VQSHL of s8/16/32'
does appear, but I refactored it a little to be clearer;
these two therefore have me as the author.

These patches have been tested by running random instruction
sequences and comparing against A8 hardware.


Juha Riihimäki (3):
  ARM: Fix arguments passed to VQSHL helpers
  ARM: Fix VQSHL of signed 64 bit values
  ARM: remove pointless else clause in VQSHL of u64

Peter Maydell (2):
  ARM: Fix VQSHL of signed 64 bit values by shift counts = 64
  ARM: Correct result in saturating cases for VQSHL of s8/16/32

 target-arm/neon_helper.c |   21 ++---
 target-arm/translate.c   |4 ++--
 2 files changed, 16 insertions(+), 9 deletions(-)




[Qemu-devel] Re: [RFC][PATCH v5 07/21] virtagent: add va.getfile RPC

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 Add RPC to retrieve a guest file. This interface is intended
 for smaller reads like peeking at logs and /proc and such.

I think you need to redesign your approach here. see below.

In 06/21 you had:

+#define VA_GETFILE_MAX 1  30

 +while ((ret = read(fd, buf, VA_FILEBUF_LEN))  0) {
 +file_contents = qemu_realloc(file_contents, count + VA_FILEBUF_LEN);
 +memcpy(file_contents + count, buf, ret);

UH OH!

realloc will do a malloc and a memcpy of the data, this is going to turn
into a really nasty malloc memcpy loop if someone tries to transfer a
large file using this method. You could end up with almost 4GB of
parallel allocations for a guest that might have been configured as a
1GB guest. This would allow the guest to effectively blow the expected
memory consumption out of the water. It's not exactly going to be fast
either :(

Maybe use a tmp file, and write data out to that as you receive it to
avoid the malloc ballooning.

Jes



[Qemu-devel] Re: [RFC][PATCH v5 08/21] virtagent: add agent_viewfile qmp/hmp command

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 Utilize the getfile RPC to provide a means to view text files in the
 guest. Getfile can handle binary files as well but we don't advertise
 that here due to the special handling requiring to store it and provide
 it back to the user (base64 encoding it for instance). Hence the
 otherwise confusing viewfile as opposed to getfile.
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  hmp-commands.hx |   16 +
  monitor.c   |1 +
  qmp-commands.hx |   33 +++
  virtagent.c |   96 
 +++
  virtagent.h |3 ++
  5 files changed, 149 insertions(+), 0 deletions(-)
 
 diff --git a/hmp-commands.hx b/hmp-commands.hx
 index e5585ba..423c752 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -1212,6 +1212,22 @@ show available trace events and their state
  ETEXI
  #endif
  
 +{
 +.name   = agent_viewfile,
 +.args_type  = filepath:s,
 +.params = filepath,
 +.help   = Echo a file from the guest filesystem,
 +.user_print = do_agent_viewfile_print,
 +.mhandler.cmd_async = do_agent_viewfile,
 +.flags  = MONITOR_CMD_ASYNC,
 +},
 +
 +STEXI
 +...@item agent_viewfile @var{filepath}
 +...@findex agent_viewfile
 +Echo the file identified by @var{filepath} on the guest filesystem
 +ETEXI
 +
  STEXI
  @end table
  ETEXI
 diff --git a/monitor.c b/monitor.c
 index 8cee35d..145895d 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -56,6 +56,7 @@
  #include json-parser.h
  #include osdep.h
  #include exec-all.h
 +#include virtagent.h
  #ifdef CONFIG_SIMPLE_TRACE
  #include trace.h
  #endif
 diff --git a/qmp-commands.hx b/qmp-commands.hx
 index 793cf1c..efa2137 100644
 --- a/qmp-commands.hx
 +++ b/qmp-commands.hx
 @@ -738,6 +738,39 @@ Example:
  EQMP
  
  {
 +.name   = agent_viewfile,
 +.args_type  = filepath:s,
 +.params = filepath,
 +.help   = Echo a file from the guest filesystem,
 +.user_print = monitor_user_noop,
 +.mhandler.cmd_async = do_agent_viewfile,
 +.flags  = MONITOR_CMD_ASYNC,
 +},
 +
 +STEXI
 +...@item agent_viewfile @var{filepath}
 +...@findex agent_viewfile
 +Echo the file identified by @var{filepath} on the guest filesystem
 +ETEXI
 +SQMP
 +agent_viewfile
 +
 +
 +Echo the file identified by @var{filepath} from the guest filesystem.
 +
 +Arguments:
 +
 +- filepath: Full guest path of the desired file
 +
 +Example:
 +
 +- { execute: agent_viewfile,
 +arguments: { filepath: /sys/kernel/kexec_loaded } }
 +- { return: { contents: 0 } }
 +
 +EQMP
 +
 +{
  .name   = qmp_capabilities,
  .args_type  = ,
  .params = ,
 diff --git a/virtagent.c b/virtagent.c
 index 34d8545..4a4dc8a 100644
 --- a/virtagent.c
 +++ b/virtagent.c
 @@ -139,3 +139,99 @@ out_free:
  out:
  return ret;
  }
 +
 +/* QMP/HMP RPC client functions */
 +
 +void do_agent_viewfile_print(Monitor *mon, const QObject *data)
 +{
 +QDict *qdict;
 +const char *contents = NULL;
 +int i;
 +
 +qdict = qobject_to_qdict(data);
 +if (!qdict_haskey(qdict, contents)) {
 +return;
 +}
 +
 +contents = qdict_get_str(qdict, contents);
 +if (contents != NULL) {
 + /* monitor_printf truncates so do it in chunks. also, file_contents
 +  * may not be null-termed at proper location so explicitly calc
 +  * last chunk sizes */
 +for (i = 0; i  strlen(contents); i += 1024) {
 +monitor_printf(mon, %.1024s, contents + i);
 +}
 +}
 +monitor_printf(mon, \n);
 +}
 +
 +static void do_agent_viewfile_cb(const char *resp_data,
 + size_t resp_data_len,
 + MonitorCompletion *mon_cb,
 + void *mon_data)
 +{
 +xmlrpc_value *resp = NULL;
 +char *file_contents = NULL;
 +size_t file_size;
 +int ret;
 +xmlrpc_env env;
 +QDict *qdict = qdict_new();
 +
 +if (resp_data == NULL) {
 +LOG(error handling RPC request);
 +goto out_no_resp;
 +}
 +
 +xmlrpc_env_init(env);
 +resp = xmlrpc_parse_response(env, resp_data, resp_data_len);
 +if (va_rpc_has_error(env)) {
 +ret = -1;
 +goto out_no_resp;
 +}
 +
 +xmlrpc_parse_value(env, resp, 6, file_contents, file_size);
 +if (va_rpc_has_error(env)) {
 +ret = -1;
 +goto out;

I believe this suffers from the same architectural problem I mentioned
in my comment to 07/21 - you don't restrict the file size, so it could
blow up the QEMU process on the host trying to view the wrong file.

I really think it is a bad idea to put this kind of command into the
monitor.

Jes




[Qemu-devel] [PATCH 3/5] ARM: Fix VQSHL of signed 64 bit values by shift counts = 64

2010-12-07 Thread Peter Maydell
VQSHL of a signed 64 bit non-zero value by a shift count = 64 should
saturate; return the correct value in this case.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
 target-arm/neon_helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index d29b884..2dc3d96 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -608,7 +608,7 @@ uint64_t HELPER(neon_qshl_s64)(CPUState *env, uint64_t 
valop, uint64_t shiftop)
 if (shift = 64) {
 if (val) {
 SET_QC();
-val = (val  63)  ~SIGNBIT64;
+val = (val  63) ^ ~SIGNBIT64;
 }
 } else if (shift = -64) {
 val = 63;
-- 
1.6.3.3




[Qemu-devel] [PATCH 4/5] ARM: remove pointless else clause in VQSHL of u64

2010-12-07 Thread Peter Maydell
From: Juha Riihimäki juha.riihim...@nokia.com

Remove a pointless else clause in the neon_qshl_u64 helper.

Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
 target-arm/neon_helper.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 2dc3d96..48b9f5b 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -560,8 +560,6 @@ uint64_t HELPER(neon_qshl_u64)(CPUState *env, uint64_t val, 
uint64_t shiftop)
 if (val) {
 val = ~(uint64_t)0;
 SET_QC();
-} else {
-val = 0;
 }
 } else if (shift = -64) {
 val = 0;
-- 
1.6.3.3




[Qemu-devel] [PATCH 2/5] ARM: Fix VQSHL of signed 64 bit values

2010-12-07 Thread Peter Maydell
From: Juha Riihimäki juha.riihim...@nokia.com

Add a missing '-' which meant that we were misinterpreting the shift
argument for VQSHL of 64 bit signed values and treating almost every
shift value as if it were an extremely large right shift.

Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
 target-arm/neon_helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index 5e6452b..d29b884 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -610,7 +610,7 @@ uint64_t HELPER(neon_qshl_s64)(CPUState *env, uint64_t 
valop, uint64_t shiftop)
 SET_QC();
 val = (val  63)  ~SIGNBIT64;
 }
-} else if (shift = 64) {
+} else if (shift = -64) {
 val = 63;
 } else if (shift  0) {
 val = -shift;
-- 
1.6.3.3




[Qemu-devel] [PATCH 1/5] ARM: Fix arguments passed to VQSHL helpers

2010-12-07 Thread Peter Maydell
From: Juha Riihimäki juha.riihim...@nokia.com

Correct the arguments passed when generating neon qshl_{u,s}64()
helpers so that we use the correct registers.

Signed-off-by: Juha Riihimäki juha.riihim...@nokia.com
Reviewed-by: Peter Maydell peter.mayd...@linaro.org
---
 target-arm/translate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 99464ab..b5af1c6 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4235,9 +4235,9 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 case 9: /* VQSHL */
 if (u) {
 gen_helper_neon_qshl_u64(cpu_V0, cpu_env,
- cpu_V0, cpu_V0);
+ cpu_V1, cpu_V0);
 } else {
-gen_helper_neon_qshl_s64(cpu_V1, cpu_env,
+gen_helper_neon_qshl_s64(cpu_V0, cpu_env,
  cpu_V1, cpu_V0);
 }
 break;
-- 
1.6.3.3




Re: [Qemu-devel] Re: [RFC][PATCH v5 00/21] virtagent: host/guest RPC communication agent

2010-12-07 Thread Michael Roth

On 12/07/2010 04:24 AM, Jes Sorensen wrote:

On 12/03/10 19:03, Michael Roth wrote:

These patches apply to master, and can also be obtained from:
git://repo.or.cz/qemu/mdroth.git virtagent_v5

CHANGES IN V5:

  - Dependency on virtproxy dropped, virtagent now handles transport and 
multiplexing of bi-directional RPCs internally
  - Removed duplification of qemu_set_fd_handler()-centered i/o code. Support 
for interacting with objects that use qemu_set_fd_handler() now available to 
tools via qemu-tools.c and a set of generalized utility functions
  - Fixed memory leaks in client/monitor functions
  - Various cleanups


Hi Michael,

Does this mean that virtproxy is now obsolete, or does it just mean
using virtproxy is optional?



As far as virtagent goes it is obsolete, and without the guest-side bits 
of virtproxy being integrated into the guest agent I don't see it being 
very useful at this point.



I would still prefer to have virtagent a separate package, rather than
part of the QEMU tree though.


There's a client and server in qemu, and a client and server in the 
agent, and all that code is shared. So even if we were to have a 
seperate tree for the agent, 90% of the code would also be sitting in 
the qemu tree anyway. I wouldn't mind hosting it outside of qemu but 
given what we're trying to do there's not a whole lot to be gained from it.


I agree it'd make sense if virtagent wasn't bidirectional since then 
there'd be a clean separation between qemu (client) and virtagent 
(server), and it would have the added benefit of enforcing 
consistent/stable client/server APIs between versions, but that's not 
the case here.




Thanks,
Jes







[Qemu-devel] Re: [RFC][PATCH v5 09/21] virtagent: add va.getdmesg RPC

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 Add RPC to view guest dmesg output.
 
 Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com
 ---
  virtagent-server.c |   46 ++
  1 files changed, 46 insertions(+), 0 deletions(-)
 
 diff --git a/virtagent-server.c b/virtagent-server.c
 index a430b58..aac8f70 100644
 --- a/virtagent-server.c
 +++ b/virtagent-server.c
 @@ -83,6 +83,50 @@ EXIT_CLOSE_BAD:
  return result;
  }
  
 +/* va_getdmesg(): return dmesg output
 + * rpc return values:
 + *   - dmesg output as a string
 + */
 +static xmlrpc_value *va_getdmesg(xmlrpc_env *env,
 +  xmlrpc_value *param,
 +  void *user_data)
 +{
 +char *dmesg_buf = NULL, cmd[256];
 +int ret;
 +xmlrpc_value *result = NULL;
 +FILE *pipe;
 +
 +SLOG(va_getdmesg());
 +
 +dmesg_buf = qemu_mallocz(VA_DMESG_LEN + 2048);
 +sprintf(cmd, dmesg -s %d, VA_DMESG_LEN);

What happens if the guest's dmesg buffer is larger than your hardcoded
value?

Jes




[Qemu-devel] Re: [RFC][PATCH v5 20/21] virtagent: integrate virtagent server/client via chardev

2010-12-07 Thread Jes Sorensen
On 12/03/10 19:03, Michael Roth wrote:
 +#include virtagent-common.h
 +
 +static CharDriverState *qemu_chr_open_virtagent(QemuOpts *opts)
 +{
 +CharDriverState *chr;
 +int fd, ret;
 +
 +/* revert to/enforce default socket chardev options for virtagent */
 +if (qemu_opt_get(opts, path) == NULL) {
 +qemu_opt_set(opts, path, /tmp/virtagent-client.sock);
 +}

More hardcoded paths, which you defined somewhere in a header already.
Again, please make it configurable.

 +//qemu_opt_set(opts, id, virtagent);

If it isn't needed, please remove it.

Cheers,
Jes



Re: [Qemu-devel] Re: [RFC][PATCH v5 01/21] Move code related to fd handlers into utility functions

2010-12-07 Thread Jes Sorensen
On 12/07/10 15:48, Michael Roth wrote:
 On 12/07/2010 07:31 AM, Jes Sorensen wrote:
 On 12/03/10 19:03, Michael Roth wrote:
 This allows us to implement an i/o loop outside of vl.c that can
 interact with objects that use qemu_set_fd_handler()

 Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com

 This commit message really tells us nothing. Please be more specific
 about what is in the commit.

 
 Currently, in qemu, the virtagent client/server functionality is driven
 by vl.c:main_loop_wait(), which implements a select() loop that kicks
 off handlers registered for various FDs/events via qemu_set_fd_handler().
 
 To share the code with the agent, qemu-va.c, I re-implemented this i/o
 loop to do same thing, along with vl.c:qemu_set_fd_handler*() and
 friends. It was big nasty copy/paste job and I think most of the
 reviewers agreed that the i/o loop code should be shared.
 
 This commit moves the shared code into back-end utility functions that
 get called by vl.c:qemu_set_fd_handler()/qemu_process_fd_handlers() and
 friends for qemu, and by
 qemu-tools.c:qemu_set_fd_handler()/qemu_process_fd_handlers() for tools.
 
 So now to interact with code that uses qemu_set_fd_handler() you can
 built a select() loop around these utility functions.

Please put some of this in the commit message.

 I am not happy with this addition of numbers to these functions, it
 doesn't tell us why we have a 3 and how it differs from 2. If 3 is
 really the backend for implementing 2, maybe it would be better to name
 it __qemu_set_fd_handler2() and then have macros/wrappers calling into
 it.
 
 That was the initial plan, but qemu_set_fd_handler2() is a back-end of
 sorts for qemu_set_fd_handler(), so I was just trying to be consistent
 with the naming. Personally I don't have any objections one way or the
 other.

Anything to avoid qemu_set_fd_handler17() at some point. I think using
__qemu_set_fd_handler() encourages people to modify that code rather
than copy it.

Cheers,
Jes



Re: [Qemu-devel] Re: [RFC][PATCH v5 03/21] virtagent: common code for managing client/server rpc jobs

2010-12-07 Thread Michael Roth

On 12/07/2010 07:38 AM, Jes Sorensen wrote:

On 12/03/10 19:03, Michael Roth wrote:

This implements a simple state machine to manage client/server rpc
jobs being multiplexed over a single channel.

A client job consists of sending an rpc request, reading an
rpc response, then making the appropriate callbacks. We allow one
client job to be processed at a time, which will make the following
state transitions:

VA_CLIENT_IDLE -  VA_CLIENT_SEND (job queued, send channel open)
VA_CLIENT_SEND -  VA_CLIENT_WAIT (request sent, awaiting response)
VA_CLIENT_WAIT -  VA_CLIENT_IDLE (response recieved, callbacks made)

A server job consists of recieving an rpc request, generating a
response, then sending the response. We expect to receive one server
request at a time due to the 1 at a time restriction for client jobs.
Server jobs make the following transitions:

VA_SERVER_IDLE -  VA_SERVER_WAIT (recieved/executed request, send
channel busy, response deferred)
VA_SERVER_IDLE -  VA_SERVER_SEND (recieved/executed request, send
channel open, sending response)
VA_SERVER_WAIT -  VA_SERVER_SEND (send channel now open, sending
response)
VA_SERVER_SEND -  VA_SERVER_IDLE (response sent)

Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com


As mentioned before, I really don't understand why this is part of QEMU,
the guest agent really should be able to run totally outside of QEMU.


+
+#define DEBUG_VA
+
+#ifdef DEBUG_VA
+#define TRACE(msg, ...) do { \
+fprintf(stderr, %s:%s():L%d:  msg \n, \
+__FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
+} while(0)
+#else
+#define TRACE(msg, ...) \
+do { } while (0)
+#endif
+
+#define LOG(msg, ...) do { \
+fprintf(stderr, %s:%s():  msg \n, \
+__FILE__, __FUNCTION__, ## __VA_ARGS__); \
+} while(0)


This must be like the 217th copy of these functions, could you please
use some of the code that is already in the tree, and make it generic if
needed.


+
+#define VERSION 1.0
+#define EOL \r\n
+
+#define VA_HDR_LEN_MAX 4096 /* http header limit */
+#define VA_CONTENT_LEN_MAX 2*1024*1024 /* rpc/http send limit */
+#define VA_CLIENT_JOBS_MAX 5 /* max client rpcs we can queue */
+#define VA_SERVER_JOBS_MAX 1 /* max server rpcs we can queue */


As mentioned last time, please make this stuff configurable and not hard
coded.



Yup, definitely on the TODO. Should be in the next round.


Cheers,
Jes






Re: [Qemu-devel] KVM call agenda for Dec 7

2010-12-07 Thread Jes Sorensen
On 12/07/10 00:51, Chris Wright wrote:
 Please send in any agenda items you are interested in covering.
 
 thanks,
 -chris
 

No agenda, no replies

Call canceled I presume?

Jes



Re: [Qemu-devel] KVM call agenda for Dec 7

2010-12-07 Thread Chris Wright
* Jes Sorensen (jes.soren...@redhat.com) wrote:
 On 12/07/10 00:51, Chris Wright wrote:
  Please send in any agenda items you are interested in covering.
  
  thanks,
  -chris
  
 
 No agenda, no replies
 
 Call canceled I presume?

Indeed, next week, then pick up next year...



[Qemu-devel] Re: arm: fix ldrexd/strexd

2010-12-07 Thread Nathan Froyd
On Thu, Nov 04, 2010 at 07:47:45AM -, Peter Maydell wrote:
 Correct ldrexd and strexd code to always read and write the
 high word of the 64-bit value from addr+4.
 Also make ldrexd and strexd agree that for a 64 bit value the
 address in env-exclusive_addr is that of the low word.
 
 This fixes the issues reported in
 https://bugs.launchpad.net/qemu/+bug/670883
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org

Reviewed-by: Nathan Froyd froy...@codesourcery.com

-Nathan



[Qemu-devel] [Bug 686613] [NEW] USB MSD are not marked as removable

2010-12-07 Thread Justin M. Forbes
Public bug reported:

 Filed from Fedora bug:
https://bugzilla.redhat.com/show_bug.cgi?id=589130

Guests can access USB Mass Storage Device, but fail to mark them as
removable.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/686613

Title:
  USB MSD are not marked as removable

Status in QEMU:
  New

Bug description:
   Filed from Fedora bug: https://bugzilla.redhat.com/show_bug.cgi?id=589130

Guests can access USB Mass Storage Device, but fail to mark them as removable.





[Qemu-devel] [PATCH 04/14] ARM: fix ldrexd/strexd

2010-12-07 Thread Peter Maydell
Correct ldrexd and strexd code to always read and write the
high word of the 64-bit value from addr+4.
Also make ldrexd and strexd agree that for a 64 bit value the
address in env-exclusive_addr is that of the low word.

This fixes the issues reported in
https://bugs.launchpad.net/qemu/+bug/670883

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 linux-user/main.c  |2 +-
 target-arm/translate.c |8 +---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 7d41d4a..0d627d6 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -589,7 +589,7 @@ static int do_strex(CPUARMState *env)
 }
 if (size == 3) {
 val = env-regs[(env-exclusive_info  12)  0xf];
-segv = put_user_u32(val, addr);
+segv = put_user_u32(val, addr + 4);
 if (segv) {
 env-cp15.c6_data = addr + 4;
 goto done;
diff --git a/target-arm/translate.c b/target-arm/translate.c
index bf1e643..7ee5375 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -5926,8 +5926,10 @@ static void gen_load_exclusive(DisasContext *s, int rt, 
int rt2,
 tcg_gen_mov_i32(cpu_exclusive_val, tmp);
 store_reg(s, rt, tmp);
 if (size == 3) {
-tcg_gen_addi_i32(addr, addr, 4);
-tmp = gen_ld32(addr, IS_USER(s));
+TCGv tmp2 = new_tmp();
+tcg_gen_addi_i32(tmp2, addr, 4);
+tmp = gen_ld32(tmp2, IS_USER(s));
+dead_tmp(tmp2);
 tcg_gen_mov_i32(cpu_exclusive_high, tmp);
 store_reg(s, rt2, tmp);
 }
@@ -5987,7 +5989,7 @@ static void gen_store_exclusive(DisasContext *s, int rd, 
int rt, int rt2,
 if (size == 3) {
 TCGv tmp2 = new_tmp();
 tcg_gen_addi_i32(tmp2, addr, 4);
-tmp = gen_ld32(addr, IS_USER(s));
+tmp = gen_ld32(tmp2, IS_USER(s));
 dead_tmp(tmp2);
 tcg_gen_brcond_i32(TCG_COND_NE, tmp, cpu_exclusive_high, fail_label);
 dead_tmp(tmp);
-- 
1.6.3.3




[Qemu-devel] [PATCH 11/14] ARM: Return correct result for single-double conversion of NaN

2010-12-07 Thread Peter Maydell
The ARM ARM defines that if the input to a single-double conversion
is a NaN then the output is always forced to be a quiet NaN by setting
the most significant bit of the fraction part.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/helper.c |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 6d2a8f2..4bd1cd4 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2528,12 +2528,20 @@ float32 VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
 /* floating point conversion */
 float64 VFP_HELPER(fcvtd, s)(float32 x, CPUState *env)
 {
-return float32_to_float64(x, env-vfp.fp_status);
+float64 r = float32_to_float64(x, env-vfp.fp_status);
+/* ARM requires that S-D conversion of any kind of NaN generates
+ * a quiet NaN by forcing the most significant frac bit to 1.
+ */
+return float64_maybe_silence_nan(r);
 }
 
 float32 VFP_HELPER(fcvts, d)(float64 x, CPUState *env)
 {
-return float64_to_float32(x, env-vfp.fp_status);
+float32 r =  float64_to_float32(x, env-vfp.fp_status);
+/* ARM requires that S-D conversion of any kind of NaN generates
+ * a quiet NaN by forcing the most significant frac bit to 1.
+ */
+return float32_maybe_silence_nan(r);
 }
 
 /* VFP3 fixed point conversion.  */
-- 
1.6.3.3




[Qemu-devel] [PATCH 05/14] ARM: Fix decoding of VFP forms of VCVT between float and int/fixed

2010-12-07 Thread Peter Maydell
Correct the decoding of source and destination registers
for the VFP forms of the VCVT instructions which convert
between floating point and integer or fixed-point.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |   19 ---
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 7ee5375..69a424a 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2870,16 +2870,18 @@ static int disas_vfp_insn(CPUState * env, DisasContext 
*s, uint32_t insn)
 VFP_DREG_N(rn, insn);
 }
 
-if (op == 15  (rn == 15 || rn  17)) {
+if (op == 15  (rn == 15 || ((rn  0x1c) == 0x18))) {
 /* Integer or single precision destination.  */
 rd = VFP_SREG_D(insn);
 } else {
 VFP_DREG_D(rd, insn);
 }
-
-if (op == 15  (rn == 16 || rn == 17)) {
-/* Integer source.  */
-rm = ((insn  1)  0x1e) | ((insn  5)  1);
+if (op == 15 
+(((rn  0x1c) == 0x10) || ((rn  0x14) == 0x14))) {
+/* VCVT from int is always from S reg regardless of dp bit.
+ * VCVT with immediate frac_bits has same format as SREG_M
+ */
+rm = VFP_SREG_M(insn);
 } else {
 VFP_DREG_M(rm, insn);
 }
@@ -2891,6 +2893,9 @@ static int disas_vfp_insn(CPUState * env, DisasContext 
*s, uint32_t insn)
 } else {
 rd = VFP_SREG_D(insn);
 }
+/* NB that we implicitly rely on the encoding for the frac_bits
+ * in VCVT of fixed to float being the same as that of an 
SREG_M
+ */
 rm = VFP_SREG_M(insn);
 }
 
@@ -3179,8 +3184,8 @@ static int disas_vfp_insn(CPUState * env, DisasContext 
*s, uint32_t insn)
 /* Write back the result.  */
 if (op == 15  (rn = 8  rn = 11))
 ; /* Comparison, do nothing.  */
-else if (op == 15  rn  17)
-/* Integer result.  */
+else if (op == 15  dp  ((rn  0x1c) == 0x18))
+/* VCVT double to int: always integer result. */
 gen_mov_vreg_F0(0, rd);
 else if (op == 15  rn == 15)
 /* conversion */
-- 
1.6.3.3




[Qemu-devel] [PATCH 00/14] [PULL] ARM fixes, v2

2010-12-07 Thread Peter Maydell
Anthony,

This is a pull request for patches which fix problems with the
generated code for ARM targets. This updates the VCVT patches
as reviewed by Nathan Froyd; all the included patches have
Reviewed-by: tags from him.

They've been sent to the list previously, and I've reviewed and
tested them. Could they be merged, please?

Adam Lackorzynski (1):
  target-arm: Handle 'smc' as an undefined instruction

Johan Bengtsson (2):
  target-arm: Add support for PKHxx in thumb2
  target-arm: Fix mixup in decoding of saturating add and sub

Peter Maydell (11):
  ARM: fix ldrexd/strexd
  ARM: Fix decoding of VFP forms of VCVT between float and int/fixed
  ARM: Fix decoding of Neon forms of VCVT between float and fixed point
  ARM: Fix sense of to_integer bit in Neon VCVT float/int conversion
  softfloat: Add float*_is_any_nan() functions
  ARM: Return correct result for float-to-integer conversion of NaN
  softfloat: Add float*_maybe_silence_nan() functions
  ARM: Return correct result for single-double conversion of NaN
  ARM: Ignore top 16 bits when doing VCVT from 16 bit fixed point
  softfloat: Add float/double to 16 bit integer conversion functions
  ARM: Implement VCVT to 16 bit integer using new softfloat routines

 fpu/softfloat-specialize.h |   38 
 fpu/softfloat.c|  136 
 fpu/softfloat.h|   16 +
 linux-user/main.c  |2 +-
 target-arm/helper.c|   43 +-
 target-arm/translate.c |  119 ++
 6 files changed, 312 insertions(+), 42 deletions(-)




[Qemu-devel] [PATCH 14/14] ARM: Implement VCVT to 16 bit integer using new softfloat routines

2010-12-07 Thread Peter Maydell
Use the softfloat conversion routines for conversion to 16 bit
integers, because just casting to a 16 bit type truncates the
value rather than saturating it at 16-bit MAXINT/MININT.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2925782..9ba2f4f 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2560,7 +2560,7 @@ ftype VFP_HELPER(to##name, p)(ftype x, uint32_t shift, 
CPUState *env) \
 return ftype##_zero; \
 } \
 tmp = ftype##_scalbn(x, shift, env-vfp.fp_status); \
-return vfp_ito##p((itype)ftype##_to_##sign##int32_round_to_zero(tmp, \
+return vfp_ito##p(ftype##_to_##itype##_round_to_zero(tmp, \
 env-vfp.fp_status)); \
 }
 
-- 
1.6.3.3




[Qemu-devel] [PATCH 13/14] softfloat: Add float/double to 16 bit integer conversion functions

2010-12-07 Thread Peter Maydell
The ARM architecture needs float/double to 16 bit integer conversions.
(The 32 bit versions aren't sufficient because of the requirement
to saturate at 16 bit MAXINT/MININT and to get the exception bits right.)

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 fpu/softfloat.c |  136 +++
 fpu/softfloat.h |4 ++
 2 files changed, 140 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 0b82797..6f5b05d 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1355,6 +1355,55 @@ int32 float32_to_int32_round_to_zero( float32 a 
STATUS_PARAM )
 
 /*
 | Returns the result of converting the single-precision floating-point value
+| `a' to the 16-bit two's complement integer format.  The conversion is
+| performed according to the IEC/IEEE Standard for Binary Floating-Point
+| Arithmetic, except that the conversion is always rounded toward zero.
+| If `a' is a NaN, the largest positive integer is returned.  Otherwise, if
+| the conversion overflows, the largest integer with the same sign as `a' is
+| returned.
+**/
+
+int16 float32_to_int16_round_to_zero( float32 a STATUS_PARAM )
+{
+flag aSign;
+int16 aExp, shiftCount;
+bits32 aSig;
+int32 z;
+
+aSig = extractFloat32Frac( a );
+aExp = extractFloat32Exp( a );
+aSign = extractFloat32Sign( a );
+shiftCount = aExp - 0x8E;
+if ( 0 = shiftCount ) {
+if ( float32_val(a) != 0xC700 ) {
+float_raise( float_flag_invalid STATUS_VAR);
+if ( ! aSign || ( ( aExp == 0xFF )  aSig ) ) {
+return 0x7FFF;
+}
+}
+return (sbits32) 0x8000;
+}
+else if ( aExp = 0x7E ) {
+if ( aExp | aSig ) {
+STATUS(float_exception_flags) |= float_flag_inexact;
+}
+return 0;
+}
+shiftCount -= 0x10;
+aSig = ( aSig | 0x0080 )8;
+z = aSig( - shiftCount );
+if ( (bits32) ( aSig( shiftCount  31 ) ) ) {
+STATUS(float_exception_flags) |= float_flag_inexact;
+}
+if ( aSign ) {
+z = - z;
+}
+return z;
+
+}
+
+/*
+| Returns the result of converting the single-precision floating-point value
 | `a' to the 64-bit two's complement integer format.  The conversion is
 | performed according to the IEC/IEEE Standard for Binary Floating-Point
 | Arithmetic---which means in particular that the conversion is rounded
@@ -2412,6 +2461,57 @@ int32 float64_to_int32_round_to_zero( float64 a 
STATUS_PARAM )
 
 /*
 | Returns the result of converting the double-precision floating-point value
+| `a' to the 16-bit two's complement integer format.  The conversion is
+| performed according to the IEC/IEEE Standard for Binary Floating-Point
+| Arithmetic, except that the conversion is always rounded toward zero.
+| If `a' is a NaN, the largest positive integer is returned.  Otherwise, if
+| the conversion overflows, the largest integer with the same sign as `a' is
+| returned.
+**/
+
+int16 float64_to_int16_round_to_zero( float64 a STATUS_PARAM )
+{
+flag aSign;
+int16 aExp, shiftCount;
+bits64 aSig, savedASig;
+int32 z;
+
+aSig = extractFloat64Frac( a );
+aExp = extractFloat64Exp( a );
+aSign = extractFloat64Sign( a );
+if ( 0x40E  aExp ) {
+if ( ( aExp == 0x7FF )  aSig ) {
+aSign = 0;
+}
+goto invalid;
+}
+else if ( aExp  0x3FF ) {
+if ( aExp || aSig ) {
+STATUS(float_exception_flags) |= float_flag_inexact;
+}
+return 0;
+}
+aSig |= LIT64( 0x0010 );
+shiftCount = 0x433 - aExp;
+savedASig = aSig;
+aSig = shiftCount;
+z = aSig;
+if ( aSign ) {
+z = - z;
+}
+if ( ( (int16_t)z  0 ) ^ aSign ) {
+ invalid:
+float_raise( float_flag_invalid STATUS_VAR);
+return aSign ? (sbits32) 0x8000 : 0x7FFF;
+}
+if ( ( aSigshiftCount ) != savedASig ) {
+STATUS(float_exception_flags) |= float_flag_inexact;
+}
+return z;
+}
+
+/*
+| Returns the result of converting the double-precision floating-point value
 | `a' to the 64-bit two's complement integer format.  The conversion is
 | performed according to the IEC/IEEE Standard for Binary Floating-Point
 | Arithmetic---which means in particular that the conversion is rounded
@@ -5632,6 +5732,24 @@ unsigned int float32_to_uint32_round_to_zero( float32 a 
STATUS_PARAM )
 return res;
 }
 
+unsigned 

[Qemu-devel] [PATCH 10/14] softfloat: Add float*_maybe_silence_nan() functions

2010-12-07 Thread Peter Maydell
Add functions float*_maybe_silence_nan() which ensure that a
value is not a signaling NaN by turning it into a quiet NaN.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 fpu/softfloat-specialize.h |   38 ++
 fpu/softfloat.h|2 ++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 8e6aceb..0746878 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -102,6 +102,25 @@ int float32_is_signaling_nan( float32 a_ )
 }
 
 /*
+| Returns a quiet NaN if the single-precision floating point value `a' is a
+| signaling NaN; otherwise returns `a'.
+**/
+
+float32 float32_maybe_silence_nan( float32 a_ )
+{
+if (float32_is_signaling_nan(a_)) {
+uint32_t a = float32_val(a_);
+#if SNAN_BIT_IS_ONE
+a = ~(1  22);
+#else
+a |= (1  22);
+#endif
+return make_float32(a);
+}
+return a_;
+}
+
+/*
 | Returns the result of converting the single-precision floating-point NaN
 | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
 | exception is raised.
@@ -234,6 +253,25 @@ int float64_is_signaling_nan( float64 a_ )
 }
 
 /*
+| Returns a quiet NaN if the double-precision floating point value `a' is a
+| signaling NaN; otherwise returns `a'.
+**/
+
+float64 float64_maybe_silence_nan( float64 a_ )
+{
+if (float64_is_signaling_nan(a_)) {
+bits64 a = float64_val(a_);
+#if SNAN_BIT_IS_ONE
+a = ~LIT64( 0x0008 );
+#else
+a |= LIT64( 0x0008 );
+#endif
+return make_float64(a);
+}
+return a_;
+}
+
+/*
 | Returns the result of converting the double-precision floating-point NaN
 | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
 | exception is raised.
diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 9bece80..2e651e2 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -287,6 +287,7 @@ int float32_compare( float32, float32 STATUS_PARAM );
 int float32_compare_quiet( float32, float32 STATUS_PARAM );
 int float32_is_nan( float32 );
 int float32_is_signaling_nan( float32 );
+float32 float32_maybe_silence_nan( float32 );
 float32 float32_scalbn( float32, int STATUS_PARAM );
 
 INLINE float32 float32_abs(float32 a)
@@ -364,6 +365,7 @@ int float64_compare( float64, float64 STATUS_PARAM );
 int float64_compare_quiet( float64, float64 STATUS_PARAM );
 int float64_is_nan( float64 a );
 int float64_is_signaling_nan( float64 );
+float64 float64_maybe_silence_nan( float64 );
 float64 float64_scalbn( float64, int STATUS_PARAM );
 
 INLINE float64 float64_abs(float64 a)
-- 
1.6.3.3




[Qemu-devel] [PATCH 09/14] ARM: Return correct result for float-to-integer conversion of NaN

2010-12-07 Thread Peter Maydell
The ARM architecture mandates that converting a NaN value to
integer gives zero (if Invalid Operation FP exceptions are
not being trapped). This isn't the behaviour of the SoftFloat
library, so NaNs must be special-cased.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/helper.c |   27 +++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2a1f448..6d2a8f2 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2463,41 +2463,65 @@ float64 VFP_HELPER(sito, d)(float32 x, CPUState *env)
 /* Float to integer conversion.  */
 float32 VFP_HELPER(toui, s)(float32 x, CPUState *env)
 {
+if (float32_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float32_to_uint32(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(toui, d)(float64 x, CPUState *env)
 {
+if (float64_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float64_to_uint32(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(tosi, s)(float32 x, CPUState *env)
 {
+if (float32_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float32_to_int32(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(tosi, d)(float64 x, CPUState *env)
 {
+if (float64_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float64_to_int32(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(touiz, s)(float32 x, CPUState *env)
 {
+if (float32_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float32_to_uint32_round_to_zero(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(touiz, d)(float64 x, CPUState *env)
 {
+if (float64_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float64_to_uint32_round_to_zero(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(tosiz, s)(float32 x, CPUState *env)
 {
+if (float32_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float32_to_int32_round_to_zero(x, env-vfp.fp_status));
 }
 
 float32 VFP_HELPER(tosiz, d)(float64 x, CPUState *env)
 {
+if (float64_is_any_nan(x)) {
+return float32_zero;
+}
 return vfp_itos(float64_to_int32_round_to_zero(x, env-vfp.fp_status));
 }
 
@@ -2524,6 +2548,9 @@ ftype VFP_HELPER(name##to, p)(ftype x, uint32_t shift, 
CPUState *env) \
 ftype VFP_HELPER(to##name, p)(ftype x, uint32_t shift, CPUState *env) \
 { \
 ftype tmp; \
+if (ftype##_is_any_nan(x)) { \
+return ftype##_zero; \
+} \
 tmp = ftype##_scalbn(x, shift, env-vfp.fp_status); \
 return vfp_ito##p((itype)ftype##_to_##sign##int32_round_to_zero(tmp, \
 env-vfp.fp_status)); \
-- 
1.6.3.3




[Qemu-devel] [PATCH 03/14] target-arm: Handle 'smc' as an undefined instruction

2010-12-07 Thread Peter Maydell
From: Adam Lackorzynski a...@os.inf.tu-dresden.de

Refine check on bkpt so that smc and undefined instruction encodings are
handled as an undefined instruction and trap.

Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |9 -
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 947de6d..bf1e643 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -6346,7 +6346,14 @@ static void disas_arm_insn(CPUState * env, DisasContext 
*s)
 dead_tmp(tmp2);
 store_reg(s, rd, tmp);
 break;
-case 7: /* bkpt */
+case 7:
+/* SMC instruction (op1 == 3)
+   and undefined instructions (op1 == 0 || op1 == 2)
+   will trap */
+if (op1 != 1) {
+goto illegal_op;
+}
+/* bkpt */
 gen_set_condexec(s);
 gen_set_pc_im(s-pc - 4);
 gen_exception(EXCP_BKPT);
-- 
1.6.3.3




[Qemu-devel] [PATCH 02/14] target-arm: Fix mixup in decoding of saturating add and sub

2010-12-07 Thread Peter Maydell
From: Johan Bengtsson teofrast...@gmail.com

The thumb2 decoder contained a mixup between the bit controlling
doubling and the bit controlling if the operation was an add or a sub.

Signed-off-by: Johan Bengtsson teofrast...@gmail.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 183928b..947de6d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7713,9 +7713,9 @@ static int disas_thumb2_insn(CPUState *env, DisasContext 
*s, uint16_t insn_hw1)
 /* Saturating add/subtract.  */
 tmp = load_reg(s, rn);
 tmp2 = load_reg(s, rm);
-if (op  2)
-gen_helper_double_saturate(tmp, tmp);
 if (op  1)
+gen_helper_double_saturate(tmp, tmp);
+if (op  2)
 gen_helper_sub_saturate(tmp, tmp2, tmp);
 else
 gen_helper_add_saturate(tmp, tmp, tmp2);
-- 
1.6.3.3




[Qemu-devel] [PATCH 06/14] ARM: Fix decoding of Neon forms of VCVT between float and fixed point

2010-12-07 Thread Peter Maydell
Fix errors in the decoding of the Neon forms of fixed-point VCVT:
 * fixed-point VCVT is op 14 and 15, not 15 and 16
 * the fbits immediate field was being misinterpreted
 * the sense of the to_fixed bit was inverted

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |8 ++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 69a424a..0b3e4e6 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4850,11 +4850,15 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 }
 neon_store_reg64(cpu_V0, rd + pass);
 }
-} else if (op == 15 || op == 16) {
+} else if (op = 14) {
 /* VCVT fixed-point.  */
+/* We have already masked out the must-be-1 top bit of imm6,
+ * hence this 32-shift where the ARM ARM has 64-imm6.
+ */
+shift = 32 - shift;
 for (pass = 0; pass  (q ? 4 : 2); pass++) {
 tcg_gen_ld_f32(cpu_F0s, cpu_env, neon_reg_offset(rm, 
pass));
-if (op  1) {
+if (!(op  1)) {
 if (u)
 gen_vfp_ulto(0, shift);
 else
-- 
1.6.3.3




[Qemu-devel] [PATCH 12/14] ARM: Ignore top 16 bits when doing VCVT from 16 bit fixed point

2010-12-07 Thread Peter Maydell
VCVT of 16 bit fixed point to float should ignore the top 16 bits
of the source register. Cast to int16_t and friends rather than
int16 -- the former is guaranteed exactly 16 bits wide where the
latter is merely at least 16 bits wide (and so is usually 32 bits).

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/helper.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4bd1cd4..2925782 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2549,7 +2549,7 @@ float32 VFP_HELPER(fcvts, d)(float64 x, CPUState *env)
 ftype VFP_HELPER(name##to, p)(ftype x, uint32_t shift, CPUState *env) \
 { \
 ftype tmp; \
-tmp = sign##int32_to_##ftype ((itype)vfp_##p##toi(x), \
+tmp = sign##int32_to_##ftype ((itype##_t)vfp_##p##toi(x), \
   env-vfp.fp_status); \
 return ftype##_scalbn(tmp, -(int)shift, env-vfp.fp_status); \
 } \
-- 
1.6.3.3




Re: [Qemu-devel] [PATCH 00/14] [PULL] ARM fixes, v2

2010-12-07 Thread Peter Maydell
Embarrassingly, I managed to fail to include the critical item for a
pull request, ie the
git repo url. Here it is, with apologies for the error:


The following changes since commit 2c90fe2b71df2534884bce96d90cbfcc93aeedb8:
  Kirill Batuzov (1):
Speedup 'tb_find_slow' by using the same heuristic as during
memory page lookup

are available in the git repository at:

  git://git.linaro.org/qemu/qemu-arm.git for-anthony

On 7 December 2010 15:43, Peter Maydell peter.mayd...@linaro.org wrote:
 Anthony,

 This is a pull request for patches which fix problems with the
 generated code for ARM targets. This updates the VCVT patches
 as reviewed by Nathan Froyd; all the included patches have
 Reviewed-by: tags from him.

 They've been sent to the list previously, and I've reviewed and
 tested them. Could they be merged, please?

 Adam Lackorzynski (1):
  target-arm: Handle 'smc' as an undefined instruction

 Johan Bengtsson (2):
  target-arm: Add support for PKHxx in thumb2
  target-arm: Fix mixup in decoding of saturating add and sub

 Peter Maydell (11):
  ARM: fix ldrexd/strexd
  ARM: Fix decoding of VFP forms of VCVT between float and int/fixed
  ARM: Fix decoding of Neon forms of VCVT between float and fixed point
  ARM: Fix sense of to_integer bit in Neon VCVT float/int conversion
  softfloat: Add float*_is_any_nan() functions
  ARM: Return correct result for float-to-integer conversion of NaN
  softfloat: Add float*_maybe_silence_nan() functions
  ARM: Return correct result for single-double conversion of NaN
  ARM: Ignore top 16 bits when doing VCVT from 16 bit fixed point
  softfloat: Add float/double to 16 bit integer conversion functions
  ARM: Implement VCVT to 16 bit integer using new softfloat routines

  fpu/softfloat-specialize.h |   38 
  fpu/softfloat.c            |  136 
 
  fpu/softfloat.h            |   16 +
  linux-user/main.c          |    2 +-
  target-arm/helper.c        |   43 +-
  target-arm/translate.c     |  119 ++
  6 files changed, 312 insertions(+), 42 deletions(-)






[Qemu-devel] [PATCH 08/14] softfloat: Add float*_is_any_nan() functions

2010-12-07 Thread Peter Maydell
Add float*_is_any_nan() functions which return true if the argument
is a NaN of any kind (quiet or signalling).

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 fpu/softfloat.h |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 9528825..9bece80 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -314,6 +314,11 @@ INLINE int float32_is_zero(float32 a)
 return (float32_val(a)  0x7fff) == 0;
 }
 
+INLINE int float32_is_any_nan(float32 a)
+{
+return ((float32_val(a)  ~(1  31))  0x7f80UL);
+}
+
 #define float32_zero make_float32(0)
 #define float32_one make_float32(0x3f80)
 #define float32_ln2 make_float32(0x3f317218)
@@ -386,6 +391,11 @@ INLINE int float64_is_zero(float64 a)
 return (float64_val(a)  0x7fffLL) == 0;
 }
 
+INLINE int float64_is_any_nan(float64 a)
+{
+return ((float64_val(a)  ~(1ULL  63))  0x7ff0ULL);
+}
+
 #define float64_zero make_float64(0)
 #define float64_one make_float64(0x3ff0LL)
 #define float64_ln2 make_float64(0x3fe62e42fefa39efLL)
-- 
1.6.3.3




[Qemu-devel] [PATCH 01/14] target-arm: Add support for PKHxx in thumb2

2010-12-07 Thread Peter Maydell
From: Johan Bengtsson teofrast...@gmail.com

The PKHxx instructions were not recognized by the thumb2 decoder. The
solution provided in this changeset is identical to the arm-mode
implementation.

Signed-off-by: Johan Bengtsson teofrast...@gmail.com
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |   63 ++-
 1 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 99464ab..183928b 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -7601,27 +7601,54 @@ static int disas_thumb2_insn(CPUState *env, 
DisasContext *s, uint16_t insn_hw1)
 }
 }
 break;
-case 5: /* Data processing register constant shift.  */
-if (rn == 15) {
-tmp = new_tmp();
-tcg_gen_movi_i32(tmp, 0);
-} else {
-tmp = load_reg(s, rn);
-}
-tmp2 = load_reg(s, rm);
+case 5:
+
 op = (insn  21)  0xf;
-shiftop = (insn  4)  3;
-shift = ((insn  6)  3) | ((insn  10)  0x1c);
-conds = (insn  (1  20)) != 0;
-logic_cc = (conds  thumb2_logic_op(op));
-gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
-if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
-goto illegal_op;
-dead_tmp(tmp2);
-if (rd != 15) {
+if (op == 6) {
+/* Halfword pack.  */
+tmp = load_reg(s, rn);
+tmp2 = load_reg(s, rm);
+shift = ((insn  10)  0x1c) | ((insn  6)  0x3);
+if (insn  (1  5)) {
+/* pkhtb */
+if (shift == 0)
+shift = 31;
+tcg_gen_sari_i32(tmp2, tmp2, shift);
+tcg_gen_andi_i32(tmp, tmp, 0x);
+tcg_gen_ext16u_i32(tmp2, tmp2);
+} else {
+/* pkhbt */
+if (shift)
+tcg_gen_shli_i32(tmp2, tmp2, shift);
+tcg_gen_ext16u_i32(tmp, tmp);
+tcg_gen_andi_i32(tmp2, tmp2, 0x);
+}
+tcg_gen_or_i32(tmp, tmp, tmp2);
+dead_tmp(tmp2);
 store_reg(s, rd, tmp);
 } else {
-dead_tmp(tmp);
+/* Data processing register constant shift.  */
+if (rn == 15) {
+tmp = new_tmp();
+tcg_gen_movi_i32(tmp, 0);
+} else {
+tmp = load_reg(s, rn);
+}
+tmp2 = load_reg(s, rm);
+
+shiftop = (insn  4)  3;
+shift = ((insn  6)  3) | ((insn  10)  0x1c);
+conds = (insn  (1  20)) != 0;
+logic_cc = (conds  thumb2_logic_op(op));
+gen_arm_shift_im(tmp2, shiftop, shift, logic_cc);
+if (gen_thumb2_data_op(s, op, conds, 0, tmp, tmp2))
+goto illegal_op;
+dead_tmp(tmp2);
+if (rd != 15) {
+store_reg(s, rd, tmp);
+} else {
+dead_tmp(tmp);
+}
 }
 break;
 case 13: /* Misc data processing.  */
-- 
1.6.3.3




[Qemu-devel] [PATCH 07/14] ARM: Fix sense of to_integer bit in Neon VCVT float/int conversion

2010-12-07 Thread Peter Maydell
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
Reviewed-by: Nathan Froyd froy...@codesourcery.com
---
 target-arm/translate.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 0b3e4e6..d4a0666 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -5664,16 +5664,16 @@ static int disas_neon_data_insn(CPUState * env, 
DisasContext *s, uint32_t insn)
 gen_helper_rsqrte_f32(cpu_F0s, cpu_F0s, cpu_env);
 break;
 case 60: /* VCVT.F32.S32 */
-gen_vfp_tosiz(0);
+gen_vfp_sito(0);
 break;
 case 61: /* VCVT.F32.U32 */
-gen_vfp_touiz(0);
+gen_vfp_uito(0);
 break;
 case 62: /* VCVT.S32.F32 */
-gen_vfp_sito(0);
+gen_vfp_tosiz(0);
 break;
 case 63: /* VCVT.U32.F32 */
-gen_vfp_uito(0);
+gen_vfp_touiz(0);
 break;
 default:
 /* Reserved: 21, 29, 39-56 */
-- 
1.6.3.3




[Qemu-devel] Re: [PATCH 1/1] qemu-img: Deprecate obsolete -6 and -e options

2010-12-07 Thread Kevin Wolf
Am 07.12.2010 12:31, schrieb jes.soren...@redhat.com:
 From: Jes Sorensen jes.soren...@redhat.com
 
 If -6 or -e is specified, an error message is printed and we exit. It
 does not print help() to avoid the error message getting lost in the
 noise.
 
 Signed-off-by: Jes Sorensen jes.soren...@redhat.com
 ---
  block_int.h |1 -
  qemu-img.c  |   53 ++---
  2 files changed, 22 insertions(+), 32 deletions(-)
 
 diff --git a/block_int.h b/block_int.h
 index 3c3adb5..3ceed47 100644
 --- a/block_int.h
 +++ b/block_int.h
 @@ -29,7 +29,6 @@
  #include qemu-queue.h
  
  #define BLOCK_FLAG_ENCRYPT   1
 -#define BLOCK_FLAG_COMPRESS  2
  #define BLOCK_FLAG_COMPAT6   4
  
  #define BLOCK_OPT_SIZE  size
 diff --git a/qemu-img.c b/qemu-img.c
 index 5b6e648..16fec40 100644
 --- a/qemu-img.c
 +++ b/qemu-img.c
 @@ -261,21 +261,9 @@ fail:
  }
  
  static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
 -int flags, const char *base_filename, const char *base_fmt)
 + const char *base_filename,
 + const char *base_fmt)
  {
 -if (flags  BLOCK_FLAG_ENCRYPT) {
 -if (set_option_parameter(list, BLOCK_OPT_ENCRYPT, on)) {
 -error(Encryption not supported for file format '%s', fmt);
 -return -1;
 -}
 -}
 -if (flags  BLOCK_FLAG_COMPAT6) {
 -if (set_option_parameter(list, BLOCK_OPT_COMPAT6, on)) {
 -error(VMDK version 6 not supported for file format '%s', fmt);
 -return -1;
 -}
 -}
 -
  if (base_filename) {
  if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, 
 base_filename)) {
  error(Backing file not supported for file format '%s', fmt);
 @@ -293,7 +281,7 @@ static int add_old_style_options(const char *fmt, 
 QEMUOptionParameter *list,
  
  static int img_create(int argc, char **argv)
  {
 -int c, ret = 0, flags;
 +int c, ret = 0;
  const char *fmt = raw;
  const char *base_fmt = NULL;
  const char *filename;
 @@ -302,7 +290,6 @@ static int img_create(int argc, char **argv)
  QEMUOptionParameter *param = NULL, *create_options = NULL;
  char *options = NULL;
  
 -flags = 0;
  for(;;) {
  c = getopt(argc, argv, F:b:f:he6o:);
  if (c == -1) {
 @@ -323,11 +310,13 @@ static int img_create(int argc, char **argv)
  fmt = optarg;
  break;
  case 'e':
 -flags |= BLOCK_FLAG_ENCRYPT;
 -break;
 +printf(qemu-img: option -e is deprecated, please use \'-o 
 +   encryption\' instead!\n);
 +return -1;

The return value of this function is used as exit code of qemu-img, so 1
is probably better than -1.

Also, is there a reason why you use printf and not error (which writes
the message to stderr)?

Kevin



[Qemu-devel] Re: [RFC][PATCH v5 07/21] virtagent: add va.getfile RPC

2010-12-07 Thread Adam Litke
Hi Jes, you raise some good points and pitfalls with the current getfile
approach.  I've been thinking about an alternative and am wondering what
you (and others) think...

First off, I think we should switch to a copyfile() API that allows us
to avoid presenting the file contents to the user.  Neither the human
monitor nor the control monitor are designed to be file pagers.  Let the
user decide how to consume the data once it has been transferred.  Now
we don't need to care if the file is binary or text.

The virtagent RPC protocol is bi-directional and supports asynchronous
events.  We can use these to implement a better copyfile RPC that can
transfer larger files without wasting memory.  The host issues a
copyfile(guest-path, host-path) RPC.  The immediate result of this
call will indicate whether the guest is able to initiate the transfer.
The guest will generate a series of events (offset, size, payload)
until the entire contents has been transferred.  The host and guest
could negotiate the chunk size if necessary.  Once the transfer is
complete, the guest sends a final event to indicate this (file-size,
0).

This interface could be integrated into the monitor with a pair of
commands (va_copyfile and info va_copyfile), the former used to initiate
transfers and the latter to check on the status.

Thoughts on this?

On Tue, 2010-12-07 at 15:18 +0100, Jes Sorensen wrote:
 On 12/03/10 19:03, Michael Roth wrote:
  Add RPC to retrieve a guest file. This interface is intended
  for smaller reads like peeking at logs and /proc and such.
 
 I think you need to redesign your approach here. see below.
 
 In 06/21 you had:
 
 +#define VA_GETFILE_MAX 1  30
 
  +while ((ret = read(fd, buf, VA_FILEBUF_LEN))  0) {
  +file_contents = qemu_realloc(file_contents, count + 
  VA_FILEBUF_LEN);
  +memcpy(file_contents + count, buf, ret);
 
 UH OH!
 
 realloc will do a malloc and a memcpy of the data, this is going to turn
 into a really nasty malloc memcpy loop if someone tries to transfer a
 large file using this method. You could end up with almost 4GB of
 parallel allocations for a guest that might have been configured as a
 1GB guest. This would allow the guest to effectively blow the expected
 memory consumption out of the water. It's not exactly going to be fast
 either :(
 
 Maybe use a tmp file, and write data out to that as you receive it to
 avoid the malloc ballooning.
 
 Jes

-- 
Thanks,
Adam




[Qemu-devel] Re: [PATCH 1/1] qemu-img: Deprecate obsolete -6 and -e options

2010-12-07 Thread Jes Sorensen
On 12/07/10 17:02, Kevin Wolf wrote:
 @@ -323,11 +310,13 @@ static int img_create(int argc, char **argv)
  fmt = optarg;
  break;
  case 'e':
 -flags |= BLOCK_FLAG_ENCRYPT;
 -break;
 +printf(qemu-img: option -e is deprecated, please use \'-o 
 +   encryption\' instead!\n);
 +return -1;
 
 The return value of this function is used as exit code of qemu-img, so 1
 is probably better than -1.
 
 Also, is there a reason why you use printf and not error (which writes
 the message to stderr)?

I looked for fprintf(stderr and found nothing so I used printf()
instead. I'm happy to change it to use error() and the return value too.

Thanks for the feedback.

Cheers,
Jes



[Qemu-devel] [PATCH v2 1/1] qemu-img: Deprecate obsolete -6 and -e options

2010-12-07 Thread Jes . Sorensen
From: Jes Sorensen jes.soren...@redhat.com

If -6 or -e is specified, an error message is printed and we exit. It
does not print help() to avoid the error message getting lost in the
noise.

Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
 block_int.h |1 -
 qemu-img.c  |   53 ++---
 2 files changed, 22 insertions(+), 32 deletions(-)

diff --git a/block_int.h b/block_int.h
index 3c3adb5..3ceed47 100644
--- a/block_int.h
+++ b/block_int.h
@@ -29,7 +29,6 @@
 #include qemu-queue.h
 
 #define BLOCK_FLAG_ENCRYPT 1
-#define BLOCK_FLAG_COMPRESS2
 #define BLOCK_FLAG_COMPAT6 4
 
 #define BLOCK_OPT_SIZE  size
diff --git a/qemu-img.c b/qemu-img.c
index 5b6e648..d146d8c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -261,21 +261,9 @@ fail:
 }
 
 static int add_old_style_options(const char *fmt, QEMUOptionParameter *list,
-int flags, const char *base_filename, const char *base_fmt)
+ const char *base_filename,
+ const char *base_fmt)
 {
-if (flags  BLOCK_FLAG_ENCRYPT) {
-if (set_option_parameter(list, BLOCK_OPT_ENCRYPT, on)) {
-error(Encryption not supported for file format '%s', fmt);
-return -1;
-}
-}
-if (flags  BLOCK_FLAG_COMPAT6) {
-if (set_option_parameter(list, BLOCK_OPT_COMPAT6, on)) {
-error(VMDK version 6 not supported for file format '%s', fmt);
-return -1;
-}
-}
-
 if (base_filename) {
 if (set_option_parameter(list, BLOCK_OPT_BACKING_FILE, base_filename)) 
{
 error(Backing file not supported for file format '%s', fmt);
@@ -293,7 +281,7 @@ static int add_old_style_options(const char *fmt, 
QEMUOptionParameter *list,
 
 static int img_create(int argc, char **argv)
 {
-int c, ret = 0, flags;
+int c, ret = 0;
 const char *fmt = raw;
 const char *base_fmt = NULL;
 const char *filename;
@@ -302,7 +290,6 @@ static int img_create(int argc, char **argv)
 QEMUOptionParameter *param = NULL, *create_options = NULL;
 char *options = NULL;
 
-flags = 0;
 for(;;) {
 c = getopt(argc, argv, F:b:f:he6o:);
 if (c == -1) {
@@ -323,11 +310,13 @@ static int img_create(int argc, char **argv)
 fmt = optarg;
 break;
 case 'e':
-flags |= BLOCK_FLAG_ENCRYPT;
-break;
+error(qemu-img: option -e is deprecated, please use \'-o 
+  encryption\' instead!);
+return 1;
 case '6':
-flags |= BLOCK_FLAG_COMPAT6;
-break;
+error(qemu-img: option -6 is deprecated, please use \'-o 
+  compat6\' instead!);
+return 1;
 case 'o':
 options = optarg;
 break;
@@ -385,7 +374,7 @@ static int img_create(int argc, char **argv)
 }
 
 /* Add old-style options to parameters */
-ret = add_old_style_options(fmt, param, flags, base_filename, base_fmt);
+ret = add_old_style_options(fmt, param, base_filename, base_fmt);
 if (ret  0) {
 goto out;
 }
@@ -674,7 +663,7 @@ static int compare_sectors(const uint8_t *buf1, const 
uint8_t *buf2, int n,
 
 static int img_convert(int argc, char **argv)
 {
-int c, ret = 0, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
+int c, ret = 0, n, n1, bs_n, bs_i, compress, cluster_size, cluster_sectors;
 const char *fmt, *out_fmt, *out_baseimg, *out_filename;
 BlockDriver *drv, *proto_drv;
 BlockDriverState **bs = NULL, *out_bs = NULL;
@@ -691,7 +680,7 @@ static int img_convert(int argc, char **argv)
 fmt = NULL;
 out_fmt = raw;
 out_baseimg = NULL;
-flags = 0;
+compress = 0;
 for(;;) {
 c = getopt(argc, argv, f:O:B:s:hce6o:);
 if (c == -1) {
@@ -712,14 +701,16 @@ static int img_convert(int argc, char **argv)
 out_baseimg = optarg;
 break;
 case 'c':
-flags |= BLOCK_FLAG_COMPRESS;
+compress = 1;
 break;
 case 'e':
-flags |= BLOCK_FLAG_ENCRYPT;
-break;
+error(qemu-img: option -e is deprecated, please use \'-o 
+  encryption\' instead!);
+return 1;
 case '6':
-flags |= BLOCK_FLAG_COMPAT6;
-break;
+error(qemu-img: option -6 is deprecated, please use \'-o 
+  compat6\' instead!);
+return 1;
 case 'o':
 options = optarg;
 break;
@@ -806,7 +797,7 @@ static int img_convert(int argc, char **argv)
 }
 
 set_option_parameter_int(param, BLOCK_OPT_SIZE, total_sectors * 512);
-ret = add_old_style_options(out_fmt, param, flags, out_baseimg, NULL);
+ret = add_old_style_options(out_fmt, param, out_baseimg, NULL);
 if (ret  0) {
 goto out;
 }
@@ -818,7 +809,7 @@ static int 

[Qemu-devel] Re: [PATCH v2 1/1] qemu-img: Deprecate obsolete -6 and -e options

2010-12-07 Thread Kevin Wolf
Am 07.12.2010 17:44, schrieb jes.soren...@redhat.com:
 From: Jes Sorensen jes.soren...@redhat.com
 
 If -6 or -e is specified, an error message is printed and we exit. It
 does not print help() to avoid the error message getting lost in the
 noise.
 
 Signed-off-by: Jes Sorensen jes.soren...@redhat.com

Thanks, applied to the block branch.

Kevin



[Qemu-devel] Re: [PATCH v2 1/2] Do not register kvmclock savevm section if kvmclock is disabled.

2010-12-07 Thread Glauber Costa
On Mon, 2010-12-06 at 19:04 -0200, Marcelo Tosatti wrote:
 On Mon, Dec 06, 2010 at 09:03:46AM -0500, Glauber Costa wrote:
  Usually nobody usually thinks about that scenario (me included and 
  specially),
  but kvmclock can be actually disabled in the host.
  
  It happens in two scenarios:
   1. host too old.
   2. we passed -kvmclock to our -cpu parameter.
  
  In both cases, we should not register kvmclock savevm section. This patch
  achives that by registering this section only if kvmclock is actually
  currently enabled in cpuid.
  
  The only caveat is that we have to register the savevm section a little bit
  later, since we won't know the final kvmclock state before cpuid gets 
  parsed.
 
 What is the problem of registering the section? Restoring the value if
 the host does not support it returns an error?
 
 Can't you ignore the error if kvmclock is not reported in cpuid, in the
 restore handler?

We can change the restore handler, but not the restore handler of
binaries that are already out there. The motivation here is precisely to
address migration to hosts without kvmclock, so it's better to have
a way to disable, than to count on the fact that the other side will be
able to ignore it.




Re: [Qemu-devel] Re: [RFC][PATCH v5 04/21] virtagent: transport definitions and job callbacks

2010-12-07 Thread Michael Roth

On 12/07/2010 07:44 AM, Jes Sorensen wrote:

On 12/03/10 19:03, Michael Roth wrote:

+static void va_server_read_cb(const char *content, size_t content_len)
+{
+xmlrpc_mem_block *resp_xml;
+VAServerData *server_data =va_state-server_data;
+int ret;
+
+TRACE(called);
+resp_xml = xmlrpc_registry_process_call(server_data-env,
+server_data-registry,
+NULL, content, content_len);
+if (resp_xml == NULL) {
+LOG(error processing RPC request);
+goto out_bad;
+}
+
+ret = va_server_job_add(resp_xml);
+if (ret != 0) {
+LOG(error adding server job: %s, strerror(ret));
+}
+
+return;
+out_bad:
+/* TODO: should reset state here */
+return;


Looks like some missing error handling is needed here?


+static void va_rpc_parse_hdr(VAHTState *s)
+{
+int i, line_pos = 0;
+bool first_line = true;
+char line_buf[4096];


In 03/21 you defined VA_HDR_LEN_MAX to 4096, here you hard code the
value  sounds like something begging to go wrong.


+static int va_end_of_header(char *buf, int end_pos)
+{
+return !strncmp(buf+(end_pos-2), \n\r\n, 3);
+}


Maybe I am missing something here, but it looks like you do a strncmp to
a char that is one past the end of the buffer, or? If this is
intentional, please document it.



buf+end_pos points to the last char we read (rather than being an offset 
to the current position). So it stops comparing when it reaches 
buf+end_pos (buf=0 + end_pos=2 implies 3 characters)


For some reason this confused the hell out of me when I looked over it 
again as well. Alternatively I can do:


static int va_end_of_header(char *buf, int end_pos)
{
return !strncmp(buf+(end_pos-2), \n\r\n, 3);
}
...
va_end_of_header(s-hdr, s-hdr_pos - 1)

-

static int va_end_of_header(char *buf, int cur_pos)
{
return !strncmp(buf+(cur_pos-3), \n\r\n, 3);
}
...
va_end_of_header(s-hdr, s-hdr_pos);

It does seem easier to parse...


All this http parsing code leaves the question open why you do it
manually, instead of relying on a library?



Something like libcurl? At some point we didn't attempt to use libraries 
provide by xmlrpc-c (which uses libcurl for http transport) for the 
client and server. The problem there is that libcurl really wants and 
tcp socket read and write from, whereas we need to support tcp/unix 
sockets on the host side and isa/virtio serial ports on the guest side.


Even assuming we could hook in wrappers for these other types of 
sockets/channels, there's also the added complexity since dropping 
virtproxy of multiplexing HTTP/RPCs using a single stream, whereas 
something like libcurl would, understandably, assume it has a dedicated 
stream to read/write from. So we wouldn't really save any work or code, 
unfortunately.



Cheers,
Jes






Re: [Qemu-devel] Re: [RFC][PATCH v5 09/21] virtagent: add va.getdmesg RPC

2010-12-07 Thread Michael Roth

On 12/07/2010 08:37 AM, Jes Sorensen wrote:

On 12/03/10 19:03, Michael Roth wrote:

Add RPC to view guest dmesg output.

Signed-off-by: Michael Rothmdr...@linux.vnet.ibm.com
---
  virtagent-server.c |   46 ++
  1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/virtagent-server.c b/virtagent-server.c
index a430b58..aac8f70 100644
--- a/virtagent-server.c
+++ b/virtagent-server.c
@@ -83,6 +83,50 @@ EXIT_CLOSE_BAD:
  return result;
  }

+/* va_getdmesg(): return dmesg output
+ * rpc return values:
+ *   - dmesg output as a string
+ */
+static xmlrpc_value *va_getdmesg(xmlrpc_env *env,
+  xmlrpc_value *param,
+  void *user_data)
+{
+char *dmesg_buf = NULL, cmd[256];
+int ret;
+xmlrpc_value *result = NULL;
+FILE *pipe;
+
+SLOG(va_getdmesg());
+
+dmesg_buf = qemu_mallocz(VA_DMESG_LEN + 2048);
+sprintf(cmd, dmesg -s %d, VA_DMESG_LEN);


What happens if the guest's dmesg buffer is larger than your hardcoded
value?


It'll end up getting truncated by the fread() later:

ret = fread(dmesg_buf, sizeof(char), VA_DMESG_LEN, pipe);

That's where the dmesg -s VA_DMESG_LEN comes into play, it should size 
things such that we can buffer up till the end of the dmesg output.


This param is kind of quirky though, size doesn't seem to have an affect 
for anything below 4KB, but if we stick with VA_DMESG_LEN = 4KB this 
should cover us, unless it's a distro-specific. But it should blow 
anything up, at least.




Jes







[Qemu-devel] [PATCH 1/1] qemu-img.c: Clean up handling of image size in img_create()

2010-12-07 Thread Jes . Sorensen
From: Jes Sorensen jes.soren...@redhat.com

This cleans up the handling of image size in img_create() by parsing
the value early, and then only setting it once if a value has been
added as the last argument to the command line.

Signed-off-by: Jes Sorensen jes.soren...@redhat.com
---
 qemu-img.c |   19 +++
 1 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index d146d8c..eaec725 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -282,6 +282,7 @@ static int add_old_style_options(const char *fmt, 
QEMUOptionParameter *list,
 static int img_create(int argc, char **argv)
 {
 int c, ret = 0;
+uint64_t img_size = 0;
 const char *fmt = raw;
 const char *base_fmt = NULL;
 const char *filename;
@@ -329,6 +330,11 @@ static int img_create(int argc, char **argv)
 }
 filename = argv[optind++];
 
+/* Get image size, if specified */
+if (optind  argc) {
+img_size = strtosz(argv[optind++], NULL);
+}
+
 if (options  !strcmp(options, ?)) {
 ret = print_block_option_help(filename, fmt);
 goto out;
@@ -356,7 +362,6 @@ static int img_create(int argc, char **argv)
 
 /* Create parameter list with default values */
 param = parse_option_parameters(, create_options, param);
-set_option_parameter_int(param, BLOCK_OPT_SIZE, -1);
 
 /* Parse -o options */
 if (options) {
@@ -368,21 +373,19 @@ static int img_create(int argc, char **argv)
 }
 }
 
-/* Add size to parameters */
-if (optind  argc) {
-set_option_parameter(param, BLOCK_OPT_SIZE, argv[optind++]);
-}
-
 /* Add old-style options to parameters */
 ret = add_old_style_options(fmt, param, base_filename, base_fmt);
 if (ret  0) {
 goto out;
 }
 
+if (img_size) {
+set_option_parameter_int(param, BLOCK_OPT_SIZE, img_size);
+}
+
 // The size for the image must always be specified, with one exception:
 // If we are using a backing file, we can obtain the size from there
-if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == -1) {
-
+if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == 0) {
 QEMUOptionParameter *backing_file =
 get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
 QEMUOptionParameter *backing_fmt =
-- 
1.7.3.2




Re: [Qemu-devel] [PATCH 0/6] [RFC] New SPARC machine: Leon3

2010-12-07 Thread Fabien Chouteau

On 12/06/2010 07:12 PM, Blue Swirl wrote:

On Mon, Dec 6, 2010 at 3:07 PM, Fabien Chouteauchout...@adacore.com  wrote:

On 12/06/2010 11:44 AM, Artyom Tarasenko wrote:


On Mon, Dec 6, 2010 at 10:26 AM, Fabien Chouteauchout...@adacore.com
  wrote:


Hi everyone,
I'm glad to submit my first patches to the Qemu-devel list.

This patch set introduces a new SPARC V8 machine: Leon3. It's an
open-source
VHDL System-On-Chip, well known in space industry (more information on
http://www.gaisler.com).


Nice! Haven't looked into the code yet, but it's great to have someone
who cares for V8.


And if this patch is accepted, we will try to submit more machines like
erc32 and leon2.


Do you also have a firmware which runs on these machines?



I can give you a binary running some basic tests.


Is the binary generally available?


No it's just a program that I use to test my work.


Otherwise it would be very hard to
test any changes and the code would bitrot. I'm not sure we even want
to support such machines.


I think we can find many people interested in Leon3, the processor is well
known in the space industry and used in many research projects. The only
emulator available is Tsim and it's not free nor open-source, so Qemu has a
real interest here.


Are the sources available? That would help debugging.


I can give you the sources but it will be difficult for you to compile them.
It's an Ada program compiled with Gnat Pro for Leon3, using the Ravenscar
run-time. If you are not familiar with the Ada language, you can see the
Ravenscar run-time as a very small kernel providing basic tasking 
support for

safety critical applications.


Leon3 is made of multiple components available in the GrLib VHDL library.
Three devices are implemented: uart, timers and IRQ manager.
You can find code for these peripherals in the grlib_* files.

Modifications have been done to the SPARC cpu emulation code to handle
Leon3's specific behavior:
  - IRQ management
  - Cache control
  - Asr17 (implementation-dependent Ancillary State Registers)


Is it the only implementation-dependent asr in Leon3? Thought there were
more.



Yes, there's also asr19 for power-down, asr16 for FPU control and others for
hardware breakpoints.
These are not required for this first implementation, but If there's a need
for more ASRs, we may have to find a generic implementation to handle those
registers.


So far I'd handle these in target-sparc/op_helper.c. If the registers
are also available as MMIO like MXCC, then we should invent a way to
handle both.


I don't see what you mean by also available as MMIO, it's just regular
registers in the CPU, except that they are available for
implementation-dependent uses.


Thank you for your review ;)

--
Fabien Chouteau




[Qemu-devel] Using the mailing list for asking questions about the source code

2010-12-07 Thread Stefano Bonifazi

Hi All!
 I am new in QEMU developing and I am not sure if I can use this 
mailing list for asking general questions about QEMU source code as I 
could not find any guidelines about it.
I noticed that, usually, questions about the source code in the QEMU 
forum never receive answers.
Surely the best help I can receive is from you, and it is essential due 
to the lack of good technical documentation, but I don't know if the 
mailing list is only for sharing patches and fixes. In this case I ask 
for an apology for my intrusion, and I'd be very glad of being addressed 
to a proper help.

Best Regards,
Stefano B.



Re: [Qemu-devel] [PATCH 1/1] qemu-img.c: Clean up handling of image size in img_create()

2010-12-07 Thread Stefan Hajnoczi
On Tue, Dec 7, 2010 at 5:39 PM,  jes.soren...@redhat.com wrote:
     // The size for the image must always be specified, with one exception:
     // If we are using a backing file, we can obtain the size from there
 -    if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == -1) {
 -
 +    if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == 0) {
         QEMUOptionParameter *backing_file =
             get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
         QEMUOptionParameter *backing_fmt =

Today it is possible to create 0 byte sized images.  Your patch will
change that:
If there is a backing file, then the size will be taken from the backing file.
If there is no backing file, then an error about missing size will be
printed, even though a size of 0 has been given.

I don't think 0 sized images are very useful, but I'm not sure we
should make this change.

Stefan



Re: [Qemu-devel] Using the mailing list for asking questions about the source code

2010-12-07 Thread Stefan Weil

Am 07.12.2010 21:22, schrieb Stefano Bonifazi:

Hi All!
 I am new in QEMU developing and I am not sure if I can use this 
mailing list for asking general questions about QEMU source code as I 
could not find any guidelines about it.
I noticed that, usually, questions about the source code in the QEMU 
forum never receive answers.
Surely the best help I can receive is from you, and it is essential 
due to the lack of good technical documentation, but I don't know if 
the mailing list is only for sharing patches and fixes. In this case I 
ask for an apology for my intrusion, and I'd be very glad of being 
addressed to a proper help.

Best Regards,
Stefano B.




Hi Stefano,

you found this mailing list, so I assume you read everything which is
available on http://wiki.qemu.org/Main_Page, and you also tried hard
to find the answers to your questions yourself, didn't you?

If there remain concrete questions, you should collect them and
send them to qemu-devel or add them to http://wiki.qemu.org/Talk:Manual
with a remark missing documentation. Precise questions have a higher
probability to get an answer than abstract ones.

Kind regards,
Stefan W.




Re: [Qemu-devel] Using the mailing list for asking questions about the source code

2010-12-07 Thread Stefano Bonifazi

On 12/07/2010 10:01 PM, Stefan Weil wrote:



Hi Stefano,

you found this mailing list, so I assume you read everything which is
available on http://wiki.qemu.org/Main_Page, and you also tried hard
to find the answers to your questions yourself, didn't you?

If there remain concrete questions, you should collect them and
send them to qemu-devel or add them to http://wiki.qemu.org/Talk:Manual
with a remark missing documentation. Precise questions have a higher
probability to get an answer than abstract ones.

Kind regards,
Stefan W.


Hi!
Thank you for your quick answer! :)
Yes, I've spent many days collecting and studying all possible documents 
about QEMU in the internet.

Unluckily there is much about using it, but few about its code.
The few documents are often old and not updated.
The project is huge and complex and seems very very hard for a student 
to face it all alone.
I am sure that few words from who has spent on it so much time, efforts, 
experience, passion will speed up my work very much :)

 Thank you again! Best regards!
Stefano B.




Re: [Qemu-devel] [PATCH 1/1] qemu-img.c: Clean up handling of image size in img_create()

2010-12-07 Thread Jes Sorensen
On 12/07/10 21:36, Stefan Hajnoczi wrote:
 On Tue, Dec 7, 2010 at 5:39 PM,  jes.soren...@redhat.com wrote:
 // The size for the image must always be specified, with one exception:
 // If we are using a backing file, we can obtain the size from there
 -if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == -1) {
 -
 +if (get_option_parameter(param, BLOCK_OPT_SIZE)-value.n == 0) {
 QEMUOptionParameter *backing_file =
 get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
 QEMUOptionParameter *backing_fmt =
 
 Today it is possible to create 0 byte sized images.  Your patch will
 change that:
 If there is a backing file, then the size will be taken from the backing file.
 If there is no backing file, then an error about missing size will be
 printed, even though a size of 0 has been given.
 
 I don't think 0 sized images are very useful, but I'm not sure we
 should make this change.

The old code also fails if there is no size, except for when a backing
file is present.

I hadn't thought of the zero sized file, but on the other hand, I don't
see it being useful.

I would like to make this change to get the option handling cleaned up
as it allows me to refactor the code in img_create().

Cheers,
Jes



Re: [Qemu-devel] [PATCH 06/21] vl: add a tmp pointer so that a handler can delete the entry to which it belongs.

2010-12-07 Thread Isaku Yamahata
QLIST_FOREACH_SAFE?

On Thu, Nov 25, 2010 at 03:06:45PM +0900, Yoshiaki Tamura wrote:
 By copying the next entry to a tmp pointer,
 qemu_del_vm_change_state_handler() can be called in the handler.
 
 Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
 ---
  vl.c |5 +++--
  1 files changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/vl.c b/vl.c
 index 805e11f..6b6aec0 100644
 --- a/vl.c
 +++ b/vl.c
 @@ -1073,11 +1073,12 @@ void 
 qemu_del_vm_change_state_handler(VMChangeStateEntry *e)
  
  void vm_state_notify(int running, int reason)
  {
 -VMChangeStateEntry *e;
 +VMChangeStateEntry *e, *ne;
  
  trace_vm_state_notify(running, reason);
  
 -for (e = vm_change_state_head.lh_first; e; e = e-entries.le_next) {
 +for (e = vm_change_state_head.lh_first; e; e = ne) {
 +ne = e-entries.le_next;
  e-cb(e-opaque, running, reason);
  }
  }
 -- 
 1.7.1.2
 
 

-- 
yamahata