Re: [Qemu-devel] [PATCH v6 1/8] atomic: introduce atomic operations

2012-11-12 Thread liu ping fan
On Mon, Nov 12, 2012 at 5:54 PM, Paolo Bonzini  wrote:
> Il 05/11/2012 06:38, Liu Ping Fan ha scritto:
>> From: Liu Ping Fan 
>>
>> If out of global lock, we will be challenged by SMP in low level,
>> so need atomic ops.
>>
>> This file is a wrapper of GCC atomic builtin.
>
> I still object to this.
>
> I know it enforces type-safety, but it is incomplete.  It doesn't

Although it is incomplete, but the rest cases are rarely used.  Linux
faces such issue, and the "int" version is enough, so I think we can
borrow experience from there.

> provide neither atomic accesses to pointers, nor useful operations such
> as exchange.  It won't be used consistently, because in some places you
> just do not have an Atomic value (see both current uses of __sync_*
> builtins).
>
> If you can make it complete, and prove it by using it where __sync_* is

For current code, __sync_* is used rarely, I think except the
barriers, only two places use it. We can substitute it easily.

Regards,
Pingfan
> used now, or just use gcc builtins directly.
>
> Paolo
>
>>
>> Signed-off-by: Liu Ping Fan 
>> ---
>>  include/qemu/atomic.h |   63 
>> +
>>  1 files changed, 63 insertions(+), 0 deletions(-)
>>  create mode 100644 include/qemu/atomic.h
>>
>> diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
>> new file mode 100644
>> index 000..a9e6d35
>> --- /dev/null
>> +++ b/include/qemu/atomic.h
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Simple wrapper of gcc Atomic-Builtins
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +#ifndef __QEMU_ATOMIC_H
>> +#define __QEMU_ATOMIC_H
>> +
>> +typedef struct Atomic {
>> +volatile int counter;
>> +} Atomic;
>> +
>> +static inline void atomic_set(Atomic *v, int i)
>> +{
>> +v->counter = i;
>> +}
>> +
>> +static inline int atomic_read(Atomic *v)
>> +{
>> +return v->counter;
>> +}
>> +
>> +static inline int atomic_return_and_add(int i, Atomic *v)
>> +{
>> +int ret;
>> +
>> +ret = __sync_fetch_and_add(&v->counter, i);
>> +return ret;
>> +}
>> +
>> +static inline int atomic_return_and_sub(int i, Atomic *v)
>> +{
>> +int ret;
>> +
>> +ret = __sync_fetch_and_sub(&v->counter, i);
>> +return ret;
>> +}
>> +
>> +/**
>> + *  * atomic_inc - increment atomic variable
>> + *   * @v: pointer of type Atomic
>> + **
>> + * * Atomically increments @v by 1.
>> + *  */
>> +static inline void atomic_inc(Atomic *v)
>> +{
>> +__sync_fetch_and_add(&v->counter, 1);
>> +}
>> +
>> +/**
>> + *  * atomic_dec - decrement atomic variable
>> + *   * @v: pointer of type Atomic
>> + **
>> + * * Atomically decrements @v by 1.
>> + *  */
>> +static inline void atomic_dec(Atomic *v)
>> +{
>> +__sync_fetch_and_sub(&v->counter, 1);
>> +}
>> +
>> +#endif
>>
>



Re: [Qemu-devel] [PATCH v6 3/8] hotplug: introduce qdev_unplug_complete() to remove device from views

2012-11-12 Thread liu ping fan
On Mon, Nov 12, 2012 at 5:27 PM, Paolo Bonzini  wrote:
> Il 05/11/2012 06:38, Liu Ping Fan ha scritto:
>> +void qdev_unplug_complete(DeviceState *dev, Error **errp)
>> +{
>> +/* isolate from mem view */
>> +qdev_unmap(dev);
>> +/* isolate from device tree */
>> +qdev_unset_parent(dev);
>> +object_unref(OBJECT(dev));
>
> This leaks the device.  I'll send a patch to fix this, please include it
> in v7.
>
OK. Please CC me.

Thanks
Pingfan
> Paolo
>
>> +}
>> +
>



Re: [Qemu-devel] [RFC v1 2/3] ramlist: apply fine grain lock for ram_list

2012-11-12 Thread liu ping fan
On Mon, Nov 12, 2012 at 4:48 PM, Paolo Bonzini  wrote:
> Il 12/11/2012 07:22, liu ping fan ha scritto:
>> On Sat, Nov 10, 2012 at 9:54 AM, Paolo Bonzini  wrote:
>>> Il 09/11/2012 04:14, Liu Ping Fan ha scritto:
 From: Liu Ping Fan 

 Signed-off-by: Liu Ping Fan 
 ---
  cpu-all.h |1 +
  exec.c|   46 +++---
  2 files changed, 40 insertions(+), 7 deletions(-)
>>>
>>> The problem here is that the ram_list is a pretty critical bit for TCG.
>>>
>> This patch does not touch the MRU, so you mean the expense of lock?
>
> Yes.
>
> One alternative is to remove the MRU, but add a 1-item cache to speed up
> the common case.  Then the case where you use the cache can be placed
> (later) in an RCU critical section.
>
>>> The migration thread series has patches that split the list in two: a
>>> MRU-accessed list that uses the BQL, and another that uses a separate lock.
>>
>> I read the thread, but I think we can not protect RAMBlock w/o a
>> unified lock.  When ram device's refcnt->0, and call
>> qemu_ram_free_from_ptr(), it can be with/without QBL.
>
> Note that you would also split between unmap (which does QLIST_REMOVE)
> and free (which actually frees the block).  qemu_ram_free_from_ptr()
> would really become qemu_ram_unmap_from_ptr(), and could do part of the
> work asynchronously---which makes it free to take and release locks as
> needed.  I don't think it is problematic to delay the freeing of the
> blocks_mru item which requires BQL.
>
Right.  Then just one thing left, the big lock may be called
recursively.  Do we have some elegant method to handle  it?

Regards,
Pingfan
> Paolo



Re: [Qemu-devel] [PATCH v3 2/2] qemu-ga: sample fsfreeze hooks

2012-11-12 Thread Tomoki Sekiyama
Adds sample hook scripts for --fsfreeze-hook option of qemu-ga.
  - fsfreeze-hook : execute scripts in fsfreeze-hook.d/
  - fsfreeze-hook.d.sample/mysql-flush.sh : quiesce MySQL before snapshot

Signed-off-by: Tomoki Sekiyama 
---
 docs/qemu-guest-agent/fsfreeze-hook|   31 +
 .../fsfreeze-hook.d.sample/mysql-flush.sh  |   47 
 2 files changed, 78 insertions(+)
 create mode 100755 docs/qemu-guest-agent/fsfreeze-hook
 create mode 100755 docs/qemu-guest-agent/fsfreeze-hook.d.sample/mysql-flush.sh

diff --git a/docs/qemu-guest-agent/fsfreeze-hook 
b/docs/qemu-guest-agent/fsfreeze-hook
new file mode 100755
index 000..319f68c
--- /dev/null
+++ b/docs/qemu-guest-agent/fsfreeze-hook
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+# This script is executed when a guest agent receives fsfreeze-freeze and
+# fsfreeze-thaw command, if it is specified in --fsfreeze-hook (-F)
+# option of qemu-ga or placed in default path (/etc/qemu/fsfreeze-hook).
+# When the agent receives fsfreeze-freeze request, this script is issued with
+# "freeze" argument before the filesystem is freezed. And for fsfreeze-thaw
+# request, it is issued with "thaw" argument after filesystem is thawed.
+
+LOGFILE=/var/log/qga-fsfreeze-hook.log
+FSFREEZE_D=$(dirname -- "$0")/fsfreeze-hook.d
+
+# Check whether file $1 is a backup or rpm-generated file and should be ignored
+is_ignored_file() {
+case "$1" in
+*~ | *.bak | *.orig | *.rpmnew | *.rpmorig | *.rpmsave)
+return 0 ;;
+esac
+return 1
+}
+
+# Iterate executables in directory "fsfreeze-hook.d" with the specified args
+[ ! -d "$FSFREEZE_D" ] && exit 1
+for file in "$FSFREEZE_D"/* ; do
+is_ignored_file "$file" && continue
+[ -x "$file" ] || continue
+echo "$(date): execute $file $@" >>$LOGFILE
+"$file" "$@" >>$LOGFILE 2>&1
+STATUS=$?
+echo "$(date): $file finished with status=$STATUS" >>$LOGFILE
+done
diff --git a/docs/qemu-guest-agent/fsfreeze-hook.d.sample/mysql-flush.sh 
b/docs/qemu-guest-agent/fsfreeze-hook.d.sample/mysql-flush.sh
new file mode 100755
index 000..e6d7998
--- /dev/null
+++ b/docs/qemu-guest-agent/fsfreeze-hook.d.sample/mysql-flush.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+
+# Flush MySQL tables to the disk before the filesystem is freezed.
+# At the same time, this keeps a read lock while the filesystem is freezed
+# in order to avoid write accesses by the other clients.
+
+MYSQL="mysql -uroot" #"-prootpassword"
+FIFO=/tmp/mysql-flush.fifo
+
+flush_and_wait() {
+printf "FLUSH TABLES WITH READ LOCK \\G\n"
+read < $FIFO
+printf "UNLOCK TABLES \\G\n"
+}
+
+case "$1" in
+freeze)
+mkfifo $FIFO || exit 1
+flush_and_wait | $MYSQL &
+# wait until every block is flushed
+while [ "$(echo 'SHOW STATUS LIKE "Key_blocks_not_flushed"' |\
+ $MYSQL | tail -1 | cut -f 2)" -gt 0 ]; do
+sleep 1
+done
+# for InnoDB, wait until every log is flushed
+INNODB_STATUS=$(mktemp /tmp/mysql-flush.XX)
+[ $? -ne 0 ] && exit 2
+trap "rm -f $INNODB_STATUS" SIGINT
+while :; do
+printf "SHOW ENGINE INNODB STATUS \\G" | $MYSQL > $INNODB_STATUS
+LOG_CURRENT=$(grep 'Log sequence number' $INNODB_STATUS |\
+  tr -s ' ' | cut -d' ' -f4)
+LOG_FLUSHED=$(grep 'Log flushed up to' $INNODB_STATUS |\
+  tr -s ' ' | cut -d' ' -f5)
+[ "$LOG_CURRENT" = "$LOG_FLUSHED" ] && break
+sleep 1
+done
+rm -f $INNODB_STATUS
+;;
+
+thaw)
+[ ! -p $FIFO ] && exit 1
+echo > $FIFO
+rm -f $FIFO
+;;
+esac
+




Re: [Qemu-devel] [PATCH v3 1/2] qemu-ga: execute hook to quiesce the guest on fsfreeze-freeze/thaw

2012-11-12 Thread Tomoki Sekiyama
To use the online disk snapshot for online-backup, application-level
consistency of the snapshot image is required. However, currently the
guest agent can provide only filesystem-level consistency, and the
snapshot may contain dirty data, for example, incomplete transactions.
This patch provides the opportunity to quiesce applications before
snapshot is taken.

When the qemu-ga receives fsfreeze-freeze command, the hook script
specified in --fsfreeze-hook option is executed with "freeze" argument
before the filesystem is frozen. For fsfreeze-thaw command, the hook
script is executed with "thaw" argument after the filesystem is thawed.

Signed-off-by: Tomoki Sekiyama 
---
 qemu-ga.c  |   42 +-
 qga/commands-posix.c   |   44 
 qga/guest-agent-core.h |1 +
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/qemu-ga.c b/qemu-ga.c
index 9b59a52..d1d9b89 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -34,6 +34,12 @@
 #include "qga/service-win32.h"
 #include 
 #endif
+#ifdef __linux__
+#include 
+#ifdef FIFREEZE
+#define CONFIG_FSFREEZE
+#endif
+#endif
 
 #ifndef _WIN32
 #define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent.0"
@@ -42,6 +48,9 @@
 #endif
 #define QGA_STATEDIR_DEFAULT CONFIG_QEMU_LOCALSTATEDIR "/run"
 #define QGA_PIDFILE_DEFAULT QGA_STATEDIR_DEFAULT "/qemu-ga.pid"
+#ifdef CONFIG_FSFREEZE
+#define QGA_FSFREEZE_HOOK_DEFAULT CONFIG_QEMU_CONFDIR "/fsfreeze-hook"
+#endif
 #define QGA_SENTINEL_BYTE 0xFF
 
 struct GAState {
@@ -64,6 +73,9 @@ struct GAState {
 const char *log_filepath;
 const char *pid_filepath;
 } deferred_options;
+#ifdef CONFIG_FSFREEZE
+const char *fsfreeze_hook;
+#endif
 };
 
 struct GAState *ga_state;
@@ -153,6 +165,10 @@ static void usage(const char *cmd)
 "%s)\n"
 "  -l, --logfile set logfile path, logs to stderr by default\n"
 "  -f, --pidfile specify pidfile (default is %s)\n"
+#ifdef CONFIG_FSFREEZE
+"  -F, --fsfreeze_hook\n"
+"specify fsfreeze hook (default is %s)\n"
+#endif
 "  -t, --statedirspecify dir to store state information (absolute paths\n"
 "only, default is %s)\n"
 "  -v, --verbose log extra debugging information\n"
@@ -167,6 +183,9 @@ static void usage(const char *cmd)
 "\n"
 "Report bugs to \n"
 , cmd, QEMU_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT,
+#ifdef CONFIG_FSFREEZE
+QGA_FSFREEZE_HOOK_DEFAULT,
+#endif
 QGA_STATEDIR_DEFAULT);
 }
 
@@ -401,6 +420,13 @@ void ga_unset_frozen(GAState *s)
 }
 }
 
+#ifdef CONFIG_FSFREEZE
+const char *ga_fsfreeze_hook(GAState *s)
+{
+return s->fsfreeze_hook;
+}
+#endif
+
 static void become_daemon(const char *pidfile)
 {
 #ifndef _WIN32
@@ -678,10 +704,13 @@ VOID WINAPI service_main(DWORD argc, TCHAR *argv[])
 
 int main(int argc, char **argv)
 {
-const char *sopt = "hVvdm:p:l:f:b:s:t:";
+const char *sopt = "hVvdm:p:l:f:F:b:s:t:";
 const char *method = NULL, *path = NULL;
 const char *log_filepath = NULL;
 const char *pid_filepath = QGA_PIDFILE_DEFAULT;
+#ifdef CONFIG_FSFREEZE
+const char *fsfreeze_hook = QGA_FSFREEZE_HOOK_DEFAULT;
+#endif
 const char *state_dir = QGA_STATEDIR_DEFAULT;
 #ifdef _WIN32
 const char *service = NULL;
@@ -691,6 +720,9 @@ int main(int argc, char **argv)
 { "version", 0, NULL, 'V' },
 { "logfile", 1, NULL, 'l' },
 { "pidfile", 1, NULL, 'f' },
+#ifdef CONFIG_FSFREEZE
+{ "fsfreeze-hook", 1, NULL, 'F' },
+#endif
 { "verbose", 0, NULL, 'v' },
 { "method", 1, NULL, 'm' },
 { "path", 1, NULL, 'p' },
@@ -723,6 +755,11 @@ int main(int argc, char **argv)
 case 'f':
 pid_filepath = optarg;
 break;
+#ifdef CONFIG_FSFREEZE
+case 'F':
+fsfreeze_hook = optarg;
+break;
+#endif
 case 't':
  state_dir = optarg;
  break;
@@ -786,6 +823,9 @@ int main(int argc, char **argv)
 s = g_malloc0(sizeof(GAState));
 s->log_level = log_level;
 s->log_file = stderr;
+#ifdef CONFIG_FSFREEZE
+s->fsfreeze_hook = fsfreeze_hook;
+#endif
 g_log_set_default_handler(ga_log, s);
 g_log_set_fatal_mask(NULL, G_LOG_LEVEL_ERROR);
 ga_enable_logging(s);
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 726930a..8c3e341 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -396,6 +396,45 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
 return GUEST_FSFREEZE_STATUS_THAWED;
 }
 
+static void execute_fsfreeze_hook(const char *arg)
+{
+int status;
+pid_t pid, rpid;
+const char *hook;
+
+hook = ga_fsfreeze_hook(ga_state);
+if (!hook || access(hook, X_OK) != 0) {
+return;
+}
+
+slog("executing fsfreeze hook with arg `%s'", arg);
+pid = fork();
+if (pid == 0) {
+setsid();
+reopen_fd_to_null(0);
+  

[Qemu-devel] [PATCH v3 0/2] qemu-ga: add hook to quiesce the guest on fsfreeze-freeze/thaw

2012-11-12 Thread Tomoki Sekiyama
Hi,

This is version 3 of the qemu-ga fsfreeze hook patchset.

*Changes from v2: ( 
https://lists.gnu.org/archive/html/qemu-devel/2012-11/msg00918.html )
  1/2: Not changed.
  2/2: fsfreeze-hook:  Fixed typo.
   mysql-flush.sh: Use printf instead of echo to make it portable

---

Tomoki Sekiyama (2):
  qemu-ga: execute hook to quiesce the guest on fsfreeze-freeze/thaw
  qemu-ga: sample fsfreeze hooks


 docs/qemu-guest-agent/fsfreeze-hook|   31 +
 .../fsfreeze-hook.d.sample/mysql-flush.sh  |   47 
 qemu-ga.c  |   42 +-
 qga/commands-posix.c   |   44 +++
 qga/guest-agent-core.h |1 
 5 files changed, 164 insertions(+), 1 deletion(-)
 create mode 100755 docs/qemu-guest-agent/fsfreeze-hook
 create mode 100755 docs/qemu-guest-agent/fsfreeze-hook.d.sample/mysql-flush.sh


Thanks,
-- 
Tomoki Sekiyama 
Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory



Re: [Qemu-devel] [PATCH v2 2/2] qemu-ga: sample fsfreeze hooks

2012-11-12 Thread Tomoki Sekiyama
Hi Eric,
thank you for the review again.

On 2012/11/13 7:16, Eric Blake wrote:
> On 11/12/2012 02:32 AM, Tomoki Sekiyama wrote:
>> Adds sample hook scripts for --fsfreeze-hook option of qemu-ga.
>>   - fsfreeze-hook : execute scripts in fsfreeze-hook.d/
>>   - fsfreeze-hook.d.sample/mysql-flush.sh : quiesce MySQL before snapshot
>>
>> Signed-off-by: Tomoki Sekiyama 
>> ---
> 
>> +LOGFILE=/var/log/qga-fsreeze-hook.log
> 
> s/fsreeze/fsfreeze/

Oops...

>> +# Iterate executables in directory "fsfreeze-hook.d" with the specified args
>> +[ ! -d "$FSFREEZE_D" ] && exit 1
>> +for file in "$FSFREEZE_D"/* ; do
>> +is_ignored_file "$file" && continue
>> +[ -x "$file" ] || continue
>> +echo $(date) ": execute $file $@" >>$LOGFILE
> 
> Put $(date) inside the ""; otherwise, single-digit days will be one
> character shorter due to IFS field splitting eating the double-space;
> and I don't like unaligned dates in logs.
> 
>> +"$file" "$@" >>$LOGFILE 2>&1
>> +STATUS=$?
>> +echo $(date) ": $file finished with status=$STATUS" >>$LOGFILE
> 
> and again.

I will fix this.

>> +INNODB_STATUS=$(mktemp /tmp/mysql-flush.XX)
>> +[ $? -ne 0 ] && exit 2
>> +trap "rm -f $INNODB_STATUS" SIGINT
>> +while :; do
>> +echo 'SHOW ENGINE INNODB STATUS \G' | $MYSQL > $INNODB_STATUS
> 
> 'echo' cannot portably be used with \.  Use 'printf' instead.

OK, I replace this with 'printf' (also in 'flush_and_wait' function).

-- 
Tomoki Sekiyama 
Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory




[Qemu-devel] [PATCH 12/12] pseries: Generate unique LIOBNs for PCI host bridges

2012-11-12 Thread David Gibson
From: Alexey Kardashevskiy 

In future (with VFIO) we will have multiple PCI host bridges on
pseries.  Each one needs a unique LIOBN (IOMMU id).  At the moment we
derive these from the pci domain number, but the whole notion of
domain numbers on the qemu side is bogus and in any case they're not
actually uniquely allocated at this point.

This patch, therefore uses a simple sequence counter to generate
unique LIOBNs for PCI host bridges.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: David Gibson 
---
 hw/spapr_pci.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index 3c5b855..f6544d7 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -521,6 +521,7 @@ static int spapr_phb_init(SysBusDevice *s)
 char *namebuf;
 int i;
 PCIBus *bus;
+static int phbnum;
 
 sphb->dtbusname = g_strdup_printf("pci@%" PRIx64, sphb->buid);
 namebuf = alloca(strlen(sphb->dtbusname) + 32);
@@ -572,7 +573,7 @@ static int spapr_phb_init(SysBusDevice *s)
PCI_DEVFN(0, 0), PCI_NUM_PINS);
 phb->bus = bus;
 
-sphb->dma_liobn = SPAPR_PCI_BASE_LIOBN | (pci_find_domain(bus) << 16);
+sphb->dma_liobn = SPAPR_PCI_BASE_LIOBN | (++phbnum << 16);
 sphb->dma_window_start = 0;
 sphb->dma_window_size = 0x4000;
 sphb->dma = spapr_tce_new_dma_context(sphb->dma_liobn, 
sphb->dma_window_size);
-- 
1.7.10.4




[Qemu-devel] [PATCH 09/12] pseries: Implement PAPR NVRAM

2012-11-12 Thread David Gibson
The PAPR specification requires a certain amount of NVRAM, accessed via
RTAS, which we don't currently implement in qemu.  This patch addresses
this deficiency, implementing the NVRAM as a VIO device, with some glue to
instantiate it automatically based on a machine option.

The machine option specifies a drive id, which is used to back the NVRAM,
making it persistent.  If nothing is specified, the driver instead simply
allocates space for the NVRAM, which will not be persistent

Signed-off-by: David Gibson 
---
 hw/ppc/Makefile.objs |2 +-
 hw/spapr.c   |   33 +
 hw/spapr.h   |2 +
 hw/spapr_nvram.c |  196 ++
 qemu-config.c|4 ++
 5 files changed, 236 insertions(+), 1 deletion(-)
 create mode 100644 hw/spapr_nvram.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 8fe2123..4492127 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -11,7 +11,7 @@ obj-y += ppc_newworld.o
 obj-$(CONFIG_PSERIES) += spapr.o spapr_hcall.o spapr_rtas.o spapr_vio.o
 obj-$(CONFIG_PSERIES) += xics.o spapr_vty.o spapr_llan.o spapr_vscsi.o
 obj-$(CONFIG_PSERIES) += spapr_pci.o pci-hotplug.o spapr_iommu.o
-obj-$(CONFIG_PSERIES) += spapr_events.o
+obj-$(CONFIG_PSERIES) += spapr_events.o spapr_nvram.o
 # PowerPC 4xx boards
 obj-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o
 obj-y += ppc440_bamboo.o
diff --git a/hw/spapr.c b/hw/spapr.c
index dc2349c..5d1401b 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -657,6 +657,36 @@ static void spapr_cpu_reset(void *opaque)
 (spapr->htab_shift - 18);
 }
 
+static void spapr_create_nvram(sPAPREnvironment *spapr)
+{
+QemuOpts *machine_opts;
+DeviceState *dev;
+
+dev = qdev_create(&spapr->vio_bus->bus, "spapr-nvram");
+
+machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
+if (machine_opts) {
+const char *drivename;
+
+drivename = qemu_opt_get(machine_opts, "nvram");
+if (drivename) {
+BlockDriverState *bs;
+
+bs = bdrv_find(drivename);
+if (!bs) {
+fprintf(stderr, "No such block device \"%s\" for nvram\n",
+drivename);
+exit(1);
+}
+qdev_prop_set_drive_nofail(dev, "drive", bs);
+}
+}
+
+qdev_init_nofail(dev);
+
+spapr->nvram = (struct sPAPRNVRAM *)dev;
+}
+
 /* Returns whether we want to use VGA or not */
 static int spapr_vga_init(PCIBus *pci_bus)
 {
@@ -820,6 +850,9 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 }
 }
 
+/* We always have at least the nvram device on VIO */
+spapr_create_nvram(spapr);
+
 /* Set up PCI */
 spapr_pci_rtas_init();
 
diff --git a/hw/spapr.h b/hw/spapr.h
index 971a50a..600722f 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -6,11 +6,13 @@
 
 struct VIOsPAPRBus;
 struct sPAPRPHBState;
+struct sPAPRNVRAM;
 struct icp_state;
 
 typedef struct sPAPREnvironment {
 struct VIOsPAPRBus *vio_bus;
 QLIST_HEAD(, sPAPRPHBState) phbs;
+struct sPAPRNVRAM *nvram;
 struct icp_state *icp;
 
 hwaddr ram_limit;
diff --git a/hw/spapr_nvram.c b/hw/spapr_nvram.c
new file mode 100644
index 000..641de48
--- /dev/null
+++ b/hw/spapr_nvram.c
@@ -0,0 +1,196 @@
+/*
+ * QEMU sPAPR NVRAM emulation
+ *
+ * Copyright (C) 2012 David Gibson, IBM Corporation.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or 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 
+#include 
+
+#include "device_tree.h"
+#include "hw/sysbus.h"
+#include "hw/spapr.h"
+#include "hw/spapr_vio.h"
+
+typedef struct sPAPRNVRAM {
+VIOsPAPRDevice sdev;
+uint32_t size;
+uint8_t *buf;
+BlockDriverState *drive;
+} sPAPRNVRAM;
+
+#define MIN_NVRAM_SIZE 8192
+#define DEFAULT_NVRAM_SIZE 16384
+#define MAX_NVRAM_SIZE (UINT16_MAX * 16)
+
+static void rtas_nvram_fetch(sPAPREnvironment *spapr,
+ uint32_t token, uint32_t nargs,
+

[Qemu-devel] [PATCH 02/12] pseries: Use #define for XICS base irq number

2012-11-12 Thread David Gibson
From: Ben Herrenschmidt 

Currently the lowest "real" irq number for the XICS irq controller (as
opposed to numbers reserved for IPIs and other special purposes) is
hard coded as 16 in two places - in xics_system_init() and in spapr.c.

As well as being generally bad practice, we're going to need to change this
number soon to fit in with the in-kernel XICS implementation.  This patch
adds a #define for this number to avoid future breakage.

Signed-off-by: Michael Ellerman 
Signed-off-by: Ben Herrenschmidt 
Signed-off-by: David Gibson 
---
 hw/spapr.c |2 +-
 hw/xics.c  |2 +-
 hw/xics.h  |1 +
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index ad3f0ea..eafee03 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -801,7 +801,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 
 /* Set up Interrupt Controller */
 spapr->icp = xics_system_init(XICS_IRQS);
-spapr->next_irq = 16;
+spapr->next_irq = XICS_IRQ_BASE;
 
 /* Set up EPOW events infrastructure */
 spapr_events_init(spapr);
diff --git a/hw/xics.c b/hw/xics.c
index edf5833..b8887cd 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -549,7 +549,7 @@ struct icp_state *xics_system_init(int nr_irqs)
 
 ics = g_malloc0(sizeof(*ics));
 ics->nr_irqs = nr_irqs;
-ics->offset = 16;
+ics->offset = XICS_IRQ_BASE;
 ics->irqs = g_malloc0(nr_irqs * sizeof(struct ics_irq_state));
 
 icp->ics = ics;
diff --git a/hw/xics.h b/hw/xics.h
index 6817268..c3bf008 100644
--- a/hw/xics.h
+++ b/hw/xics.h
@@ -28,6 +28,7 @@
 #define __XICS_H__
 
 #define XICS_IPI0x2
+#define XICS_IRQ_BASE   0x10
 
 struct icp_state;
 
-- 
1.7.10.4




[Qemu-devel] [PATCH 04/12] pseries: Return the token when we register an RTAS call

2012-11-12 Thread David Gibson
From: Michael Ellerman 

The kernel will soon be able to service some RTAS calls. However the
choice of tokens will still be up to userspace. To support this have
spapr_rtas_register() return the token that is allocated for an
RTAS call, that allows the calling code to tell the kernel what the
token value is.

Signed-off-by: Michael Ellerman 
Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: David Gibson 
---
 hw/spapr.h  |2 +-
 hw/spapr_rtas.c |4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/spapr.h b/hw/spapr.h
index efe7f57..971a50a 100644
--- a/hw/spapr.h
+++ b/hw/spapr.h
@@ -320,7 +320,7 @@ static inline void rtas_st(target_ulong phys, int n, 
uint32_t val)
 typedef void (*spapr_rtas_fn)(sPAPREnvironment *spapr, uint32_t token,
   uint32_t nargs, target_ulong args,
   uint32_t nret, target_ulong rets);
-void spapr_rtas_register(const char *name, spapr_rtas_fn fn);
+int spapr_rtas_register(const char *name, spapr_rtas_fn fn);
 target_ulong spapr_rtas_call(sPAPREnvironment *spapr,
  uint32_t token, uint32_t nargs, target_ulong args,
  uint32_t nret, target_ulong rets);
diff --git a/hw/spapr_rtas.c b/hw/spapr_rtas.c
index 6d5c48a..45294e8 100644
--- a/hw/spapr_rtas.c
+++ b/hw/spapr_rtas.c
@@ -242,7 +242,7 @@ target_ulong spapr_rtas_call(sPAPREnvironment *spapr,
 return H_PARAMETER;
 }
 
-void spapr_rtas_register(const char *name, spapr_rtas_fn fn)
+int spapr_rtas_register(const char *name, spapr_rtas_fn fn)
 {
 int i;
 
@@ -258,7 +258,7 @@ void spapr_rtas_register(const char *name, spapr_rtas_fn fn)
 rtas_next->name = name;
 rtas_next->fn = fn;
 
-rtas_next++;
+return (rtas_next++ - rtas_table) + TOKEN_BASE;
 }
 
 int spapr_rtas_device_tree_setup(void *fdt, hwaddr rtas_addr,
-- 
1.7.10.4




[Qemu-devel] [PATCH 03/12] pseries: Move XICS initialization before cpu initialization

2012-11-12 Thread David Gibson
From: Ben Herrenschmidt 

Currently, the pseries machine initializes the cpus, then the XICS
interrupt controller.  However, to support the upcoming in-kernel XICS
implementation we will need to initialize the irq controller before the
vcpus.  This patch makes the necesssary rearrangement.  This means the
xics init code can no longer auto-detect the number of cpus ("interrupt
servers" in XICS terminology) and so we must pass that in explicitly from
the platform code.

Signed-off-by: Michael Ellerman 
Signed-off-by: Ben Herrenschmidt 
Signed-off-by: David Gibson 
---
 hw/spapr.c |   12 +++-
 hw/xics.c  |   52 +++-
 hw/xics.h  |3 ++-
 3 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index eafee03..dc2349c 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -747,6 +747,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 spapr->htab_shift++;
 }
 
+/* Set up Interrupt Controller before we create the VCPUs */
+spapr->icp = xics_system_init(smp_cpus * kvmppc_smt_threads() / 
smp_threads,
+  XICS_IRQS);
+spapr->next_irq = XICS_IRQ_BASE;
+
 /* init CPUs */
 if (cpu_model == NULL) {
 cpu_model = kvm_enabled() ? "host" : "POWER7";
@@ -759,6 +764,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 }
 env = &cpu->env;
 
+xics_cpu_setup(spapr->icp, env);
+
 /* Set time-base frequency to 512 MHz */
 cpu_ppc_tb_init(env, TIMEBASE_FREQ);
 
@@ -798,11 +805,6 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
 }
 g_free(filename);
 
-
-/* Set up Interrupt Controller */
-spapr->icp = xics_system_init(XICS_IRQS);
-spapr->next_irq = XICS_IRQ_BASE;
-
 /* Set up EPOW events infrastructure */
 spapr_events_init(spapr);
 
diff --git a/hw/xics.c b/hw/xics.c
index b8887cd..f72dfae 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -510,42 +510,36 @@ static void xics_reset(void *opaque)
 }
 }
 
-struct icp_state *xics_system_init(int nr_irqs)
+void xics_cpu_setup(struct icp_state *icp, CPUPPCState *env)
 {
-CPUPPCState *env;
-int max_server_num;
-struct icp_state *icp;
-struct ics_state *ics;
+struct icp_server_state *ss = &icp->ss[env->cpu_index];
 
-max_server_num = -1;
-for (env = first_cpu; env != NULL; env = env->next_cpu) {
-if (env->cpu_index > max_server_num) {
-max_server_num = env->cpu_index;
-}
-}
+assert(env->cpu_index < icp->nr_servers);
 
-icp = g_malloc0(sizeof(*icp));
-icp->nr_servers = max_server_num + 1;
-icp->ss = g_malloc0(icp->nr_servers*sizeof(struct icp_server_state));
+switch (PPC_INPUT(env)) {
+case PPC_FLAGS_INPUT_POWER7:
+ss->output = env->irq_inputs[POWER7_INPUT_INT];
+break;
 
-for (env = first_cpu; env != NULL; env = env->next_cpu) {
-struct icp_server_state *ss = &icp->ss[env->cpu_index];
+case PPC_FLAGS_INPUT_970:
+ss->output = env->irq_inputs[PPC970_INPUT_INT];
+break;
 
-switch (PPC_INPUT(env)) {
-case PPC_FLAGS_INPUT_POWER7:
-ss->output = env->irq_inputs[POWER7_INPUT_INT];
-break;
+default:
+fprintf(stderr, "XICS interrupt controller does not support this CPU "
+"bus model\n");
+abort();
+}
+}
 
-case PPC_FLAGS_INPUT_970:
-ss->output = env->irq_inputs[PPC970_INPUT_INT];
-break;
+struct icp_state *xics_system_init(int nr_servers, int nr_irqs)
+{
+struct icp_state *icp;
+struct ics_state *ics;
 
-default:
-hw_error("XICS interrupt model does not support this CPU bus "
- "model\n");
-exit(1);
-}
-}
+icp = g_malloc0(sizeof(*icp));
+icp->nr_servers = nr_servers;
+icp->ss = g_malloc0(icp->nr_servers*sizeof(struct icp_server_state));
 
 ics = g_malloc0(sizeof(*ics));
 ics->nr_irqs = nr_irqs;
diff --git a/hw/xics.h b/hw/xics.h
index c3bf008..b43678a 100644
--- a/hw/xics.h
+++ b/hw/xics.h
@@ -35,6 +35,7 @@ struct icp_state;
 qemu_irq xics_get_qirq(struct icp_state *icp, int irq);
 void xics_set_irq_type(struct icp_state *icp, int irq, bool lsi);
 
-struct icp_state *xics_system_init(int nr_irqs);
+struct icp_state *xics_system_init(int nr_servers, int nr_irqs);
+void xics_cpu_setup(struct icp_state *icp, CPUPPCState *env);
 
 #endif /* __XICS_H__ */
-- 
1.7.10.4




[Qemu-devel] [PATCH 01/12] pseries: Fix incorrect initialization of interrupt controller

2012-11-12 Thread David Gibson
Currently in the reset code for the XICS interrupt controller, we
initialize the pending_priority field to 0 (most favored, by XICS
convention).  This is incorrect, since there is no pending interrupt, it
should be set to least favored - 0xff.  At the moment our XICS
implementation doesn't get hurt by this edge case, but it does confuse the
upcoming kernel XICS implementation.

Signed-off-by: David Gibson 
---
 hw/xics.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/xics.c b/hw/xics.c
index 1da3106..edf5833 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -495,7 +495,7 @@ static void xics_reset(void *opaque)
 
 for (i = 0; i < icp->nr_servers; i++) {
 icp->ss[i].xirr = 0;
-icp->ss[i].pending_priority = 0;
+icp->ss[i].pending_priority = 0xff;
 icp->ss[i].mfrr = 0xff;
 /* Make all outputs are deasserted */
 qemu_set_irq(icp->ss[i].output, 0);
-- 
1.7.10.4




[Qemu-devel] [PATCH 08/12] target-ppc: Convert ppcemb_tlb_t to use fixed 64-bit RPN

2012-11-12 Thread David Gibson
Currently the ppcemb_tlb_t struct, used on a number of embedded ppc models
to represent a TLB entry contains a hwaddr.  That works reasonably for now,
but is troublesome for saving the state, which we'll want to do in future.
hwaddr is a large enough type to contain a physical address for any
supported machine - and can thus, in theory at least, vary depending on
what machines are enabled other than the one we're actually using right
now (though in fact it doesn't for ppc).  This makes it unsuitable for
describing in vmstate.

This patch therefore changes ppcemb_tlb_t to use a fixed 64-bit integer
which we know is sufficient for all the machines which use this structure.

Signed-off-by: David Gibson 
---
 target-ppc/cpu.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 5f1dc8b..742d4f8 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -355,7 +355,7 @@ struct ppc6xx_tlb_t {
 
 typedef struct ppcemb_tlb_t ppcemb_tlb_t;
 struct ppcemb_tlb_t {
-hwaddr RPN;
+uint64_t RPN;
 target_ulong EPN;
 target_ulong PID;
 target_ulong size;
-- 
1.7.10.4




[Qemu-devel] [0/12] Pending pseries patches

2012-11-12 Thread David Gibson
Here again is my current set of outstanding pseries patches, updated
for current upstream.  I don't think any of these has changed in
substance since their last posting.  As explained last time around,
some of the From/Signed-off-by combinations are a bit odd, but I think
as accurate reflection of the patch's history as is possible.

Please apply.




[Qemu-devel] [PATCH 11/12] pseries: Fix bug in PCI MSI allocation

2012-11-12 Thread David Gibson
From: Alexey Kardashevskiy 

In one of the recent reworks to the XICS code, a bug was introduced where
we use the wrong sense and allocate level interrupts instead of message
interrupts for PCI MSIs.  This patch fixes it.

Signed-off-by: Alexey Kardashevskiy 
Signed-off-by: David Gibson 
---
 hw/spapr_pci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index a08ed11..3c5b855 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -351,7 +351,7 @@ static void rtas_ibm_change_msi(sPAPREnvironment *spapr,
 
 /* There is no cached config, allocate MSIs */
 if (!phb->msi_table[ndev].nvec) {
-irq = spapr_allocate_irq_block(req_num, true);
+irq = spapr_allocate_irq_block(req_num, false);
 if (irq < 0) {
 fprintf(stderr, "Cannot allocate MSIs for device#%d", ndev);
 rtas_st(rets, 0, -1); /* Hardware error */
-- 
1.7.10.4




[Qemu-devel] [PATCH 07/12] pseries: Split xics irq configuration from state information

2012-11-12 Thread David Gibson
Currently the XICS irq controller code has a per-irq state structure which
amongst other things includes whether the interrupt is level or message
triggered - this is configured by the platform code, and is not directly
visible to the guest.  This leads to a slightly awkward construct at reset
time where we need to reset everything in the state structure _except_ the
lsi/msi flag, which needs to retain the information given at platform init
time.

More importantly this flag will make matching the qemu state to the KVM
state for the upcoming in-kernel XICS implementation more awkward.  This
patch, therefore, removes this flag from the per-irq state structure,
instead adding a parallel array giving the lsi/msi configuration per irq.

Signed-off-by: David Gibson 
---
 hw/xics.c |   20 
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/xics.c b/hw/xics.c
index 87aea0f..d6f2179 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -179,13 +179,13 @@ struct ics_irq_state {
 #define XICS_STATUS_REJECTED   0x4
 #define XICS_STATUS_MASKED_PENDING 0x8
 uint8_t status;
-bool lsi;
 };
 
 struct ics_state {
 int nr_irqs;
 int offset;
 qemu_irq *qirqs;
+bool *islsi;
 struct ics_irq_state *irqs;
 struct icp_state *icp;
 };
@@ -254,9 +254,8 @@ static void set_irq_lsi(struct ics_state *ics, int srcno, 
int val)
 static void ics_set_irq(void *opaque, int srcno, int val)
 {
 struct ics_state *ics = (struct ics_state *)opaque;
-struct ics_irq_state *irq = ics->irqs + srcno;
 
-if (irq->lsi) {
+if (ics->islsi[srcno]) {
 set_irq_lsi(ics, srcno, val);
 } else {
 set_irq_msi(ics, srcno, val);
@@ -293,7 +292,7 @@ static void ics_write_xive(struct ics_state *ics, int nr, 
int server,
 
 trace_xics_ics_write_xive(nr, srcno, server, priority);
 
-if (irq->lsi) {
+if (ics->islsi[srcno]) {
 write_xive_lsi(ics, srcno);
 } else {
 write_xive_msi(ics, srcno);
@@ -314,10 +313,8 @@ static void ics_resend(struct ics_state *ics)
 int i;
 
 for (i = 0; i < ics->nr_irqs; i++) {
-struct ics_irq_state *irq = ics->irqs + i;
-
 /* FIXME: filter by server#? */
-if (irq->lsi) {
+if (ics->islsi[i]) {
 resend_lsi(ics, i);
 } else {
 resend_msi(ics, i);
@@ -332,7 +329,7 @@ static void ics_eoi(struct ics_state *ics, int nr)
 
 trace_xics_ics_eoi(nr);
 
-if (irq->lsi) {
+if (ics->islsi[srcno]) {
 irq->status &= ~XICS_STATUS_SENT;
 }
 }
@@ -354,7 +351,7 @@ void xics_set_irq_type(struct icp_state *icp, int irq, bool 
lsi)
 {
 assert(ics_valid_irq(icp->ics, irq));
 
-icp->ics->irqs[irq - icp->ics->offset].lsi = lsi;
+icp->ics->islsi[irq - icp->ics->offset] = lsi;
 }
 
 static target_ulong h_cppr(PowerPCCPU *cpu, sPAPREnvironment *spapr,
@@ -518,10 +515,8 @@ static void xics_reset(void *opaque)
 qemu_set_irq(icp->ss[i].output, 0);
 }
 
+memset(ics->irqs, 0, sizeof(struct ics_irq_state) * ics->nr_irqs);
 for (i = 0; i < ics->nr_irqs; i++) {
-/* Reset everything *except* the type */
-ics->irqs[i].server = 0;
-ics->irqs[i].status = 0;
 ics->irqs[i].priority = 0xff;
 ics->irqs[i].saved_priority = 0xff;
 }
@@ -562,6 +557,7 @@ struct icp_state *xics_system_init(int nr_servers, int 
nr_irqs)
 ics->nr_irqs = nr_irqs;
 ics->offset = XICS_IRQ_BASE;
 ics->irqs = g_malloc0(nr_irqs * sizeof(struct ics_irq_state));
+ics->islsi = g_malloc0(nr_irqs * sizeof(bool));
 
 icp->ics = ics;
 ics->icp = icp;
-- 
1.7.10.4




[Qemu-devel] [PATCH 06/12] pseries: Add tracepoints to the XICS interrupt controller

2012-11-12 Thread David Gibson
This patch adds tracing / debugging calls to the XICS interrupt controller
implementation used on the pseries machine.

Signed-off-by: Ben Herrenschmidt 
Signed-off-by: David Gibson 
---
 hw/xics.c|   23 ---
 trace-events |   13 +
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/hw/xics.c b/hw/xics.c
index f72dfae..87aea0f 100644
--- a/hw/xics.c
+++ b/hw/xics.c
@@ -26,6 +26,7 @@
  */
 
 #include "hw.h"
+#include "trace.h"
 #include "hw/spapr.h"
 #include "hw/xics.h"
 
@@ -66,6 +67,8 @@ static void icp_check_ipi(struct icp_state *icp, int server)
 return;
 }
 
+trace_xics_icp_check_ipi(server, ss->mfrr);
+
 if (XISR(ss)) {
 ics_reject(icp->ics, XISR(ss));
 }
@@ -120,11 +123,13 @@ static void icp_set_mfrr(struct icp_state *icp, int 
server, uint8_t mfrr)
 
 static uint32_t icp_accept(struct icp_server_state *ss)
 {
-uint32_t xirr;
+uint32_t xirr = ss->xirr;
 
 qemu_irq_lower(ss->output);
-xirr = ss->xirr;
 ss->xirr = ss->pending_priority << 24;
+
+trace_xics_icp_accept(xirr, ss->xirr);
+
 return xirr;
 }
 
@@ -134,6 +139,7 @@ static void icp_eoi(struct icp_state *icp, int server, 
uint32_t xirr)
 
 /* Send EOI -> ICS */
 ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
+trace_xics_icp_eoi(server, xirr, ss->xirr);
 ics_eoi(icp->ics, xirr & XISR_MASK);
 if (!XISR(ss)) {
 icp_resend(icp, server);
@@ -144,6 +150,8 @@ static void icp_irq(struct icp_state *icp, int server, int 
nr, uint8_t priority)
 {
 struct icp_server_state *ss = icp->ss + server;
 
+trace_xics_icp_irq(server, nr, priority);
+
 if ((priority >= CPPR(ss))
 || (XISR(ss) && (ss->pending_priority <= priority))) {
 ics_reject(icp->ics, nr);
@@ -153,6 +161,7 @@ static void icp_irq(struct icp_state *icp, int server, int 
nr, uint8_t priority)
 }
 ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
 ss->pending_priority = priority;
+trace_xics_icp_raise(ss->xirr, ss->pending_priority);
 qemu_irq_raise(ss->output);
 }
 }
@@ -217,10 +226,12 @@ static void set_irq_msi(struct ics_state *ics, int srcno, 
int val)
 {
 struct ics_irq_state *irq = ics->irqs + srcno;
 
+trace_xics_set_irq_msi(srcno, srcno + ics->offset);
+
 if (val) {
 if (irq->priority == 0xff) {
 irq->status |= XICS_STATUS_MASKED_PENDING;
-/* masked pending */ ;
+trace_xics_masked_pending();
 } else  {
 icp_irq(ics->icp, irq->server, srcno + ics->offset, irq->priority);
 }
@@ -231,6 +242,7 @@ static void set_irq_lsi(struct ics_state *ics, int srcno, 
int val)
 {
 struct ics_irq_state *irq = ics->irqs + srcno;
 
+trace_xics_set_irq_lsi(srcno, srcno + ics->offset);
 if (val) {
 irq->status |= XICS_STATUS_ASSERTED;
 } else {
@@ -279,6 +291,8 @@ static void ics_write_xive(struct ics_state *ics, int nr, 
int server,
 irq->priority = priority;
 irq->saved_priority = saved_priority;
 
+trace_xics_ics_write_xive(nr, srcno, server, priority);
+
 if (irq->lsi) {
 write_xive_lsi(ics, srcno);
 } else {
@@ -290,6 +304,7 @@ static void ics_reject(struct ics_state *ics, int nr)
 {
 struct ics_irq_state *irq = ics->irqs + nr - ics->offset;
 
+trace_xics_ics_reject(nr, nr - ics->offset);
 irq->status |= XICS_STATUS_REJECTED; /* Irrelevant but harmless for LSI */
 irq->status &= ~XICS_STATUS_SENT; /* Irrelevant but harmless for MSI */
 }
@@ -315,6 +330,8 @@ static void ics_eoi(struct ics_state *ics, int nr)
 int srcno = nr - ics->offset;
 struct ics_irq_state *irq = ics->irqs + srcno;
 
+trace_xics_ics_eoi(nr);
+
 if (irq->lsi) {
 irq->status &= ~XICS_STATUS_SENT;
 }
diff --git a/trace-events b/trace-events
index b84d631..5bf01f9 100644
--- a/trace-events
+++ b/trace-events
@@ -1020,3 +1020,16 @@ spapr_pci_rtas_ibm_change_msi(unsigned func, unsigned 
req) "func %u, requested %
 spapr_pci_rtas_ibm_query_interrupt_source_number(unsigned ioa, unsigned intr) 
"queries for #%u, IRQ%u"
 spapr_pci_msi_write(uint64_t addr, uint64_t data, uint32_t dt_irq) 
"@%"PRIx64"<=%"PRIx64" IRQ %u"
 spapr_pci_lsi_set(const char *busname, int pin, uint32_t irq) "%s PIN%d IRQ %u"
+
+# hw/xics.c
+xics_icp_check_ipi(int server, uint8_t mfrr) "CPU %d can take IPI mfrr=%#x"
+xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) "icp_accept: XIRR 
%#"PRIx32"->%#"PRIx32
+xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d 
given XIRR %#"PRIx32" new XIRR %#"PRIx32
+xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver 
irq %#"PRIx32" priority %#x"
+xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new 
XIRR=%#x new pending priority=%#x"
+xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
+xics_masked_pending(void) "set_irq_msi: masked pending"
+xics_set_irq_

[Qemu-devel] [PATCH 05/12] pseries: Allow RTAS tokens without a qemu handler

2012-11-12 Thread David Gibson
From: Ben Herrenschmidt 

Kernel-based RTAS calls will not have a qemu handler, but will
still be registered in qemu in order to be assigned a token
number and appear in the device-tree.

Let's test for the name being NULL rather than the handler
when deciding to skip an entry while building the device-tree

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: David Gibson 
---
 hw/spapr_rtas.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/spapr_rtas.c b/hw/spapr_rtas.c
index 45294e8..e618c2d 100644
--- a/hw/spapr_rtas.c
+++ b/hw/spapr_rtas.c
@@ -301,7 +301,7 @@ int spapr_rtas_device_tree_setup(void *fdt, hwaddr 
rtas_addr,
 for (i = 0; i < TOKEN_MAX; i++) {
 struct rtas_call *call = &rtas_table[i];
 
-if (!call->fn) {
+if (!call->name) {
 continue;
 }
 
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v11] kvm: notify host when the guest is panicked

2012-11-12 Thread Marcelo Tosatti
On Fri, Nov 09, 2012 at 03:17:39PM -0500, Sasha Levin wrote:
> On Mon, Nov 5, 2012 at 8:58 PM, Hu Tao  wrote:
> > But in the case of panic notification, more dependency means more
> > chances of failure of panic notification. Say, if we use a virtio device
> > to do panic notification, then we will fail if: virtio itself has
> > problems, virtio for some reason can't be deployed(neither built-in or
> > as a module), or guest doesn't support virtio, etc.
> 
> Add polling to your virtio device. If it didn't notify of a panic but
> taking more than 20 sec to answer your poll request you can assume
> it's dead.
> 
> Actually, just use virtio-serial and something in userspace on the guest.

They want the guest to stop, so a memory dump can be taken by management
interface.

Hu Tao, lets assume port I/O is the preferred method for communication.
Now, the following comments have still not been addressed:

1) Lifecycle of the stopped guest and interaction with other stopped
states in QEMU.

2) Format of the interface for other architectures (you can choose
a different KVM supported architecture and write an example).

3) Clear/documented management interface for the feature.




Re: [Qemu-devel] [PATCH 2/3] s390: Virtual channel subsystem support.

2012-11-12 Thread Marcelo Tosatti
Hi Cornelia,

On Wed, Oct 31, 2012 at 05:24:47PM +0100, Cornelia Huck wrote:
> Provide a mechanism for qemu to provide fully virtual subchannels to
> the guest. In the KVM case, this relies on the kernel's css support
> for I/O and machine check interrupt handling. The !KVM case handles
> interrupts on its own.
> 
> Signed-off-by: Cornelia Huck 
> ---
>  hw/s390x/Makefile.objs |1 +
>  hw/s390x/css.c | 1209 
> 
>  hw/s390x/css.h |   90 
>  target-s390x/Makefile.objs |2 +-
>  target-s390x/cpu.h |  232 +
>  target-s390x/helper.c  |  146 ++
>  target-s390x/ioinst.c  |  737 +++
>  target-s390x/ioinst.h  |  213 
>  target-s390x/kvm.c |  251 -
>  target-s390x/misc_helper.c |6 +-
>  10 files changed, 2872 insertions(+), 15 deletions(-)
>  create mode 100644 hw/s390x/css.c
>  create mode 100644 hw/s390x/css.h
>  create mode 100644 target-s390x/ioinst.c
>  create mode 100644 target-s390x/ioinst.h

> +void kvm_s390_enable_css_support(CPUS390XState *env)
> +{
> +struct kvm_enable_cap cap = {};
> +int r;
> +
> +/* Activate host kernel channel subsystem support. */
> +if (kvm_enabled()) {
> +/* One CPU has to run */
> +s390_add_running_cpu(env);

Care to explain this?

> +
> +cap.cap = KVM_CAP_S390_CSS_SUPPORT;
> +r = kvm_vcpu_ioctl(env, KVM_ENABLE_CAP, &cap);
> +assert(r == 0);
> +}
> +}





[Qemu-devel] [Bug 1077838] Re: qemu-nbd -r -c taints device for subsequent usage, even after -d

2012-11-12 Thread Serge Hallyn
Thanks, this still applies upstream as well.

** Changed in: qemu-kvm (Ubuntu)
   Status: New => Triaged

** Changed in: qemu-kvm (Ubuntu)
   Importance: Undecided => High

** Also 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/1077838

Title:
  qemu-nbd -r -c taints device for subsequent usage, even after -d

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Triaged

Bug description:
  Something about qemu-nbd -r -c /dev/nbd0 someimg leaves cruft behind -
  subsequent connections get marked readonly.

  This is on quantal, haven't checked precise or raring.

  To demonstrate:
  # use one image
  qemu-img create -f qcow2 /tmp/1.qcow2 100M
  sudo qemu-nbd -c /dev/nbd2 /tmp/1.qcow2
  sudo mkfs -t ext4 /dev/nbd2
  sudo qemu-nbd -d /dev/nbd2
  # use a second one on the same nbd device, shows that reuse works:
  qemu-img create -f qcow2 /tmp/2.qcow2 100M
  sudo qemu-nbd -c /dev/nbd2 /tmp/2.qcow2
  sudo mkfs -t ext4 /dev/nbd2
  sudo qemu-nbd -d /dev/nbd2
  # connect an image in read only mode
  sudo qemu-nbd -r -c /dev/nbd2 /tmp/2.qcow2
  sudo dumpe2fs /dev/nbd2 | head -n 3
  sudo qemu-nbd -d /dev/nbd2
  # now try to reuse in read-write mode again:
  qemu-img create -f qcow2 /tmp/3.qcow2 100M
  sudo qemu-nbd -c /dev/nbd2 /tmp/3.qcow2
  sudo mkfs -t ext4 /dev/nbd2
  # here it goes boom:
  mke2fs 1.42.5 (29-Jul-2012)
  /dev/nbd2: Operation not permitted while setting up superblock
  # still need to cleanup
  sudo qemu-nbd -d /dev/nbd2

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1077838/+subscriptions



[Qemu-devel] [PATCH 00/17] target-i386: CPU init cleanup for CPU classes/properties

2012-11-12 Thread Eduardo Habkost
Hi,

One of the problems when coordinating with the recent x86 CPU work (CPU
properties, APIC ID topology fixes) is to have a sane x86 CPU initialization
code to be used as base.

To make things worse, the cleanups we're doing are buried inside series that do
more intrusive work. This is an attempt to establish a saner base for further
work.

Except for additional error messages when initialization fails, this series
should not introduce any behavior change, but just clean up cpu_x86_init() to
look like this:

name, features = g_strsplit(cpu_string, ",", 2);
cpu = X86_CPU(object_new(TYPE_X86_CPU));
cpu_x86_find_cpudef(name, &def, &error)
cpu_x86_parse_featurestr(&def, features, &error)
cpudef_2_x86_cpu(cpu, &def, &error)
x86_cpu_realize(OBJECT(cpu), &error);

Basically, this isolates the parts of the code that do:
 - Splitting cpu_model string into CPU model and feature string;
 - CPU object creation;
 - CPU model lookup;
 - CPU feature string parsing.

After this, I will send an additional patch to introduce CPU classes on
target-i386, based on this series.

Eduardo Habkost (16):
  target-i386/cpu.c: coding style fix
  target-i386: move cpu_x86_init() to cpu.c
  target-i386: cpu: rename x86_def_t to X86CPUDefinition
  target-i386: x86_cpudef_setup(): cosmetic change on comment
  target-i386: cpu_x86_init(): move error handling to end of function
  target-i386: cpu_x86_init(): print error message in case of error
  target-i386: cpu_x86_register(): report errors using Error parameter
  target-i386: cpu_x86_register(): reorder CPU property setting
  target-i386: kill cpu_x86_register()
  target-i386: return Error from cpu_x86_find_by_name()
  target-i386: cpu_x86_find_by_name(): split CPU model and feature
string first
  target-i386: cpu: create cpu_x86_find_cpudef() function
  target-i386: cpu_x86_init(): rename cpu_model to cpu_string
  target-i386: cpu_x86_init(): eliminate extra 'def1' variable
  target-i386: cpu: separate cpudef lookup from feature string parsing
  target-i386: cpu_x86_init(): reorder split of CPU string and creation
of CPU object

Igor Mammedov (1):
  target-i386: move out CPU features initialization to separate func

 target-i386/cpu.c| 175 ++-
 target-i386/cpu.h|   1 -
 target-i386/helper.c |  24 ---
 3 files changed, 118 insertions(+), 82 deletions(-)

-- 
1.7.11.7




[Qemu-devel] [PATCH 14/17] target-i386: cpu_x86_init(): rename cpu_model to cpu_string

2012-11-12 Thread Eduardo Habkost
Rename the variable, to avoid confusion between the actual CPU model
name and the -cpu string argument (that may contain additional
parameters).

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index c2594bd..57acf3a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1521,7 +1521,7 @@ static int cpudef_2_x86_cpu(X86CPU *cpu, X86CPUDefinition 
*def, Error **errp)
 return 0;
 }
 
-X86CPU *cpu_x86_init(const char *cpu_model)
+X86CPU *cpu_x86_init(const char *cpu_string)
 {
 X86CPU *cpu;
 CPUX86State *env;
@@ -1530,11 +1530,11 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
 env = &cpu->env;
-env->cpu_model_str = cpu_model;
+env->cpu_model_str = cpu_string;
 
 memset(def, 0, sizeof(*def));
 
-if (cpu_x86_find_by_name(def, cpu_model, &error) < 0) {
+if (cpu_x86_find_by_name(def, cpu_string, &error) < 0) {
 goto error;
 }
 
-- 
1.7.11.7




[Qemu-devel] [PATCH 02/17] target-i386: move cpu_x86_init() to cpu.c

2012-11-12 Thread Eduardo Habkost
Eventually all of the CPU init code will probably become just a simple
object_new() call, with some arch-independent function that handles the
CPU model string parsing. But right now we need to reorder and split
many of the steps invoved in the CPU model string parsing and CPU object
creation, and it will be easier to do that inside cpu.c, by now.

Also, make cpu_x86_register() static, as now it is only used inside
cpu.c.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c| 26 +-
 target-i386/cpu.h|  1 -
 target-i386/helper.c | 24 
 3 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index fa8b5bd..b50ca8c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1423,7 +1423,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
 CPUX86State *env = &cpu->env;
 x86_def_t def1, *def = &def1;
@@ -1494,6 +1494,30 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 return 0;
 }
 
+X86CPU *cpu_x86_init(const char *cpu_model)
+{
+X86CPU *cpu;
+CPUX86State *env;
+Error *error = NULL;
+
+cpu = X86_CPU(object_new(TYPE_X86_CPU));
+env = &cpu->env;
+env->cpu_model_str = cpu_model;
+
+if (cpu_x86_register(cpu, cpu_model) < 0) {
+object_delete(OBJECT(cpu));
+return NULL;
+}
+
+x86_cpu_realize(OBJECT(cpu), &error);
+if (error) {
+error_free(error);
+object_delete(OBJECT(cpu));
+return NULL;
+}
+return cpu;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 
 void cpu_clear_apic_feature(CPUX86State *env)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index cdc59dc..4d5510e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -956,7 +956,6 @@ int cpu_x86_signal_handler(int host_signum, void *pinfo,
 void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
uint32_t *eax, uint32_t *ebx,
uint32_t *ecx, uint32_t *edx);
-int cpu_x86_register(X86CPU *cpu, const char *cpu_model);
 void cpu_clear_apic_feature(CPUX86State *env);
 void host_cpuid(uint32_t function, uint32_t count,
 uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index bf206cf..47b53ed 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1240,30 +1240,6 @@ int cpu_x86_get_descr_debug(CPUX86State *env, unsigned 
int selector,
 return 1;
 }
 
-X86CPU *cpu_x86_init(const char *cpu_model)
-{
-X86CPU *cpu;
-CPUX86State *env;
-Error *error = NULL;
-
-cpu = X86_CPU(object_new(TYPE_X86_CPU));
-env = &cpu->env;
-env->cpu_model_str = cpu_model;
-
-if (cpu_x86_register(cpu, cpu_model) < 0) {
-object_delete(OBJECT(cpu));
-return NULL;
-}
-
-x86_cpu_realize(OBJECT(cpu), &error);
-if (error) {
-error_free(error);
-object_delete(OBJECT(cpu));
-return NULL;
-}
-return cpu;
-}
-
 #if !defined(CONFIG_USER_ONLY)
 void do_cpu_init(X86CPU *cpu)
 {
-- 
1.7.11.7




[Qemu-devel] [PATCH 11/17] target-i386: return Error from cpu_x86_find_by_name()

2012-11-12 Thread Eduardo Habkost
It will allow us to use property setters there later.

Signed-off-by: Igor Mammedov 
[ehabkost: rebased on top of CPU classes work in progress]
Signed-off-by: Eduardo Habkost 
---
v2:
- style change, add braces (requested by Blue Swirl)
- removed unused error_is_set(errp) in properties set loop
v3:
- removed unnecessary whitespace change

v4 (ehabkost):
- rebased on top of CPU classes work in progress
- reworded commit message
---
 target-i386/cpu.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3f04054..214a292 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1147,7 +1147,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 }
 
 static int cpu_x86_find_by_name(X86CPUDefinition *x86_cpu_def,
-const char *cpu_model)
+const char *cpu_model, Error **errp)
 {
 unsigned int i;
 X86CPUDefinition *def;
@@ -1320,6 +1320,9 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 
 error:
 g_free(s);
+if (!error_is_set(errp)) {
+error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+}
 return -1;
 }
 
@@ -1502,7 +1505,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 
 memset(def, 0, sizeof(*def));
 
-if (cpu_x86_find_by_name(def, cpu_model) < 0) {
+if (cpu_x86_find_by_name(def, cpu_model, &error) < 0) {
 goto error;
 }
 
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

2012-11-12 Thread Eduardo Habkost
On Mon, Nov 12, 2012 at 10:18:29PM +, Peter Maydell wrote:
> On 12 November 2012 22:16, Eduardo Habkost  wrote:
> >
> > Sorry for replying to a patch 7 months later, but I just have a question
> > related to how we will handle CPU model classes on all targets:
> >
> > On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> >> Register subclasses for each ARM CPU implementation (with the
> >> exception of "pxa270", which is an alias for "pxa270-a0").
> >>
> >> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
> >> except for special value "any".
> >>
> >> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
> >> CPUID (aka MIDR, Main ID Register) value in the class.
> >>
> >> Signed-off-by: Andreas Färber 
> >> Signed-off-by: Peter Maydell 
> >> ---
> >>  target-arm/cpu-qom.h |   12 +++
> >>  target-arm/cpu.c |  226 
> >> +-
> >>  target-arm/helper.c  |  109 ++--
> >>  3 files changed, 282 insertions(+), 65 deletions(-)
> >>
> >> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> >> index 42d2a6b..a4bcb31 100644
> >> --- a/target-arm/cpu-qom.h
> >> +++ b/target-arm/cpu-qom.h
> >> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
> > [...]
> >> +typedef struct ARMCPUInfo {
> >> +const char *name;
> >> +void (*initfn)(Object *obj);
> >> +} ARMCPUInfo;
> >> +
> >> +static const ARMCPUInfo arm_cpus[] = {
> > [...]
> >> +{ .name = "any", .initfn = arm_any_initfn },
> >> +};
> >> +
> >
> > Do we really want to use "any" as the class name?
> 
> Probably not, since it would make it tricky to (in some future
> utopia) have a QEMU which supported more than one CPU architecture
> in the same binary if they all wanted to use "any"...

In that case, "cpu-any" wouldn't work, either. What about
"-cpu-"?

> 
> > Maybe we should use
> > "cpu-" as the namespace for the CPU model class names?
> 
> Sounds reasonable.
> 
> -- PMM

-- 
Eduardo



Re: [Qemu-devel] KVM call agenda for 2012-11-12

2012-11-12 Thread Marcelo Tosatti
On Mon, Nov 12, 2012 at 01:58:38PM +0100, Juan Quintela wrote:
> 
> Hi
> 
> Please send in any agenda topics you are interested in.
> 
> Later, Juan.

It would be good to have a status report on qemu-kvm compatibility
(the remaining TODO items are with Anthony). They are:

- qemu-kvm 1.2 machine type.
- default accelerator being KVM.

Note migration will remain broken due to 

https://patchwork.kernel.org/patch/1674521/

BTW, this can be via email, if preferred (i cannot attend the call).





[Qemu-devel] [PATCH] vga: fix mmio vga register mapping

2012-11-12 Thread Gerd Hoffmann
---
 hw/vga-pci.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index ec29cac..947e35c 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -84,9 +84,10 @@ static void pci_vga_ioport_write(void *ptr, hwaddr addr,
  uint64_t val, unsigned size)
 {
 PCIVGAState *d = ptr;
+
 switch (size) {
 case 1:
-vga_ioport_write(&d->vga, addr, val);
+vga_ioport_write(&d->vga, addr + 0x3c0, val);
 break;
 case 2:
 /*
@@ -94,8 +95,8 @@ static void pci_vga_ioport_write(void *ptr, hwaddr addr,
  * indexed registers with a single word write because the
  * index byte is updated first.
  */
-vga_ioport_write(&d->vga, addr, val & 0xff);
-vga_ioport_write(&d->vga, addr+1, (val >> 8) & 0xff);
+vga_ioport_write(&d->vga, addr + 0x3c0, val & 0xff);
+vga_ioport_write(&d->vga, addr + 0x3c1, (val >> 8) & 0xff);
 break;
 }
 }
-- 
1.7.1




Re: [Qemu-devel] [PATCH 01/24] user: move *-user/qemu-types.h to main directory

2012-11-12 Thread Andreas Färber
Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The bsd-user/qemu-types.h and linux-user/qemu-types.h files are almost
> the same, but linux-user have the additional definitions of tswapal().
> 
> This moves the linux-user file to the main directory, so the same file
> can be used by linux-user and bsd-user.
> 
> Signed-off-by: Eduardo Habkost 

A quick diff -u confirms that this is (still) the case. I had suggested
unifying these files long time ago when I tried fixing darwin-user, but
hit resistance in favor of having the *-users separate. I'm still in
favor of not duplicating identical code, so

Acked-by: Andreas Färber 

Riku, do you agree?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 04/17] target-i386: x86_cpudef_setup(): cosmetic change on comment

2012-11-12 Thread Eduardo Habkost
There are no "cpudef" models, all of them are builtin, now.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2dfcc9c..73b0fa1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1539,8 +1539,7 @@ void x86_cpudef_setup(void)
 X86CPUDefinition *def = &builtin_x86_defs[i];
 def->next = x86_defs;
 
-/* Look for specific "cpudef" models that */
-/* have the QEMU version in .model_id */
+/* Look for specific models that have the QEMU version in .model_id */
 for (j = 0; j < ARRAY_SIZE(model_with_versions); j++) {
 if (strcmp(model_with_versions[j], def->name) == 0) {
 pstrcpy(def->model_id, sizeof(def->model_id),
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

2012-11-12 Thread Peter Maydell
On 12 November 2012 22:16, Eduardo Habkost  wrote:
>
> Sorry for replying to a patch 7 months later, but I just have a question
> related to how we will handle CPU model classes on all targets:
>
> On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
>> Register subclasses for each ARM CPU implementation (with the
>> exception of "pxa270", which is an alias for "pxa270-a0").
>>
>> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
>> except for special value "any".
>>
>> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
>> CPUID (aka MIDR, Main ID Register) value in the class.
>>
>> Signed-off-by: Andreas Färber 
>> Signed-off-by: Peter Maydell 
>> ---
>>  target-arm/cpu-qom.h |   12 +++
>>  target-arm/cpu.c |  226 
>> +-
>>  target-arm/helper.c  |  109 ++--
>>  3 files changed, 282 insertions(+), 65 deletions(-)
>>
>> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
>> index 42d2a6b..a4bcb31 100644
>> --- a/target-arm/cpu-qom.h
>> +++ b/target-arm/cpu-qom.h
>> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
> [...]
>> +typedef struct ARMCPUInfo {
>> +const char *name;
>> +void (*initfn)(Object *obj);
>> +} ARMCPUInfo;
>> +
>> +static const ARMCPUInfo arm_cpus[] = {
> [...]
>> +{ .name = "any", .initfn = arm_any_initfn },
>> +};
>> +
>
> Do we really want to use "any" as the class name?

Probably not, since it would make it tricky to (in some future
utopia) have a QEMU which supported more than one CPU architecture
in the same binary if they all wanted to use "any"...

> Maybe we should use
> "cpu-" as the namespace for the CPU model class names?

Sounds reasonable.

-- PMM



Re: [Qemu-devel] [PATCH v2 2/2] qemu-ga: sample fsfreeze hooks

2012-11-12 Thread Eric Blake
On 11/12/2012 02:32 AM, Tomoki Sekiyama wrote:
> Adds sample hook scripts for --fsfreeze-hook option of qemu-ga.
>   - fsfreeze-hook : execute scripts in fsfreeze-hook.d/
>   - fsfreeze-hook.d.sample/mysql-flush.sh : quiesce MySQL before snapshot
> 
> Signed-off-by: Tomoki Sekiyama 
> ---

> +LOGFILE=/var/log/qga-fsreeze-hook.log

s/fsreeze/fsfreeze/

> +# Iterate executables in directory "fsfreeze-hook.d" with the specified args
> +[ ! -d "$FSFREEZE_D" ] && exit 1
> +for file in "$FSFREEZE_D"/* ; do
> +is_ignored_file "$file" && continue
> +[ -x "$file" ] || continue
> +echo $(date) ": execute $file $@" >>$LOGFILE

Put $(date) inside the ""; otherwise, single-digit days will be one
character shorter due to IFS field splitting eating the double-space;
and I don't like unaligned dates in logs.

> +"$file" "$@" >>$LOGFILE 2>&1
> +STATUS=$?
> +echo $(date) ": $file finished with status=$STATUS" >>$LOGFILE

and again.

> +INNODB_STATUS=$(mktemp /tmp/mysql-flush.XX)
> +[ $? -ne 0 ] && exit 2
> +trap "rm -f $INNODB_STATUS" SIGINT
> +while :; do
> +echo 'SHOW ENGINE INNODB STATUS \G' | $MYSQL > $INNODB_STATUS

'echo' cannot portably be used with \.  Use 'printf' instead.

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 12/17] target-i386: cpu_x86_find_by_name(): split CPU model and feature string first

2012-11-12 Thread Eduardo Habkost
Instead of using strtok() for the whole cpu_model string, first split it
into the CPU model name and the full feature string, then parse the
feature string into pieces.

When using CPU model classes, those two pieces of information will be
used at different moments (CPU model name will be used to find CPU
class, feature string will be used after CPU object was created), so
making the split in two steps will make it easier to refactor the code
later.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 21 +++--
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 214a292..3a85989 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1151,9 +1151,10 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 {
 unsigned int i;
 X86CPUDefinition *def;
-
-char *s = g_strdup(cpu_model);
-char *featurestr, *name = strtok(s, ",");
+char *name; /* CPU model name */
+char *features; /* Full feature "key=value,..." string */
+char *featurestr; /* Single 'key=value" string being parsed */
+gchar **model_pieces; /* array after split of name,features */
 /* Features to be added*/
 uint32_t plus_features = 0, plus_ext_features = 0;
 uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
@@ -1166,6 +1167,14 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 uint32_t minus_7_0_ebx_features = 0;
 uint32_t numvalue;
 
+model_pieces = g_strsplit(cpu_model, ",", 2);
+if (!model_pieces[0]) {
+goto error;
+}
+
+name = model_pieces[0];
+features = model_pieces[1];
+
 for (def = x86_defs; def; def = def->next)
 if (name && !strcmp(name, def->name))
 break;
@@ -1181,7 +1190,7 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
 &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
 
-featurestr = strtok(NULL, ",");
+featurestr = features ? strtok(features, ",") : NULL;
 
 while (featurestr) {
 char *val;
@@ -1315,11 +1324,11 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
 x86_cpu_def->level = 7;
 }
-g_free(s);
+g_strfreev(model_pieces);
 return 0;
 
 error:
-g_free(s);
+g_strfreev(model_pieces);
 if (!error_is_set(errp)) {
 error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
 }
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH v2 01/14] target-arm: Add QOM subclasses for each ARM cpu implementation

2012-11-12 Thread Eduardo Habkost

Sorry for replying to a patch 7 months later, but I just have a question
related to how we will handle CPU model classes on all targets:

On Sat, Apr 14, 2012 at 05:42:10PM +0100, Peter Maydell wrote:
> Register subclasses for each ARM CPU implementation (with the
> exception of "pxa270", which is an alias for "pxa270-a0").
> 
> Let arm_cpu_list() enumerate CPU subclasses in alphabetical order,
> except for special value "any".
> 
> Replace cpu_arm_find_by_name()'s string -> CPUID lookup by storing the
> CPUID (aka MIDR, Main ID Register) value in the class.
> 
> Signed-off-by: Andreas Färber 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/cpu-qom.h |   12 +++
>  target-arm/cpu.c |  226 
> +-
>  target-arm/helper.c  |  109 ++--
>  3 files changed, 282 insertions(+), 65 deletions(-)
> 
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 42d2a6b..a4bcb31 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -58,6 +58,18 @@ typedef struct ARMCPU {
[...]
> +typedef struct ARMCPUInfo {
> +const char *name;
> +void (*initfn)(Object *obj);
> +} ARMCPUInfo;
> +
> +static const ARMCPUInfo arm_cpus[] = {
[...]
> +{ .name = "any", .initfn = arm_any_initfn },
> +};
> +

Do we really want to use "any" as the class name? Maybe we should use
"cpu-" as the namespace for the CPU model class names? Or maybe
try "cpu-" first, and then "" as a fallback (making sure
that the class we found is a subclass of TYPE__CPU).

I guess we will want address this before qdevifying the CPU class, as
the qdevification will make the CPU class names visible through the
monitor.


>  static void arm_cpu_class_init(ObjectClass *oc, void *data)
>  {
>  ARMCPUClass *acc = ARM_CPU_CLASS(oc);
> @@ -43,18 +248,37 @@ static void arm_cpu_class_init(ObjectClass *oc, void 
> *data)
>  cc->reset = arm_cpu_reset;
>  }
>  
> +static void cpu_register(const ARMCPUInfo *info)
> +{
> +TypeInfo type_info = {
> +.name = info->name,
> +.parent = TYPE_ARM_CPU,
> +.instance_size = sizeof(ARMCPU),
> +.instance_init = info->initfn,
> +.class_size = sizeof(ARMCPUClass),
> +};
> +
> +type_register_static(&type_info);
> +}
> +
>  static const TypeInfo arm_cpu_type_info = {
>  .name = TYPE_ARM_CPU,
>  .parent = TYPE_CPU,
>  .instance_size = sizeof(ARMCPU),
> -.abstract = false,
> +.instance_init = arm_cpu_initfn,
> +.abstract = true,
>  .class_size = sizeof(ARMCPUClass),
>  .class_init = arm_cpu_class_init,
>  };
>  
>  static void arm_cpu_register_types(void)
>  {
> +int i;
> +
>  type_register_static(&arm_cpu_type_info);
> +for (i = 0; i < ARRAY_SIZE(arm_cpus); i++) {
> +cpu_register(&arm_cpus[i]);
> +}
>  }
>  
>  type_init(arm_cpu_register_types)

-- 
Eduardo



Re: [Qemu-devel] [PATCH 03/24] qemu-common.h: comment about usage rules

2012-11-12 Thread Eduardo Habkost
On Mon, Nov 12, 2012 at 10:57:42PM +0100, Andreas Färber wrote:
> Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> > Every time we make a tiny change on a header file, we often find
> > circular header dependency problems. To avoid this nightmare, we need to
> > stop including qemu-common.h on other headers, and we should gradually
> 
> "from other headers" as below?

Both forms sounds equivalent, to my non-native-speaker ears. :-)

But I guess "including from other headers" is better.

> 
> > move the declarations from the catchall qemu-common.h header to their
> > specific headers.
> > 
> > This simply adds a comment documenting the rules about qemu-common.h,
> > hoping that people will see it before including qemu-common.h from other
> > header files, and before adding more declarations to qemu-common.h.
> 
> This reminds me that I had once posted a patch moving a declaration I
> had once added for Cocoa to a new ui/ui.h... seems it never made it to
> master, I'll go search, maybe we can smuggle that in now. ;)
> 
> > 
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  qemu-common.h | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qemu-common.h b/qemu-common.h
> > index ac9985c..ea43bfa 100644
> > --- a/qemu-common.h
> > +++ b/qemu-common.h
> > @@ -1,5 +1,14 @@
> >  
> > -/* Common header file that is included by all of qemu.  */
> > +/* Common header file that is included by all of qemu.
> 
> "QEMU", while at it.
> 
> > + *
> > + * This file is supposed to be included only by .c files. No header file 
> > should
> > + * depend on qemu-common.h, as this would easily lead to circular header
> > + * dependencies.
> > + *
> > + * If a header files uses a definition from qemu-common.h, that definition
> 
> "a header file"

Oops. Thanks.

> 
> > + * must be moved to a separate header file, and the header that uses it
> > + * must include that header.
> > + */
> >  #ifndef QEMU_COMMON_H
> >  #define QEMU_COMMON_H
> >  
> 
> I'll fix this up myself to spare you a resend and me another full review.

Thanks!

> 
> Regards,
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

-- 
Eduardo



[Qemu-devel] [PATCH 07/17] target-i386: cpu_x86_register(): report errors using Error parameter

2012-11-12 Thread Eduardo Habkost
Do it using a local Error variable and error_propagate(), so we don't
miss any error reported by the property setters in case errp is NULL.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 2907fb0..9334e0c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1424,7 +1424,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
+static int cpu_x86_register(X86CPU *cpu, const char *cpu_model, Error **errp)
 {
 CPUX86State *env = &cpu->env;
 X86CPUDefinition def1, *def = &def1;
@@ -1488,8 +1488,7 @@ static int cpu_x86_register(X86CPU *cpu, const char 
*cpu_model)
 }
 object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
 if (error) {
-fprintf(stderr, "%s\n", error_get_pretty(error));
-error_free(error);
+error_propagate(errp, error);
 return -1;
 }
 return 0;
@@ -1505,7 +1504,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 env = &cpu->env;
 env->cpu_model_str = cpu_model;
 
-if (cpu_x86_register(cpu, cpu_model) < 0) {
+if (cpu_x86_register(cpu, cpu_model, &error) < 0) {
 goto error;
 }
 
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH 03/24] qemu-common.h: comment about usage rules

2012-11-12 Thread Andreas Färber
Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> Every time we make a tiny change on a header file, we often find
> circular header dependency problems. To avoid this nightmare, we need to
> stop including qemu-common.h on other headers, and we should gradually

"from other headers" as below?

> move the declarations from the catchall qemu-common.h header to their
> specific headers.
> 
> This simply adds a comment documenting the rules about qemu-common.h,
> hoping that people will see it before including qemu-common.h from other
> header files, and before adding more declarations to qemu-common.h.

This reminds me that I had once posted a patch moving a declaration I
had once added for Cocoa to a new ui/ui.h... seems it never made it to
master, I'll go search, maybe we can smuggle that in now. ;)

> 
> Signed-off-by: Eduardo Habkost 
> ---
>  qemu-common.h | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-common.h b/qemu-common.h
> index ac9985c..ea43bfa 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -1,5 +1,14 @@
>  
> -/* Common header file that is included by all of qemu.  */
> +/* Common header file that is included by all of qemu.

"QEMU", while at it.

> + *
> + * This file is supposed to be included only by .c files. No header file 
> should
> + * depend on qemu-common.h, as this would easily lead to circular header
> + * dependencies.
> + *
> + * If a header files uses a definition from qemu-common.h, that definition

"a header file"

> + * must be moved to a separate header file, and the header that uses it
> + * must include that header.
> + */
>  #ifndef QEMU_COMMON_H
>  #define QEMU_COMMON_H
>  

I'll fix this up myself to spare you a resend and me another full review.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 15/17] target-i386: cpu_x86_init(): eliminate extra 'def1' variable

2012-11-12 Thread Eduardo Habkost
Just use '&def' where a pointer to the under-construction
X86CPUDefinition struct is being used.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 57acf3a..9b8e480 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1526,19 +1526,19 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 X86CPU *cpu;
 CPUX86State *env;
 Error *error = NULL;
-X86CPUDefinition def1, *def = &def1;
+X86CPUDefinition def;
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
 env = &cpu->env;
 env->cpu_model_str = cpu_string;
 
-memset(def, 0, sizeof(*def));
+memset(&def, 0, sizeof(def));
 
-if (cpu_x86_find_by_name(def, cpu_string, &error) < 0) {
+if (cpu_x86_find_by_name(&def, cpu_string, &error) < 0) {
 goto error;
 }
 
-if (cpudef_2_x86_cpu(cpu, def, &error) < 0) {
+if (cpudef_2_x86_cpu(cpu, &def, &error) < 0) {
 goto error;
 }
 
-- 
1.7.11.7




[Qemu-devel] [RFC] target-i386: register a class for each CPU model

2012-11-12 Thread Eduardo Habkost
This creates the following class hierarchy:

- TYPE_X86_CPU ("-cpu")
 - TYPE_X86_DEFCPU "-cpu-predefined": abstract base class for the
   predefined CPU models
   - "-cpu-model-": a class for each predefined CPU model
 - TYPE_X86_HOST_CPU ("-cpu-model-host"): class for the "-cpu host" CPU
   model

On TARGET_X86_64, "" is "x86_64", on TARGET_I386, "" is
"i386".

The trick here is to replace exactly what's implemented in the
cpu_x86_find_cpudef() logic and nothing else. So, the only difference in
relation to the previous code is that instead of looking at the CPU model table
on cpu_x86_find_cpudef(), we just use the right CPU class, that will fill the
X86CPUDefinition struct on a ->init_cpudef() method.

The init_cpudef() method was added jut to not require us to kill the
X86CPUDefinition array in the same patch. But the X86CPUDefinition
struct may eventually go away, and all the default-value initialization
for each CPU model can become just different defaults set on
instance_init() or class_init().

Signed-off-by: Eduardo Habkost 
---
I am using those class names because I don't want the CPU model names to live
in the same namespace as all device names. I want to avoid doing the same
mistake that was done in the arm code, that registers a QOM class named "any".
Some CPU model names, like "qemu64", don't make any sense unless you already
know it is a CPU model name.

As an alternative to the complex naming above, I was considering simply using
"cpu-" as the class name. I don't know what others think.

This patch depends on the "[PATCH 00/17] target-i386: CPU init cleanup for CPU
classes/properties" series I have just sent.

Changes v1 -> v2:
 - Rebase on top of CPU properties series
 - Use a static type (with a different class init function) for the
   "-cpu host" class

Changes v2 -> v3:
 - Fix the "device not found" error to use the CPU model name, not
   the full -cpu string
 - kill cpudef_init() and cpudef_setup(), as we don't need a
   cpudef-specific initialization function, anymore
 - Instead of keeping a copy of a X86CPUDefinition struct inside
   X86CPUClass, keep only a pointer
 - Add a init_cpudef() class method, used to fill the X86CPUDefinition
   struct for the CPU
 - Use separate subclass for "-cpu host", that uses cpu_x86_fill_host()
   on init_cpudef()

Changes v3 -> v4:
 - Rebase on top of qemu.git master, without the CPU properties series
 - Rename CPU classes to "-cpu-model"
---
 arch_init.c   |   7 --
 arch_init.h   |   1 -
 bsd-user/main.c   |   3 -
 linux-user/main.c |   3 -
 target-i386/cpu-qom.h |  22 +-
 target-i386/cpu.c | 208 +-
 target-i386/cpu.h |   2 -
 vl.c  |   7 --
 8 files changed, 192 insertions(+), 61 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index e6effe8..bea1b9c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1124,13 +1124,6 @@ void do_smbios_option(const char *optarg)
 #endif
 }
 
-void cpudef_init(void)
-{
-#if defined(cpudef_setup)
-cpudef_setup(); /* parse cpu definitions in target config file */
-#endif
-}
-
 int audio_available(void)
 {
 #ifdef HAS_AUDIO
diff --git a/arch_init.h b/arch_init.h
index 5fc780c..84a7f9a 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -27,7 +27,6 @@ extern const uint32_t arch_type;
 void select_soundhw(const char *optarg);
 void do_acpitable_option(const char *optarg);
 void do_smbios_option(const char *optarg);
-void cpudef_init(void);
 int audio_available(void);
 void audio_init(ISABus *isa_bus, PCIBus *pci_bus);
 int tcg_available(void);
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 095ae8e..cf0a345 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -762,9 +762,6 @@ int main(int argc, char **argv)
 }
 
 cpu_model = NULL;
-#if defined(cpudef_setup)
-cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
-#endif
 
 optind = 1;
 for(;;) {
diff --git a/linux-user/main.c b/linux-user/main.c
index 25e35cd..e881fcf 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3426,9 +3426,6 @@ int main(int argc, char **argv, char **envp)
 }
 
 cpu_model = NULL;
-#if defined(cpudef_setup)
-cpudef_setup(); /* parse cpu definitions in target config file (TBD) */
-#endif
 
 /* init debug */
 cpu_set_log_filename(log_file);
diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index 5901140..01ea9de 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -37,19 +37,37 @@
 #define X86_CPU_GET_CLASS(obj) \
 OBJECT_GET_CLASS(X86CPUClass, (obj), TYPE_X86_CPU)
 
+
+struct X86CPUDefinition;
+typedef struct X86CPUDefinition X86CPUDefinition;
+
+struct X86CPUClass;
+typedef struct X86CPUClass X86CPUClass;
+
 /**
  * X86CPUClass:
  * @parent_reset: The parent class' reset handler.
+ * @init_cpudef: initialize X86CPUDefinition struct for CPU class
  *
  * An x86 CPU model or family.
  */
-typedef struct X86CPUClass {
+struct X86CPUClass {
 /*< private >*/
 

[Qemu-devel] [PATCH 13/17] target-i386: cpu: create cpu_x86_find_cpudef() function

2012-11-12 Thread Eduardo Habkost
Move the code that looks for a given CPU model to a separate function.

This will make it easier to separate the cpudef lookup code and the
feature string parsing code, later.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 3a85989..c2594bd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1146,6 +1146,33 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 cpu->env.tsc_khz = value / 1000;
 }
 
+/* Find a CPU model definition and put the result on a X86CPUDefinition struct
+ */
+static int cpu_x86_find_cpudef(const char *name,
+   X86CPUDefinition *result,
+   Error **errp)
+{
+X86CPUDefinition *def;
+
+for (def = x86_defs; def; def = def->next)
+if (name && !strcmp(name, def->name))
+break;
+if (kvm_enabled() && name && strcmp(name, "host") == 0) {
+kvm_cpu_fill_host(result);
+} else if (!def) {
+goto error;
+} else {
+memcpy(result, def, sizeof(*def));
+}
+return 0;
+
+error:
+if (!error_is_set(errp)) {
+error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
+}
+return -1;
+}
+
 static int cpu_x86_find_by_name(X86CPUDefinition *x86_cpu_def,
 const char *cpu_model, Error **errp)
 {
@@ -1175,15 +1202,8 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 name = model_pieces[0];
 features = model_pieces[1];
 
-for (def = x86_defs; def; def = def->next)
-if (name && !strcmp(name, def->name))
-break;
-if (kvm_enabled() && name && strcmp(name, "host") == 0) {
-kvm_cpu_fill_host(x86_cpu_def);
-} else if (!def) {
+if (cpu_x86_find_cpudef(name, x86_cpu_def, errp) < 0) {
 goto error;
-} else {
-memcpy(x86_cpu_def, def, sizeof(*def));
 }
 
 add_flagname_to_bitmaps("hypervisor", &plus_features,
-- 
1.7.11.7




Re: [Qemu-devel] [PATCH 02/24] user: rename qemu-types.h to qemu-user-types.h

2012-11-12 Thread Andreas Färber
Am 09.11.2012 15:56, schrieb Eduardo Habkost:
> The header file is specific for *-user, but I plan to introduce a more
> generic qemu-types.h file, so I'm renaming it.
> 
> Signed-off-by: Eduardo Habkost 

linux-user builds okay and the bsd-user change looks fine, too.
git-grep shows no more occurrences of qemu-types.h.

Acked-by: Andreas Färber 

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 01/17] target-i386/cpu.c: coding style fix

2012-11-12 Thread Eduardo Habkost
Use spaces instead of tabs on cpu_x86_cpuid().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e1db639..fa8b5bd 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1773,17 +1773,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 }
 break;
 case 0x800A:
-   if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
-   *eax = 0x0001; /* SVM Revision */
-   *ebx = 0x0010; /* nr of ASIDs */
-   *ecx = 0;
-   *edx = env->cpuid_svm_features; /* optional features */
-   } else {
-   *eax = 0;
-   *ebx = 0;
-   *ecx = 0;
-   *edx = 0;
-   }
+if (env->cpuid_ext3_features & CPUID_EXT3_SVM) {
+*eax = 0x0001; /* SVM Revision */
+*ebx = 0x0010; /* nr of ASIDs */
+*ecx = 0;
+*edx = env->cpuid_svm_features; /* optional features */
+} else {
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+}
 break;
 case 0xC000:
 *eax = env->cpuid_xlevel2;
-- 
1.7.11.7




[Qemu-devel] [PATCH 16/17] target-i386: cpu: separate cpudef lookup from feature string parsing

2012-11-12 Thread Eduardo Habkost
- Move the CPU string split and cpudef lookup to cpu_x86_init();
- Rename cpu_x86_find_by_name() to cpu_x86_parse_feature_string(),
  and make it just get the feature string as input.

This will allow us to use the CPU model name for the CPU class lookup,
inside cpu_x86_init().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 40 +++-
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9b8e480..e47ec5d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1173,15 +1173,11 @@ error:
 return -1;
 }
 
-static int cpu_x86_find_by_name(X86CPUDefinition *x86_cpu_def,
-const char *cpu_model, Error **errp)
+static int cpu_x86_parse_featurestr(X86CPUDefinition *x86_cpu_def,
+char *features, Error **errp)
 {
 unsigned int i;
-X86CPUDefinition *def;
-char *name; /* CPU model name */
-char *features; /* Full feature "key=value,..." string */
 char *featurestr; /* Single 'key=value" string being parsed */
-gchar **model_pieces; /* array after split of name,features */
 /* Features to be added*/
 uint32_t plus_features = 0, plus_ext_features = 0;
 uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
@@ -1194,18 +1190,6 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 uint32_t minus_7_0_ebx_features = 0;
 uint32_t numvalue;
 
-model_pieces = g_strsplit(cpu_model, ",", 2);
-if (!model_pieces[0]) {
-goto error;
-}
-
-name = model_pieces[0];
-features = model_pieces[1];
-
-if (cpu_x86_find_cpudef(name, x86_cpu_def, errp) < 0) {
-goto error;
-}
-
 add_flagname_to_bitmaps("hypervisor", &plus_features,
 &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
 &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
@@ -1344,11 +1328,9 @@ static int cpu_x86_find_by_name(X86CPUDefinition 
*x86_cpu_def,
 if (x86_cpu_def->cpuid_7_0_ebx_features && x86_cpu_def->level < 7) {
 x86_cpu_def->level = 7;
 }
-g_strfreev(model_pieces);
 return 0;
 
 error:
-g_strfreev(model_pieces);
 if (!error_is_set(errp)) {
 error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
 }
@@ -1527,6 +1509,8 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 CPUX86State *env;
 Error *error = NULL;
 X86CPUDefinition def;
+char *name, *features;
+gchar **model_pieces;
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
 env = &cpu->env;
@@ -1534,7 +1518,18 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 
 memset(&def, 0, sizeof(def));
 
-if (cpu_x86_find_by_name(&def, cpu_string, &error) < 0) {
+model_pieces = g_strsplit(cpu_string, ",", 2);
+if (!model_pieces[0]) {
+goto error;
+}
+name = model_pieces[0];
+features = model_pieces[1];
+
+if (cpu_x86_find_cpudef(name, &def, &error) < 0) {
+goto error;
+}
+
+if (cpu_x86_parse_featurestr(&def, features, &error) < 0) {
 goto error;
 }
 
@@ -1546,8 +1541,11 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 if (error) {
 goto error;
 }
+
+g_strfreev(model_pieces);
 return cpu;
 error:
+g_strfreev(model_pieces);
 object_delete(OBJECT(cpu));
 if (error) {
 error_report("cpu_x86_init: %s", error_get_pretty(error));
-- 
1.7.11.7




[Qemu-devel] [PATCH 17/17] target-i386: cpu_x86_init(): reorder split of CPU string and creation of CPU object

2012-11-12 Thread Eduardo Habkost
A step towards making the creation of CPU objects use the CPU model name
as class name.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e47ec5d..5f2ce7d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1505,19 +1505,13 @@ static int cpudef_2_x86_cpu(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 
 X86CPU *cpu_x86_init(const char *cpu_string)
 {
-X86CPU *cpu;
+X86CPU *cpu = NULL;
 CPUX86State *env;
 Error *error = NULL;
 X86CPUDefinition def;
 char *name, *features;
 gchar **model_pieces;
 
-cpu = X86_CPU(object_new(TYPE_X86_CPU));
-env = &cpu->env;
-env->cpu_model_str = cpu_string;
-
-memset(&def, 0, sizeof(def));
-
 model_pieces = g_strsplit(cpu_string, ",", 2);
 if (!model_pieces[0]) {
 goto error;
@@ -1525,6 +1519,12 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 name = model_pieces[0];
 features = model_pieces[1];
 
+cpu = X86_CPU(object_new(TYPE_X86_CPU));
+env = &cpu->env;
+env->cpu_model_str = cpu_string;
+
+memset(&def, 0, sizeof(def));
+
 if (cpu_x86_find_cpudef(name, &def, &error) < 0) {
 goto error;
 }
@@ -1546,7 +1546,9 @@ X86CPU *cpu_x86_init(const char *cpu_string)
 return cpu;
 error:
 g_strfreev(model_pieces);
-object_delete(OBJECT(cpu));
+if (cpu) {
+object_delete(OBJECT(cpu));
+}
 if (error) {
 error_report("cpu_x86_init: %s", error_get_pretty(error));
 error_free(error);
-- 
1.7.11.7




[Qemu-devel] [PATCH 06/17] target-i386: cpu_x86_init(): print error message in case of error

2012-11-12 Thread Eduardo Habkost
Error information is being ignored and never returned to the caller.

While we don't change cpu_x86_init() to not return error information, print
error message inside cpu_x86_init() in case of error.

Signed-off-by: Eduardo Habkost 
---
Changes v2:
 - Use error_report()
 - Add "cpu_x86_init:" prefix to error message
---
 target-i386/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 69f1204..2907fb0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1517,6 +1517,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 error:
 object_delete(OBJECT(cpu));
 if (error) {
+error_report("cpu_x86_init: %s", error_get_pretty(error));
 error_free(error);
 }
 return NULL;
-- 
1.7.11.7




[Qemu-devel] [PATCH 09/17] target-i386: move out CPU features initialization to separate func

2012-11-12 Thread Eduardo Habkost
From: Igor Mammedov 

Later it will be used in cpu_x86_init() to init CPU from found cpudef.

This is will make it easier to reorder and clean up the cpu_x86_init()
code later.

Signed-off-by: Igor Mammedov 
[ehabkost: added error reporting to function]
Signed-off-by: Eduardo Habkost 
---
v2:
   - rebased on top of  "i386: cpu: remove duplicate feature names"
  http://www.mail-archive.com/qemu-devel@nongnu.org/msg129458.html
v3 (ehabkost):
 - Rebased on top of CPU model classes work in progress
 - Added error reporting to new function
 - Changed commit message
---
 target-i386/cpu.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 7f4e8f0..cef120e 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1424,16 +1424,11 @@ static void filter_features_for_kvm(X86CPU *cpu)
 }
 #endif
 
-static int cpu_x86_register(X86CPU *cpu, const char *cpu_model, Error **errp)
+static int cpudef_2_x86_cpu(X86CPU *cpu, X86CPUDefinition *def, Error **errp)
 {
 CPUX86State *env = &cpu->env;
-X86CPUDefinition def1, *def = &def1;
 Error *error = NULL;
 
-memset(def, 0, sizeof(*def));
-
-if (cpu_x86_find_by_name(def, cpu_model) < 0)
-return -1;
 if (def->vendor1) {
 env->cpuid_vendor1 = def->vendor1;
 env->cpuid_vendor2 = def->vendor2;
@@ -1494,6 +1489,19 @@ static int cpu_x86_register(X86CPU *cpu, const char 
*cpu_model, Error **errp)
 return 0;
 }
 
+static int cpu_x86_register(X86CPU *cpu, const char *cpu_model, Error **errp)
+{
+X86CPUDefinition def1, *def = &def1;
+
+memset(def, 0, sizeof(*def));
+
+if (cpu_x86_find_by_name(def, cpu_model) < 0)
+return -1;
+if (cpudef_2_x86_cpu(cpu, def, errp) < 0)
+return -1;
+return 0;
+}
+
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
 X86CPU *cpu;
-- 
1.7.11.7




[Qemu-devel] [PATCH 08/17] target-i386: cpu_x86_register(): reorder CPU property setting

2012-11-12 Thread Eduardo Habkost
Trivial code movement, before moving the code to another function.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 9334e0c..7f4e8f0 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1448,18 +1448,19 @@ static int cpu_x86_register(X86CPU *cpu, const char 
*cpu_model, Error **errp)
 object_property_set_int(OBJECT(cpu), def->family, "family", &error);
 object_property_set_int(OBJECT(cpu), def->model, "model", &error);
 object_property_set_int(OBJECT(cpu), def->stepping, "stepping", &error);
+object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
+object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
+"tsc-frequency", &error);
+object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
 env->cpuid_features = def->features;
 env->cpuid_ext_features = def->ext_features;
 env->cpuid_ext2_features = def->ext2_features;
 env->cpuid_ext3_features = def->ext3_features;
-object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", &error);
 env->cpuid_kvm_features = def->kvm_features;
 env->cpuid_svm_features = def->svm_features;
 env->cpuid_ext4_features = def->ext4_features;
 env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
 env->cpuid_xlevel2 = def->xlevel2;
-object_property_set_int(OBJECT(cpu), (int64_t)def->tsc_khz * 1000,
-"tsc-frequency", &error);
 
 /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
  * CPUID[1].EDX.
@@ -1486,7 +1487,6 @@ static int cpu_x86_register(X86CPU *cpu, const char 
*cpu_model, Error **errp)
 filter_features_for_kvm(cpu);
 #endif
 }
-object_property_set_str(OBJECT(cpu), def->model_id, "model-id", &error);
 if (error) {
 error_propagate(errp, error);
 return -1;
-- 
1.7.11.7




[Qemu-devel] [PATCH 10/17] target-i386: kill cpu_x86_register()

2012-11-12 Thread Eduardo Habkost
Move the cpu_x86_register() code inside cpu_x86_init(), as the initialization
steps are going to be reordered.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index cef120e..3f04054 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1489,30 +1489,24 @@ static int cpudef_2_x86_cpu(X86CPU *cpu, 
X86CPUDefinition *def, Error **errp)
 return 0;
 }
 
-static int cpu_x86_register(X86CPU *cpu, const char *cpu_model, Error **errp)
-{
-X86CPUDefinition def1, *def = &def1;
-
-memset(def, 0, sizeof(*def));
-
-if (cpu_x86_find_by_name(def, cpu_model) < 0)
-return -1;
-if (cpudef_2_x86_cpu(cpu, def, errp) < 0)
-return -1;
-return 0;
-}
-
 X86CPU *cpu_x86_init(const char *cpu_model)
 {
 X86CPU *cpu;
 CPUX86State *env;
 Error *error = NULL;
+X86CPUDefinition def1, *def = &def1;
 
 cpu = X86_CPU(object_new(TYPE_X86_CPU));
 env = &cpu->env;
 env->cpu_model_str = cpu_model;
 
-if (cpu_x86_register(cpu, cpu_model, &error) < 0) {
+memset(def, 0, sizeof(*def));
+
+if (cpu_x86_find_by_name(def, cpu_model) < 0) {
+goto error;
+}
+
+if (cpudef_2_x86_cpu(cpu, def, &error) < 0) {
 goto error;
 }
 
-- 
1.7.11.7




[Qemu-devel] [PATCH 05/17] target-i386: cpu_x86_init(): move error handling to end of function

2012-11-12 Thread Eduardo Habkost
Doing error handling on a single place will make it easier to make sure
memory is freed, and that error information is properly printed or
returned to the caller.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 73b0fa1..69f1204 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1506,17 +1506,20 @@ X86CPU *cpu_x86_init(const char *cpu_model)
 env->cpu_model_str = cpu_model;
 
 if (cpu_x86_register(cpu, cpu_model) < 0) {
-object_delete(OBJECT(cpu));
-return NULL;
+goto error;
 }
 
 x86_cpu_realize(OBJECT(cpu), &error);
 if (error) {
-error_free(error);
-object_delete(OBJECT(cpu));
-return NULL;
+goto error;
 }
 return cpu;
+error:
+object_delete(OBJECT(cpu));
+if (error) {
+error_free(error);
+}
+return NULL;
 }
 
 #if !defined(CONFIG_USER_ONLY)
-- 
1.7.11.7




[Qemu-devel] [PATCH 03/17] target-i386: cpu: rename x86_def_t to X86CPUDefinition

2012-11-12 Thread Eduardo Habkost
Change to match QEMU coding style.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b50ca8c..2dfcc9c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -271,8 +271,8 @@ static void add_flagname_to_bitmaps(const char *flagname, 
uint32_t *features,
 fprintf(stderr, "CPU feature %s not found\n", flagname);
 }
 
-typedef struct x86_def_t {
-struct x86_def_t *next;
+typedef struct X86CPUDefinition {
+struct X86CPUDefinition *next;
 const char *name;
 uint32_t level;
 uint32_t vendor1, vendor2, vendor3;
@@ -290,7 +290,7 @@ typedef struct x86_def_t {
 uint32_t xlevel2;
 /* The feature bits on CPUID[EAX=7,ECX=0].EBX */
 uint32_t cpuid_7_0_ebx_features;
-} x86_def_t;
+} X86CPUDefinition;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
@@ -330,13 +330,14 @@ typedef struct x86_def_t {
 #define TCG_SVM_FEATURES 0
 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
 
+
 /* maintains list of cpu model definitions
  */
-static x86_def_t *x86_defs = {NULL};
+static X86CPUDefinition *x86_defs = {NULL};
 
 /* built-in cpu model definitions (deprecated)
  */
-static x86_def_t builtin_x86_defs[] = {
+static X86CPUDefinition builtin_x86_defs[] = {
 {
 .name = "qemu64",
 .level = 4,
@@ -775,12 +776,12 @@ static int cpu_x86_fill_model_id(char *str)
 }
 #endif
 
-/* Fill a x86_def_t struct with information about the host CPU, and
+/* Fill a X86CPUDefinition struct with information about the host CPU, and
  * the CPU features supported by the host hardware + host kernel
  *
  * This function may be called only if KVM is enabled.
  */
-static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
+static void kvm_cpu_fill_host(X86CPUDefinition *x86_cpu_def)
 {
 #ifdef CONFIG_KVM
 KVMState *s = kvm_state;
@@ -788,7 +789,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
 
 assert(kvm_enabled());
 
-x86_cpu_def->name = "host";
 host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx);
 x86_cpu_def->vendor1 = ebx;
 x86_cpu_def->vendor2 = edx;
@@ -865,9 +865,9 @@ static int unavailable_host_feature(struct model_features_t 
*f, uint32_t mask)
  *
  * This function may be called only if KVM is enabled.
  */
-static int kvm_check_features_against_host(x86_def_t *guest_def)
+static int kvm_check_features_against_host(X86CPUDefinition *guest_def)
 {
-x86_def_t host_def;
+X86CPUDefinition host_def;
 uint32_t mask;
 int rv, i;
 struct model_features_t ft[] = {
@@ -1146,10 +1146,11 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor 
*v, void *opaque,
 cpu->env.tsc_khz = value / 1000;
 }
 
-static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
+static int cpu_x86_find_by_name(X86CPUDefinition *x86_cpu_def,
+const char *cpu_model)
 {
 unsigned int i;
-x86_def_t *def;
+X86CPUDefinition *def;
 
 char *s = g_strdup(cpu_model);
 char *featurestr, *name = strtok(s, ",");
@@ -1355,7 +1356,7 @@ static void listflags(char *buf, int bufsize, uint32_t 
fbits,
 /* generate CPU information. */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-x86_def_t *def;
+X86CPUDefinition *def;
 char buf[256];
 
 for (def = x86_defs; def; def = def->next) {
@@ -1379,7 +1380,7 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
 CpuDefinitionInfoList *cpu_list = NULL;
-x86_def_t *def;
+X86CPUDefinition *def;
 
 for (def = x86_defs; def; def = def->next) {
 CpuDefinitionInfoList *entry;
@@ -1426,7 +1427,7 @@ static void filter_features_for_kvm(X86CPU *cpu)
 static int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 {
 CPUX86State *env = &cpu->env;
-x86_def_t def1, *def = &def1;
+X86CPUDefinition def1, *def = &def1;
 Error *error = NULL;
 
 memset(def, 0, sizeof(*def));
@@ -1535,7 +1536,7 @@ void x86_cpudef_setup(void)
 static const char *model_with_versions[] = { "qemu32", "qemu64", "athlon" 
};
 
 for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); ++i) {
-x86_def_t *def = &builtin_x86_defs[i];
+X86CPUDefinition *def = &builtin_x86_defs[i];
 def->next = x86_defs;
 
 /* Look for specific "cpudef" models that */
-- 
1.7.11.7




Re: [Qemu-devel] DOS boot problem with LSI 53C895A SCSI controller and LSI option ROM

2012-11-12 Thread Gerhard Wiesinger

On 12.11.2012 09:26, Paolo Bonzini wrote:

Il 10/11/2012 22:39, Gerhard Wiesinger ha scritto:

Hello,

I bisected down a DOS boot problem with LSI 53C895A SCSI controller and
LSI option ROM to the following commit:
e93176d55f1eb4be1a366b51afeaf4f4c8c31d75

The emulation is known to be incomplete; the option ROM is not really
supported, just like the support for the LSI controller in SeaBIOS is
not meant for real hardware.



The option ROM worked perfect for legacy before this commit for years.


But if this is a regression, I can look at it.  Problem is, I don't have
the option ROM and I don't think I can obtain one legally.  Please
provide at least a trace of the SCSI commands that are sent.


Yes, it is a regression problem.

You can download the option ROM from the LSI homepage:
http://www.lsi.com/support/Pages/Download-Results.aspx?productcode=P00536&assettype=0&component=Storage%20Component&productfamily=0&productname=LSI53C895A
http://www.lsi.com/downloads/Public/Host%20Bus%20Adapters/Host%20Bus%20Adapters%20Common%20Files/lsi_bios.zip
http://www.lsi.com/downloads/Public/SCSI%20HBAs/SCSI%20HBAs%20Common%20Files/lsi_bios.zip
http://www.lsi.com/downloads/Public/SCSI%20ICs%20and%20Expanders/SCSI%20ICs%20and%20Expanders%20Common%20Files/lsi_bios.zip
http://www.lsi.com/downloads/Public/Obsolete/Obsolete%20Common%20Files/lsi_bios.zip

Trace will follow (currently very busy). Best solution to turn it on?


BTW: Nearly all KVM coredumps aren't valid anymore and have only a
garbage stack trace. Any ideas?

It seems strange that this would be limited to KVM.  What about other
programs?  Or try --disable-pie.



Looks like a general problem on the FC17 machine with latest updates. 
Was ok 11 days ago on FC17. program below cores, but also no valid core 
dump. Also further information below.


Doesn't look like PIE is active.

Any further hints?

Ciao,
Gerhard

coredump.c:
int main(int argc, char* argv[])
{
  char* p = 0;
  *p = 0;
  return 0;
}

gcc -g coredump.c -o a.out
eu-readelf -h a.out
ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00
  Class: ELF64
  Data:  2's complement, little endian
  Ident Version: 1 (current)
  OS/ABI:UNIX - System V
  ABI Version:   0
  Type:  EXEC (Executable file)
  Machine:   AMD x86-64
  Version:   1 (current)
  Entry point address:   0x400390
  Start of program headers:  64 (bytes into file)
  Start of section headers:  3032 (bytes into file)
  Flags:
  Size of this header:   64 (bytes)
  Size of program header entries:56 (bytes)
  Number of program headers entries: 8
  Size of section header entries:64 (bytes)
  Number of section headers entries: 35
  Section header string table index: 32

gdb -c core.5671
GNU gdb (GDB) Fedora (7.4.50.20120120-52.fc17)
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 


This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
For bug reporting instructions, please see:
.
Missing separate debuginfo for the main executable file
Try: yum --disablerepo='*' --enablerepo='*-debug*' install 
/usr/lib/debug/.build-id/ff/36de4d6ecfe0c5b80cbc805916d9acc829619e

[New LWP 5671]
Core was generated by `./a.out'.
Program terminated with signal 11, Segmentation fault.
#0  0x004004b3 in ?? ()
(gdb) ba
#0  0x004004b3 in ?? ()
#1  0x in ?? ()





[Qemu-devel] [PATCHv3 5/5] seccomp: adding debug mode

2012-11-12 Thread Eduardo Otubo
This patch is meant for developer debug purposes only.  It adds
support that displays the offending system call number if QEMU
is being killed by seccomp.  The offending system call may need
to be added to the appropriate system call white list in
qemu-seccomp.c to prevent seccomp from killing QEMU.

When the seccomp filter is configured with SCMP_ACT_TRAP, the
kernel sends a SIGSYS every time an illegal syscall is called.
The role of the debug mode is to handle the SIGSYS, determine
the illegal syscall, and print the syscall number to stderr.

v3: New in v3.

Signed-off-by: Eduardo Otubo 
Signed-off-by: Corey Bryant 
---
 Makefile.objs|3 ++
 configure|   16 +
 qemu-seccomp-debug.c |   93 ++
 qemu-seccomp-debug.h |   40 ++
 qemu-seccomp.c   |4 +--
 qemu-seccomp.h   |   13 +++
 qemu-thread-posix.c  |3 ++
 vl.c |   10 ++
 8 files changed, 180 insertions(+), 2 deletions(-)
 create mode 100644 qemu-seccomp-debug.c
 create mode 100644 qemu-seccomp-debug.h

diff --git a/Makefile.objs b/Makefile.objs
index 682b1e6..4ece4d8 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -104,6 +104,9 @@ common-obj-$(CONFIG_SLIRP) += slirp/
 ##
 # libseccomp
 common-obj-y += qemu-seccomp.o
+ifeq ($(CONFIG_SECCOMP_DEBUG),y)
+common-obj-y += qemu-seccomp-debug.o
+endif
 
 ##
 # libuser
diff --git a/configure b/configure
index d28f8d5..e2417fe 100755
--- a/configure
+++ b/configure
@@ -222,6 +222,7 @@ want_tools="yes"
 libiscsi=""
 coroutine=""
 seccomp="yes"
+seccomp_debug="no"
 glusterfs=""
 
 # parse CC options first
@@ -867,6 +868,10 @@ for opt do
   ;;
   --disable-seccomp) seccomp="no"
   ;;
+  --enable-seccomp-debug) seccomp_debug="yes"
+  ;;
+  --disable-seccomp-debug) seccomp_debug="no"
+  ;;
   --disable-glusterfs) glusterfs="no"
   ;;
   --enable-glusterfs) glusterfs="yes"
@@ -876,6 +881,10 @@ for opt do
   esac
 done
 
+if test "$seccomp" = "no"; then
+seccomp_debug="no";
+fi
+
 case "$cpu" in
 sparc)
LDFLAGS="-m32 $LDFLAGS"
@@ -1115,6 +1124,8 @@ echo "  --disable-guest-agentdisable building of the 
QEMU Guest Agent"
 echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
 echo "  --disable-seccompdisable seccomp support"
 echo "  --enable-seccomp enables seccomp support"
+echo "  --disable-seccomp-debug  disable seccomp debug support"
+echo "  --enable-seccomp-debug   enables seccomp debug support"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "   gthread, ucontext, sigaltstack, windows"
 echo "  --enable-glusterfs   enable GlusterFS backend"
@@ -3230,6 +3241,7 @@ echo "OpenGL support$opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "seccomp support   $seccomp"
+echo "seccomp debug $seccomp_debug"
 echo "coroutine backend $coroutine_backend"
 echo "GlusterFS support $glusterfs"
 
@@ -3534,6 +3546,10 @@ if test "$seccomp" = "yes"; then
   echo "CONFIG_SECCOMP=y" >> $config_host_mak
 fi
 
+if test "$seccomp_debug" = "yes"; then
+  echo "CONFIG_SECCOMP_DEBUG=y" >> $config_host_mak
+fi
+
 # XXX: suppress that
 if [ "$bsd" = "yes" ] ; then
   echo "CONFIG_BSD=y" >> $config_host_mak
diff --git a/qemu-seccomp-debug.c b/qemu-seccomp-debug.c
new file mode 100644
index 000..4b64e8c
--- /dev/null
+++ b/qemu-seccomp-debug.c
@@ -0,0 +1,93 @@
+/*
+ * QEMU seccomp mode 2 support with libseccomp
+ * Debug system calls helper functions
+ *
+ * Copyright IBM, Corp. 2012
+ *
+ * Authors:
+ *  Eduardo Otubo
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu-seccomp-debug.h"
+#include 
+
+#define safe_warn(data, len) write(STDERR_FILENO, (const void *) data, len)
+
+static int count_digits(int number)
+{
+int digits = 0;
+while (number) {
+number /= 10;
+digits++;
+}
+
+return digits;
+}
+
+static char *sput_i(int integer, char *string)
+{
+if (integer / 10 != 0) {
+string = sput_i(integer / 10, string);
+}
+*string++ = (char) ('0' + integer % 10);
+return string;
+}
+
+static void int_to_asc(int integer, char *string)
+{
+*sput_i(integer, string) = '\n';
+}
+
+static void syscall_debug(int nr, siginfo_t *info, void *void_context)
+{
+ucontext_t *ctx = (ucontext_t *) (void_context);
+char errormsg[] = "seccomp: illegal syscall trapped: ";
+char syscall_char[count_digits(__NR_syscalls) + 1];
+int syscall_num = 0;
+int i;
+
+for (i = 0; i < count_digits(__NR_syscalls) + 1; i++) {
+   

[Qemu-devel] [PATCHv3 1/5] seccomp: adding new syscalls (bugzilla 855162)

2012-11-12 Thread Eduardo Otubo
According to the bug 855162[0] - there's the need of adding new syscalls
to the whitelist when using Qemu with Libvirt.

[0] - https://bugzilla.redhat.com/show_bug.cgi?id=855162

v2: Adding new syscalls to the list: readlink, rt_sigpending, and
rt_sigtimedwait

v3:
 * Added new syscalls based on further tests.
 * Added new syscalls with priority 241 that are unknown to be
   used by QEMU.  We'll attempt to remove these after QEMU 1.3.

Reported-by: Paul Moore 
Signed-off-by: Eduardo Otubo 
Signed-off-by: Corey Bryant 
---
 qemu-seccomp.c |  148 +---
 1 file changed, 131 insertions(+), 17 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 64329a3..b06a2c6 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -26,8 +26,12 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = 
{
 { SCMP_SYS(timer_gettime), 254 },
 { SCMP_SYS(futex), 253 },
 { SCMP_SYS(select), 252 },
+#if defined(__x86_64__)
 { SCMP_SYS(recvfrom), 251 },
 { SCMP_SYS(sendto), 250 },
+#elif defined(__i386__)
+{ SCMP_SYS(socketcall), 250 },
+#endif
 { SCMP_SYS(read), 249 },
 { SCMP_SYS(brk), 248 },
 { SCMP_SYS(clone), 247 },
@@ -36,15 +40,30 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(execve), 245 },
 { SCMP_SYS(open), 245 },
 { SCMP_SYS(ioctl), 245 },
+#if defined(__x86_64__)
+{ SCMP_SYS(socket), 245 },
+{ SCMP_SYS(setsockopt), 245 },
 { SCMP_SYS(recvmsg), 245 },
 { SCMP_SYS(sendmsg), 245 },
 { SCMP_SYS(accept), 245 },
 { SCMP_SYS(connect), 245 },
+{ SCMP_SYS(socketpair), 245 },
+{ SCMP_SYS(bind), 245 },
+{ SCMP_SYS(listen), 245 },
+{ SCMP_SYS(semget), 245 },
+#elif defined(__i386__)
+{ SCMP_SYS(ipc), 245 },
+#endif
 { SCMP_SYS(gettimeofday), 245 },
 { SCMP_SYS(readlink), 245 },
 { SCMP_SYS(access), 245 },
 { SCMP_SYS(prctl), 245 },
 { SCMP_SYS(signalfd), 245 },
+{ SCMP_SYS(getrlimit), 245 },
+{ SCMP_SYS(set_tid_address), 245 },
+{ SCMP_SYS(statfs), 245 },
+{ SCMP_SYS(unlink), 245 },
+{ SCMP_SYS(wait4), 245 },
 #if defined(__i386__)
 { SCMP_SYS(fcntl64), 245 },
 { SCMP_SYS(fstat64), 245 },
@@ -56,24 +75,26 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] 
= {
 { SCMP_SYS(sigreturn), 245 },
 { SCMP_SYS(_newselect), 245 },
 { SCMP_SYS(_llseek), 245 },
-{ SCMP_SYS(mmap2), 245},
+{ SCMP_SYS(mmap2), 245 },
 { SCMP_SYS(sigprocmask), 245 },
-#elif defined(__x86_64__)
-{ SCMP_SYS(sched_getparam), 245},
-{ SCMP_SYS(sched_getscheduler), 245},
-{ SCMP_SYS(fstat), 245},
-{ SCMP_SYS(clock_getres), 245},
-{ SCMP_SYS(sched_get_priority_min), 245},
-{ SCMP_SYS(sched_get_priority_max), 245},
-{ SCMP_SYS(stat), 245},
-{ SCMP_SYS(socket), 245},
-{ SCMP_SYS(setsockopt), 245},
-{ SCMP_SYS(uname), 245},
-{ SCMP_SYS(semget), 245},
 #endif
+{ SCMP_SYS(sched_getparam), 245 },
+{ SCMP_SYS(sched_getscheduler), 245 },
+{ SCMP_SYS(fstat), 245 },
+{ SCMP_SYS(clock_getres), 245 },
+{ SCMP_SYS(sched_get_priority_min), 245 },
+{ SCMP_SYS(sched_get_priority_max), 245 },
+{ SCMP_SYS(stat), 245 },
+{ SCMP_SYS(uname), 245 },
 { SCMP_SYS(eventfd2), 245 },
 { SCMP_SYS(dup), 245 },
+{ SCMP_SYS(dup2), 245 },
+{ SCMP_SYS(dup3), 245 },
 { SCMP_SYS(gettid), 245 },
+{ SCMP_SYS(getgid), 245 },
+{ SCMP_SYS(getegid), 245 },
+{ SCMP_SYS(getuid), 245 },
+{ SCMP_SYS(geteuid), 245 },
 { SCMP_SYS(timer_create), 245 },
 { SCMP_SYS(exit), 245 },
 { SCMP_SYS(clock_gettime), 245 },
@@ -93,8 +114,6 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = 
{
 { SCMP_SYS(lseek), 245 },
 { SCMP_SYS(pselect6), 245 },
 { SCMP_SYS(fork), 245 },
-{ SCMP_SYS(bind), 245 },
-{ SCMP_SYS(listen), 245 },
 { SCMP_SYS(eventfd), 245 },
 { SCMP_SYS(rt_sigprocmask), 245 },
 { SCMP_SYS(write), 244 },
@@ -104,10 +123,105 @@ static const struct QemuSeccompSyscall 
seccomp_whitelist[] = {
 { SCMP_SYS(pipe2), 242 },
 { SCMP_SYS(munmap), 242 },
 { SCMP_SYS(mremap), 242 },
+{ SCMP_SYS(fdatasync), 242 },
+{ SCMP_SYS(close), 242 },
+{ SCMP_SYS(rt_sigpending), 242 },
+{ SCMP_SYS(rt_sigtimedwait), 242 },
+{ SCMP_SYS(readv), 242 },
+{ SCMP_SYS(writev), 242 },
+{ SCMP_SYS(preadv), 242 },
+{ SCMP_SYS(pwritev), 242 },
+{ SCMP_SYS(setrlimit), 242 },
+{ SCMP_SYS(ftruncate), 242 },
+{ SCMP_SYS(lstat), 242 },
+{ SCMP_SYS(pipe), 242 },
+{ SCMP_SYS(umask), 242 },
+{ SCMP_SYS(chdir), 242 },
+{ SCMP_SYS(setitimer), 242 },
+{ SCMP_SYS(setsid), 242 },
+{ SCMP_SYS(poll), 242 },
+#if defined(__i386__)
+{ SCMP_SYS(waitpid), 242 },
+#elif defined(__x86_64__)
 { SCMP_SYS(getsockname), 242 },
 { SCMP_SYS(getpeername), 242 },
-{ SCMP_SYS(fdatasync), 242 },
-{ SCMP_SYS(close), 242 }
+{ SCMP_SYS(accept4), 

[Qemu-devel] [PATCH] vga: fix bochs alignment issue

2012-11-12 Thread Gerd Hoffmann
The bochs dispi interface traditionally uses port 0x1ce as 16bit index
register and port 0x1cf as 16bit data register.  The later is unaligned,
and probably for that reason the the data register was moved to 0x1d0
for non-x86 archs.

This patch makes the data register available at 0x1d0 on x86 too.  The
old x86 location is kept for compatibility reasons, so both 0x1cf and
0x1d0 can be used as data register on x86.

Signed-off-by: Gerd Hoffmann 
---
 hw/vga.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 81aa76b..2b0200a 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -2321,9 +2321,8 @@ static const MemoryRegionPortio vbe_portio_list[] = {
 { 0, 1, 2, .read = vbe_ioport_read_index, .write = vbe_ioport_write_index 
},
 # ifdef TARGET_I386
 { 1, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
-# else
-{ 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
 # endif
+{ 2, 1, 2, .read = vbe_ioport_read_data, .write = vbe_ioport_write_data },
 PORTIO_END_OF_LIST(),
 };
 
-- 
1.7.1




[Qemu-devel] [PATCHv3 4/5] seccomp: double whitelist support

2012-11-12 Thread Eduardo Otubo
This patch includes a second whitelist right before the main loop. The
second whitelist is more restricted and does not contain execve().
Although it works fine the way it is now, it's optimal to update and
fine tune it.

v2: * ctx changed to main_loop_ctx
* seccomp_on now inside ifdef
* open() syscall added to the main_loop whitelist

v3: * Main loop now has whitelist without execve().
* Use enum seccomp_states, seccomp_get_state(), and
  seccomp_install_filter()
* Added new syscalls with priority 241 that are unknown to be
  used by QEMU.  We'll attempt to remove these after QEMU 1.3.

Signed-off-by: Eduardo Otubo 
Signed-off-by: Corey Bryant 
---
 qemu-seccomp.c |  252 +---
 qemu-seccomp.h |2 +-
 vl.c   |9 +-
 3 files changed, 251 insertions(+), 12 deletions(-)

diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 2386996..d5a3b0f 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -13,9 +13,9 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "config-host.h"
-#include 
 #include "osdep.h"
 #include "qemu-seccomp.h"
+#include 
 
 #ifdef CONFIG_SECCOMP
 int seccomp_state = SECCOMP_ON;
@@ -39,7 +39,7 @@ struct QemuSeccompSyscall {
 uint8_t priority;
 };
 
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall whitelist_init[] = {
 { SCMP_SYS(timer_settime), 255 },
 { SCMP_SYS(timer_gettime), 254 },
 { SCMP_SYS(futex), 253 },
@@ -241,14 +241,238 @@ static const struct QemuSeccompSyscall 
seccomp_whitelist[] = {
 { SCMP_SYS(prlimit64), 241 },
 { SCMP_SYS(waitid), 241 }
 };
+
+static const struct QemuSeccompSyscall whitelist_main[] = {
+{ SCMP_SYS(timer_settime), 255 },
+{ SCMP_SYS(timer_gettime), 254 },
+{ SCMP_SYS(futex), 253 },
+{ SCMP_SYS(select), 252 },
+#if defined(__x86_64__)
+{ SCMP_SYS(recvfrom), 251 },
+{ SCMP_SYS(sendto), 250 },
+#elif defined(__i386__)
+{ SCMP_SYS(socketcall), 250 },
+#endif
+{ SCMP_SYS(read), 249 },
+{ SCMP_SYS(brk), 248 },
+{ SCMP_SYS(clone), 247 },
+{ SCMP_SYS(mmap), 247 },
+{ SCMP_SYS(mprotect), 246 },
+{ SCMP_SYS(open), 245 },
+{ SCMP_SYS(ioctl), 245 },
+#if defined(__x86_64__)
+{ SCMP_SYS(socket), 245 },
+{ SCMP_SYS(setsockopt), 245 },
+{ SCMP_SYS(recvmsg), 245 },
+{ SCMP_SYS(sendmsg), 245 },
+{ SCMP_SYS(accept), 245 },
+{ SCMP_SYS(connect), 245 },
+{ SCMP_SYS(socketpair), 245 },
+{ SCMP_SYS(bind), 245 },
+{ SCMP_SYS(listen), 245 },
+{ SCMP_SYS(semget), 245 },
+#elif defined(__i386__)
+{ SCMP_SYS(ipc), 245 },
+#endif
+{ SCMP_SYS(gettimeofday), 245 },
+{ SCMP_SYS(readlink), 245 },
+{ SCMP_SYS(access), 245 },
+{ SCMP_SYS(prctl), 245 },
+{ SCMP_SYS(signalfd), 245 },
+{ SCMP_SYS(getrlimit), 245 },
+{ SCMP_SYS(set_tid_address), 245 },
+{ SCMP_SYS(statfs), 245 },
+{ SCMP_SYS(unlink), 245 },
+{ SCMP_SYS(wait4), 245 },
+#if defined(__i386__)
+{ SCMP_SYS(fcntl64), 245 },
+{ SCMP_SYS(fstat64), 245 },
+{ SCMP_SYS(stat64), 245 },
+{ SCMP_SYS(getgid32), 245 },
+{ SCMP_SYS(getegid32), 245 },
+{ SCMP_SYS(getuid32), 245 },
+{ SCMP_SYS(geteuid32), 245 },
+{ SCMP_SYS(sigreturn), 245 },
+{ SCMP_SYS(_newselect), 245 },
+{ SCMP_SYS(_llseek), 245 },
+{ SCMP_SYS(mmap2), 245 },
+{ SCMP_SYS(sigprocmask), 245 },
+#endif
+{ SCMP_SYS(sched_getparam), 245 },
+{ SCMP_SYS(sched_getscheduler), 245 },
+{ SCMP_SYS(fstat), 245 },
+{ SCMP_SYS(clock_getres), 245 },
+{ SCMP_SYS(sched_get_priority_min), 245 },
+{ SCMP_SYS(sched_get_priority_max), 245 },
+{ SCMP_SYS(stat), 245 },
+{ SCMP_SYS(uname), 245 },
+{ SCMP_SYS(eventfd2), 245 },
+{ SCMP_SYS(dup), 245 },
+{ SCMP_SYS(dup2), 245 },
+{ SCMP_SYS(dup3), 245 },
+{ SCMP_SYS(gettid), 245 },
+{ SCMP_SYS(getgid), 245 },
+{ SCMP_SYS(getegid), 245 },
+{ SCMP_SYS(getuid), 245 },
+{ SCMP_SYS(geteuid), 245 },
+{ SCMP_SYS(timer_create), 245 },
+{ SCMP_SYS(exit), 245 },
+{ SCMP_SYS(clock_gettime), 245 },
+{ SCMP_SYS(time), 245 },
+{ SCMP_SYS(restart_syscall), 245 },
+{ SCMP_SYS(pwrite64), 245 },
+{ SCMP_SYS(chown), 245 },
+{ SCMP_SYS(openat), 245 },
+{ SCMP_SYS(getdents), 245 },
+{ SCMP_SYS(timer_delete), 245 },
+{ SCMP_SYS(exit_group), 245 },
+{ SCMP_SYS(rt_sigreturn), 245 },
+{ SCMP_SYS(sync), 245 },
+{ SCMP_SYS(pread64), 245 },
+{ SCMP_SYS(madvise), 245 },
+{ SCMP_SYS(set_robust_list), 245 },
+{ SCMP_SYS(lseek), 245 },
+{ SCMP_SYS(pselect6), 245 },
+{ SCMP_SYS(fork), 245 },
+{ SCMP_SYS(eventfd), 245 },
+{ SCMP_SYS(rt_sigprocmask), 245 },
+{ SCMP_SYS(write), 244 },
+{ SCMP_SYS(fcntl), 243 },
+{ SCMP_SYS(tgkill), 242 },
+{ SCMP_SYS(rt_sigaction), 242 },
+{ SCMP_SYS(pipe2), 242 },
+{ SCMP_SYS(munmap), 242 },
+{ SCMP_SYS(mrem

[Qemu-devel] [PATCHv3 2/5] seccomp: setting "-sandbox on" as deafult

2012-11-12 Thread Eduardo Otubo
Now the seccomp filter will be set to "on" even if no argument
"-sandbox" is given.

v3: * Introduced seccomp_states enum and new functions named
  seccomp_set_state() and seccomp_get_state()
 (pbonz...@redhat.com).
* Merged seccomp_start() and install_seccomp_filter(),
  moved install_seccomp_filter() to qemu-seccomp.c,
  and renamed it.
* Moved CONFIG_SECCOMP pre-processor checks from Makefile.objs
  to qemu-seccomp.c.
* Replace qerror_report with fprintf(stderr, "..") in main()
  (lcapitul...@redhat.com).

Note: This support requires libseccomp.  If you don't have access
to libseccomp packages, you can manually build with the following
steps:

  1) git clone git://git.code.sf.net/p/libseccomp/libseccomp
  2) cd libseccomp
  3) ./configure
  4) make
  5) make install
  6) export PKG_CONFIG_PATH="/usr/local/lib/pkgconfig/"

Signed-off-by: Eduardo Otubo 
Signed-off-by: Corey Bryant 
---
 Makefile.objs  |2 --
 configure  |2 +-
 qemu-seccomp.c |   26 --
 qemu-seccomp.h |   13 +++--
 vl.c   |   31 ---
 5 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 593a592..682b1e6 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -103,9 +103,7 @@ common-obj-$(CONFIG_SLIRP) += slirp/
 
 ##
 # libseccomp
-ifeq ($(CONFIG_SECCOMP),y)
 common-obj-y += qemu-seccomp.o
-endif
 
 ##
 # libuser
diff --git a/configure b/configure
index 7290f50..d28f8d5 100755
--- a/configure
+++ b/configure
@@ -221,7 +221,7 @@ guest_agent="yes"
 want_tools="yes"
 libiscsi=""
 coroutine=""
-seccomp=""
+seccomp="yes"
 glusterfs=""
 
 # parse CC options first
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index b06a2c6..2386996 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -12,10 +12,28 @@
  * Contributions after 2012-01-13 are licensed under the terms of the
  * GNU GPL, version 2 or (at your option) any later version.
  */
+#include "config-host.h"
 #include 
-#include 
+#include "osdep.h"
 #include "qemu-seccomp.h"
 
+#ifdef CONFIG_SECCOMP
+int seccomp_state = SECCOMP_ON;
+#else
+int seccomp_state = SECCOMP_OFF;
+#endif
+
+void seccomp_set_state(int state)
+{
+seccomp_state = state;
+}
+
+int seccomp_get_state(void)
+{
+return seccomp_state;
+}
+
+#ifdef CONFIG_SECCOMP
 struct QemuSeccompSyscall {
 int32_t num;
 uint8_t priority;
@@ -223,15 +241,18 @@ static const struct QemuSeccompSyscall 
seccomp_whitelist[] = {
 { SCMP_SYS(prlimit64), 241 },
 { SCMP_SYS(waitid), 241 }
 };
+#endif
 
-int seccomp_start(void)
+int seccomp_install_filter(void)
 {
 int rc = 0;
+#ifdef CONFIG_SECCOMP
 unsigned int i = 0;
 scmp_filter_ctx ctx;
 
 ctx = seccomp_init(SCMP_ACT_KILL);
 if (ctx == NULL) {
+rc = -1;
 goto seccomp_return;
 }
 
@@ -251,5 +272,6 @@ int seccomp_start(void)
 
   seccomp_return:
 seccomp_release(ctx);
+#endif
 return rc;
 }
diff --git a/qemu-seccomp.h b/qemu-seccomp.h
index b2fc3f8..fa26d70 100644
--- a/qemu-seccomp.h
+++ b/qemu-seccomp.h
@@ -15,8 +15,17 @@
 #ifndef QEMU_SECCOMP_H
 #define QEMU_SECCOMP_H
 
+#ifdef CONFIG_SECCOMP
 #include 
-#include "osdep.h"
+#endif
+
+enum seccomp_states {
+SECCOMP_OFF,
+SECCOMP_ON
+};
+
+void seccomp_set_state(int);
+int seccomp_get_state(void);
+int seccomp_install_filter(void);
 
-int seccomp_start(void);
 #endif
diff --git a/vl.c b/vl.c
index 4f03a72..cb3d85e 100644
--- a/vl.c
+++ b/vl.c
@@ -64,9 +64,7 @@
 #include 
 #endif
 
-#ifdef CONFIG_SECCOMP
 #include "qemu-seccomp.h"
-#endif
 
 #ifdef __sun__
 #include 
@@ -772,22 +770,17 @@ static int bt_parse(const char *opt)
 
 static int parse_sandbox(QemuOpts *opts, void *opaque)
 {
-/* FIXME: change this to true for 1.3 */
-if (qemu_opt_get_bool(opts, "enable", false)) {
 #ifdef CONFIG_SECCOMP
-if (seccomp_start() < 0) {
-qerror_report(ERROR_CLASS_GENERIC_ERROR,
-  "failed to install seccomp syscall filter in the 
kernel");
-return -1;
-}
-#else
-qerror_report(ERROR_CLASS_GENERIC_ERROR,
-  "sandboxing request but seccomp is not compiled into 
this build");
-return -1;
-#endif
+/* seccomp sandboxing is on by default */
+if (!qemu_opt_get_bool(opts, "enable", true)) {
+seccomp_set_state(SECCOMP_OFF);
 }
-
 return 0;
+#else
+fprintf(stderr, "sandbox option specified but seccomp is not compiled "
+"into this build\n");
+return -1;
+#endif
 }
 
 /*QEMU USB setting**/
@@ -3489,6 +3482,14 @@ int main(int argc, char **argv, char **envp)
 exit(1);
 }
 
+if (seccomp_get_state() == SECCOMP_ON) {
+if (seccomp_install_filter() < 0) {
+fprintf(stderr, "failed to install seccomp syscall "
+   

[Qemu-devel] [PATCHv3 3/5] net: Disallow device hotplug that causes execve()

2012-11-12 Thread Eduardo Otubo
We'll soon be introducing a second whitelist that prevents
execve() right before the main_loop() is entered.  In preparation,
we need to gracefully disable use of exec'd scripts/binaries when
hotplugging network devices.  For example, the following will not
be allowed:

netdev_add tap,id=tapdev0
netdev_add bridge
host_net_add tap
host_net_add bridge

v2: * Error messages moved to the backend function, net_init_tap(),
  recommended by Paolo Bonzini
* Documentation added to QMP and HMP commands, and also to the Qemu
* options.

v3: * Prevent hotplug of network devices only when execve() would be
  called by checking seccomp_get_state(). (pbonz...@redhat.com)
* Update enum seccomp_states with new states for 2 whitelists.
* Remove #ifdef preprocesser tests where possible
  (pbonz...@redhat.com)
* Update network monitor and -sandbox command line documentation.

Signed-off-by: Eduardo Otubo 
Signed-off-by: Corey Bryant 
---
 hmp-commands.hx |   12 ++--
 net/tap.c   |   13 +
 qemu-options.hx |   11 +--
 qemu-seccomp.h  |4 +++-
 qmp-commands.hx |3 ++-
 5 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f916385..6530a21 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1103,15 +1103,15 @@ ETEXI
 {
 .name   = "host_net_add",
 .args_type  = "device:s,opts:s?",
-.params = "tap|user|socket|vde|dump [options]",
-.help   = "add host VLAN client",
+.params = "tap|bridge|user|socket|vde|dump [options]",
+.help   = "add host VLAN client (options that exec programs are 
disabled when -sandbox is in use)",
 .mhandler.cmd = net_host_device_add,
 },
 
 STEXI
 @item host_net_add
 @findex host_net_add
-Add host VLAN client.
+Add host VLAN client (options that exec programs are disabled when -sandbox is 
in use).
 ETEXI
 
 {
@@ -1131,15 +1131,15 @@ ETEXI
 {
 .name   = "netdev_add",
 .args_type  = "netdev:O",
-.params = "[user|tap|socket],id=str[,prop=value][,...]",
-.help   = "add host network device",
+.params = "[user|tap|bridge|socket],id=str[,prop=value][,...]",
+.help   = "add host network device (options that exec programs are 
disabled when -sandbox is in use)",
 .mhandler.cmd = hmp_netdev_add,
 },
 
 STEXI
 @item netdev_add
 @findex netdev_add
-Add host network device.
+Add host network device (options that exec programs are disabled when -sandbox 
is in use).
 ETEXI
 
 {
diff --git a/net/tap.c b/net/tap.c
index df89caa..b72a012 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -40,6 +40,7 @@
 #include "qemu-char.h"
 #include "qemu-common.h"
 #include "qemu-error.h"
+#include "qemu-seccomp.h"
 
 #include "net/tap-linux.h"
 
@@ -352,6 +353,12 @@ static int launch_script(const char *setup_script, const 
char *ifname, int fd)
 char *args[3];
 char **parg;
 
+if (seccomp_get_state() >= SECCOMP_MAIN_LOOP) {
+error_report("Cannot execute network script from QEMU monitor "
+ "when -sandbox is in effect");
+return -1;
+}
+
 /* try to launch network script */
 pid = fork();
 if (pid == 0) {
@@ -426,6 +433,12 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 char **parg;
 int sv[2];
 
+if (seccomp_get_state() >= SECCOMP_MAIN_LOOP) {
+error_report("Cannot execute network helper from QEMU monitor "
+ "when -sandbox is in effect");
+return -1;
+}
+
 sigemptyset(&mask);
 sigaddset(&mask, SIGCHLD);
 sigprocmask(SIG_BLOCK, &mask, &oldmask);
diff --git a/qemu-options.hx b/qemu-options.hx
index fe8f15c..f7277a0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1584,6 +1584,8 @@ attach it to the bridge. The default network helper 
executable is
 @file{/usr/local/libexec/qemu-bridge-helper} and the default bridge
 device is @file{br0}.
 
+Note that QEMU cannot execute a setuid program if -sandbox is in effect.
+
 Examples:
 
 @example
@@ -2798,8 +2800,13 @@ DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
 STEXI
 @item -sandbox
 @findex -sandbox
-Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering 
and 'off' will
-disable it.  The default is 'off'.
+Enable Seccomp mode 2 system call filter.  'on' will enable system call 
filtering
+and 'off' will disable it.  The default is 'on'.
+
+Note that when '-sandbox on' is in effect, execution of programs where 
privilege
+granting operations occur during exec will be disabled.  For example, QEMU will
+not be able to execute a setuid binary to change its uid or gid.  Additionally,
+network monitor commands that cause programs to be executed will be disabled.
 ETEXI
 
 DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/qemu-seccomp.h b/qemu-seccomp.h
index fa26d70..686db09 100644
--- a/qemu-seccomp.h
+++ b/qemu-secc

Re: [Qemu-devel] [PATCH] block: vpc initialize the uuid footer field

2012-11-12 Thread Charles Arnold
Ping?

Is this ok?

- Charles

>>> On 11/2/2012 at 09:54 AM, in message <50a0e829.5b74.009...@suse.com>, 
>>> Charles
Arnold wrote: 
> block/vpc: Initialize the uuid field in the footer with a generated uuid.
> 
> Signed-off-by: Charles Arnold 
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index b6bf52f..f14c6ae 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -26,6 +26,9 @@
>  #include "block_int.h"
>  #include "module.h"
>  #include "migration.h"
> +#if defined(CONFIG_UUID)
> +#include 
> +#endif
>  
>  /**/
>  
> @@ -739,7 +742,9 @@ static int vpc_create(const char *filename, 
> QEMUOptionParameter *options)
>  
>  footer->type = be32_to_cpu(disk_type);
>  
> -/* TODO uuid is missing */
> +#if defined(CONFIG_UUID)
> +uuid_generate(footer->uuid);
> +#endif
>  
>  footer->checksum = be32_to_cpu(vpc_checksum(buf, HEADER_SIZE));
>  





Re: [Qemu-devel] [PATCH] block: vpc support for ~2 TB disks

2012-11-12 Thread Charles Arnold
Ping?

Any thoughts on whether this is acceptable?

- Charles

>>> On 10/30/2012 at 08:59 PM, in message <50a0e561.5b74.009...@suse.com>, 
>>> Charles
Arnold wrote: 
> The VHD specification allows for up to a 2 TB disk size. The current
> implementation in qemu emulates EIDE and ATA-2 hardware which only allows
> for up to 127 GB.  This disk size limitation can be overridden by allowing
> up to 255 heads instead of the normal 4 bit limitation of 16.  Doing so
> allows disk images to be created of up to nearly 2 TB.  This change does
> not violate the VHD format specification nor does it change how smaller
> disks (ie, <=127GB) are defined. 
> 
> Signed-off-by: Charles Arnold 
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index b6bf52f..0c2eaf8 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -198,7 +198,8 @@ static int vpc_open(BlockDriverState *bs, int flags)
>  bs->total_sectors = (int64_t)
>  be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl;
>  
> -if (bs->total_sectors >= 65535 * 16 * 255) {
> +/* Allow a maximum disk size of approximately 2 TB */
> +if (bs->total_sectors >= 65535LL * 255 * 255) {qemu-devel@nongnu.org
>  err = -EFBIG;
>  goto fail;
>  }
> @@ -524,19 +525,27 @@ static coroutine_fn int vpc_co_write(BlockDriverState 
> *bs, int64_t sector_num,
>   * Note that the geometry doesn't always exactly match total_sectors but
>   * may round it down.
>   *
> - * Returns 0 on success, -EFBIG if the size is larger than 127 GB
> + * Returns 0 on success, -EFBIG if the size is larger than ~2 TB. Override
> + * the hardware EIDE and ATA-2 limit of 16 heads (max disk size of 127 GB)
> + * and instead allow up to 255 heads.
>   */
>  static int calculate_geometry(int64_t total_sectors, uint16_t* cyls,
>  uint8_t* heads, uint8_t* secs_per_cyl)
>  {
>  uint32_t cyls_times_heads;
>  
> -if (total_sectors > 65535 * 16 * 255)
> +/* Allow a maximum disk size of approximately 2 TB */
> +if (total_sectors > 65535LL * 255 * 255) {
>  return -EFBIG;
> +}
>  
>  if (total_sectors > 65535 * 16 * 63) {
>  *secs_per_cyl = 255;
> -*heads = 16;
> +if (total_sectors > 65535 * 16 * 255) {
> +*heads = 255;
> +} else {
> +*heads = 16;
> +}
>  cyls_times_heads = total_sectors / *secs_per_cyl;
>  } else {
>  *secs_per_cyl = 17;





Re: [Qemu-devel] [PATCH V19 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

2012-11-12 Thread Corey Bryant



On 11/12/2012 08:16 AM, Stefan Berger wrote:

On 11/08/2012 10:39 AM, Corey Bryant wrote:

Thanks for your responses.  I have a few comments below.

On 10/24/2012 02:46 PM, Stefan Berger wrote:

On 09/27/2012 10:22 AM, Corey Bryant wrote:



On 06/04/2012 03:37 PM, Stefan Berger wrote:



+/* check for ongoing seize by a higher locality */
+for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES;
l++) {
+if ((tis->loc[l].access &
TPM_TIS_ACCESS_SEIZE)) {
+break;


Were you intending to break from the for loop or the while?



Right. I am setting a flag here now to then leave the while loop.



Are you setting the flag or testing it?  I'm not sure this code is
serving any purpose the way it is, since it is testing the flag and
then breaking from the for loop if it's on.  That's why I was
wondering if you meant to break from the while loop instead.



Here's how the patch looks now:


+if ((val & TPM_TIS_ACCESS_SEIZE)) {
+/*
+ * allow seize if a locality is active and the requesting
+ * locality is higher than the one that's active
+ * OR
+ * allow seize for requesting locality if no locality is
+ * active
+ */
+while ((TPM_TIS_IS_VALID_LOCTY(tis->active_locty) &&
+locty > tis->active_locty) ||
+!TPM_TIS_IS_VALID_LOCTY(tis->active_locty)) {
+bool higher_seize = FALSE;
+
+/* already a pending SEIZE ? */
+if ((tis->loc[locty].access & TPM_TIS_ACCESS_SEIZE)) {
+break;
+}
+
+/* check for ongoing seize by a higher locality */
+


for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; l++) {

+if ((tis->loc[l].access & TPM_TIS_ACCESS_SEIZE)) {
+higher_seize = TRUE;
+break;
+}
+}
+
+if (higher_seize) {
+break;
+}
+
+/* cancel any seize by a lower locality */
+for (l = 0; l < locty - 1; l++) {
+tis->loc[l].access &= ~TPM_TIS_ACCESS_SEIZE;
+}
[...]




Ok that looks good.

--
Regards,
Corey Bryant




[Qemu-devel] [PATCH v2] slirp: Don't crash on packets from 0.0.0.0/8.

2012-11-12 Thread Nickolai Zeldovich
LWIP can generate packets with a source of 0.0.0.0, which triggers an
assertion failure in arp_table_add().  Instead of crashing, simply return
to avoid adding an invalid ARP table entry.

Signed-off-by: Nickolai Zeldovich 
---
 slirp/arp_table.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Change from v1: adhere to qemu's code style (put braces around all
indentation blocks).


diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 5d7b8ac..bf698c1 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -38,7 +38,9 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t 
ethaddr[ETH_ALEN])
 ethaddr[3], ethaddr[4], ethaddr[5]));
 
 /* Check 0.0.0.0/8 invalid source-only addresses */
-assert((ip_addr & htonl(~(0xf << 28))) != 0);
+if ((ip_addr & htonl(~(0xf << 28))) == 0) {
+return;
+}
 
 if (ip_addr == 0x || ip_addr == broadcast_addr) {
 /* Do not register broadcast addresses */
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH] slirp: Don't crash on packets from 0.0.0.0/8.

2012-11-12 Thread Jan Kiszka
On 2012-11-12 15:41, Nickolai Zeldovich wrote:
> On Mon, Nov 12, 2012 at 4:37 AM, Jan Kiszka  wrote:
>> On 2012-11-12 01:59, Nickolai Zeldovich wrote:
>>> LWIP can generate packets with a source of 0.0.0.0, which triggers an
>>> assertion failure in arp_table_add().  Instead of crashing, simply return
>>> to avoid adding an invalid ARP table entry.
>>
>> I would prefer to filter out such invalid packets at a different level.
>> Did you analyzed which path it takes through the stack?
> 
> The particular packet that crashed qemu for me was a gratuitous ARP,
> though it looks like all three calls to arp_table_add() in arp_input()
> can trigger this.
> 
> Popping up one level, I'm not sure why arp_table_add() and
> arp_table_search() need a special case for 0.0.0.0/8 in the first
> place.  I couldn't find any other code that assumes the ARP table
> cannot contain 0.0.0.0/8 entries.  Would anything break if the check
> for 0.0.0.0/8 was removed from arp_table_add() and arp_table_search()
> altogether?

0.0.0.0/8 are source-only, invalid as destination. So they have no place
in the ARP table.

OK, let's follow your path and filter them in arp_table_add. Just add
the missing braces and resend.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH] aio: fix aio_ctx_prepare with idle bottom halves

2012-11-12 Thread malc
On Mon, 12 Nov 2012, Paolo Bonzini wrote:

> Commit ed2aec4867f0d5f5de496bb765347b5d0cfe113d changed the return
> value of aio_ctx_prepare from false to true when only idle bottom
> halves are available.  This broke PC old-style DMA, which uses them.
> Fix this by making aio_ctx_prepare return true only when non-idle
> bottom halves are scheduled to run.
> 

Applied, thanks.

[..snip..]

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



[Qemu-devel] -mtdblock QEMU option

2012-11-12 Thread Vipin Gahlaut
Hi Experts,

QEMU 1.2.0
Virtual Platform:versatilepb
Linux Kernel:3.5.5

Command to run QEMU:
/usr/local/bin/qemu-system-arm -M versatilepb -kernel
/home/vgahlaut/labs/linux-3.5.5/arch/arm/boot/zImage -nographic -hda
/home/vgahlaut/labs/disk.img -append "root=/dev/sda mem=128M
console=ttyAMA0,115200"

After my system boots fine, I can see /dev/mtdblock0 but when I tried to
mount mtdblock0 linux gives me error as below

/ # mount /dev/mtdblock0 mnt/
mount: mounting /dev/mtdblock0 on mnt/ failed: Invalid argument
/ # mount -t ext2 /dev/mtdblock0 mnt/
mount: mounting /dev/mtdblock0 on mnt/ failed: Invalid argument
/ # mount -t ext2 -o ro /dev/mtdblock0 mnt/
mount: mounting /dev/mtdblock0 on mnt/ failed: Invalid argument

Then I changed my command to below (added -mtdblock option)

/usr/local/bin/qemu-system-arm -M versatilepb -kernel
/home/vgahlaut/labs/linux-3.5.5/arch/arm/boot/zImage -nographic -hda
/home/vgahlaut/labs/disk.img -mtdblock /home/vgahlaut/labs/nand.img -append
"root=/dev/sda mem=128M console=ttyAMA0,115200"

but then again I get same errors while mounting /dev/mtdblock0. I have
created nand.img is created with qemu-img command and formatted with ext2
after mounting with loopback.

I didn't find much information in documentation on how to use mtdblock
device with QEMU. Can someone please guide me how can I mount qemu
simulated mtdblock device in linux.

Thanks in Advance.

Best Regards
Vipin


Re: [Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

2012-11-12 Thread Paolo Bonzini
Il 12/11/2012 15:03, Paolo Bonzini ha scritto:
> Anthony,
> 
> The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:
> 
>   tcg: properly check that op's output needs to be synced to memory 
> (2012-11-11 16:06:46 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/bonzini/qemu.git scsi-next
> 
> for you to fetch changes up to accfeb2dd32ece73350b06cee1b2403f47e86fe3:
> 
>   scsi-disk: flush cache after disabling it (2012-11-12 15:00:27 +0100)

Updated to commit 4003e24fce1df84a2e8c992376ed2c294816c33c to include
"megasas: Correct target/lun mapping".

Paolo



Re: [Qemu-devel] [PATCH] megasas: Correct target/lun mapping

2012-11-12 Thread Paolo Bonzini
Il 12/11/2012 15:42, Hannes Reinecke ha scritto:
> The structure to reference a logical drive has an unused field,
> which can be used to carry the lun ID. This enabled seabios to
> establish the proper target/LUN mapping.
> 
> Signed-off-by: Hannes Reinecke 
> Cc: Paolo Bonzini 
> Cc: Gerd Hofmann 

Applied to scsi-next branch.

Paolo

> 
> diff --git a/hw/megasas.c b/hw/megasas.c
> index 7a2036e..395feb3 100644
> --- a/hw/megasas.c
> +++ b/hw/megasas.c
> @@ -1080,6 +1080,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, 
> MegasasCmd *cmd)
>  /* Logical device size is in blocks */
>  bdrv_get_geometry(conf->bs, &ld_size);
>  info.ld_list[num_ld_disks].ld.v.target_id = sdev->id;
> +info.ld_list[num_ld_disks].ld.v.lun_id = sdev->lun;
>  info.ld_list[num_ld_disks].state = MFI_LD_STATE_OPTIMAL;
>  info.ld_list[num_ld_disks].size = cpu_to_le64(ld_size);
>  num_ld_disks++;
> diff --git a/hw/mfi.h b/hw/mfi.h
> index 436b690..cd8355b 100644
> --- a/hw/mfi.h
> +++ b/hw/mfi.h
> @@ -1085,7 +1085,7 @@ struct mfi_pd_list {
>  union mfi_ld_ref {
>  struct {
>  uint8_t target_id;
> -uint8_t reserved;
> +uint8_t lun_id;
>  uint16_t seq;
>  } v;
>  uint32_t ref;
> 
> 




[Qemu-devel] [PATCH] megasas: Correct target/lun mapping

2012-11-12 Thread Hannes Reinecke
The structure to reference a logical drive has an unused field,
which can be used to carry the lun ID. This enabled seabios to
establish the proper target/LUN mapping.

Signed-off-by: Hannes Reinecke 
Cc: Paolo Bonzini 
Cc: Gerd Hofmann 

diff --git a/hw/megasas.c b/hw/megasas.c
index 7a2036e..395feb3 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -1080,6 +1080,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, 
MegasasCmd *cmd)
 /* Logical device size is in blocks */
 bdrv_get_geometry(conf->bs, &ld_size);
 info.ld_list[num_ld_disks].ld.v.target_id = sdev->id;
+info.ld_list[num_ld_disks].ld.v.lun_id = sdev->lun;
 info.ld_list[num_ld_disks].state = MFI_LD_STATE_OPTIMAL;
 info.ld_list[num_ld_disks].size = cpu_to_le64(ld_size);
 num_ld_disks++;
diff --git a/hw/mfi.h b/hw/mfi.h
index 436b690..cd8355b 100644
--- a/hw/mfi.h
+++ b/hw/mfi.h
@@ -1085,7 +1085,7 @@ struct mfi_pd_list {
 union mfi_ld_ref {
 struct {
 uint8_t target_id;
-uint8_t reserved;
+uint8_t lun_id;
 uint16_t seq;
 } v;
 uint32_t ref;



Re: [Qemu-devel] qemu and transparent huge pages

2012-11-12 Thread Michael Tokarev
Ping^2 ?

Thanks,

/mjt

16.09.2012 15:19, Michael Tokarev wrote:
> So, is the patch okay?
> 
> Thanks,
> 
> /mjt
> 
> On 15.08.2012 19:03, Michael Tokarev wrote:
>> On 15.08.2012 18:26, Avi Kivity wrote:
>>> On 08/15/2012 05:22 PM, Michael Tokarev wrote:
>>>
>
> Please provide extra info, like the setting of
> /sys/kernel/mm/transparent_hugepage/enabled.

 That was it - sort of.  Default value here is enabled=madvise.
 When setting it to always the effect finally started appearing,
 so it is actually working.

 But can't qemu set MADV_HUGEPAGE flag too, so it works automatically?
>>>
>>> It can and should.
>>
>> Something like the attached patch?
>>
>> Thanks,
>>
>> /mjt
> 




Re: [Qemu-devel] q35, usb-ehci1, and Mac OS X boot problems

2012-11-12 Thread Gabriel L. Somlo
Gerd,

On Mon, Nov 12, 2012 at 11:26:18AM +0100, Gerd Hoffmann wrote:
> Seems macos doesn't like something in our ehci emulation ...
> Can you send a trace with all ehci tracepoints enabled?

For brevity, I replaced each cluster of six "Port X not enabled"
lines with one "Port 0..5 not enabled", but otherwise the output
below is what I got running the "-usb -device usb-kbd -device usb-mouse"
command line with EHCI_DEBUG set to 1.

Thanks,
--Gabriel


Port 0 not enabled
Port 1 not enabled
Port 2 not enabled
Port 3 not enabled
Port 4 not enabled
Port 5 not enabled
PERIODIC state adv fr=1.  [7FFFB004] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
PERIODIC state adv fr=2.  [7FFFB008] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=3.  [7FFFB00C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=4.  [7FFFB010] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=5.  [7FFFB014] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=6.  [7FFFB018] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=7.  [7FFFB01C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=8.  [7FFFB020] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=9.  [7FFFB024] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
PERIODIC state adv fr=10.  [7FFFB028] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
PERIODIC state adv fr=11.  [7FFFB02C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
PERIODIC state adv fr=12.  [7FFFB030] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
PERIODIC state adv fr=13.  [7FFFB034] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=14.  [7FFFB038] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=15.  [7FFFB03C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=16.  [7FFFB040] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=17.  [7FFFB044] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=18.  [7FFFB048] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=19.  [7FFFB04C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=20.  [7FFFB050] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=21.  [7FFFB054] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=22.  [7FFFB058] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=23.  [7FFFB05C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=24.  [7FFFB060] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=25.  [7FFFB064] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=26.  [7FFFB068] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=27.  [7FFFB06C] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=28.  [7FFFB070] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enabled
PERIODIC state adv fr=29.  [7FFFB074] -> 7FFFC902
Port 0..5 not enabled
FETCHQH:  QH 0x7fffc902 (h 0 halt 40 active 0) next 0x0001
Port 0..5 not enab

Re: [Qemu-devel] [PATCH] Revert "serial: fix retry logic"

2012-11-12 Thread Michael Tokarev
Ping^2 ?

/mjt

27.10.2012 12:31, Michael Tokarev wrote:
> Ping?
> 
> On 19.09.2012 12:08, Michael Tokarev wrote:
>> This reverts commit 67c5322d7000fd105a926eec44bc1765b7d70bdd:
>>
>> I'm not sure if the retry logic has ever worked when not using FIFO 
>> mode.  I
>> found this while writing a test case although code inspection confirms 
>> it is
>> definitely broken.
>>
>> The TSR retry logic will never actually happen because it is guarded by 
>> an
>> 'if (s->tsr_rety > 0)' but this is the only place that can ever make the
>> variable greater than zero.  That effectively makes the retry logic an 
>> 'if (0)
>>
>> I believe this is a typo and the intention was >= 0.  Once this is fixed 
>> thoug
>> I see double transmits with my test case.  This is because in the non 
>> FIFO
>> case, serial_xmit may get invoked while LSR.THRE is still high because 
>> the
>> character was processed but the retransmit timer was still active.
>>
>> We can handle this by simply checking for LSR.THRE and returning early.  
>> It's
>> possible that the FIFO paths also need some attention.
>>
>> Cc: Stefano Stabellini 
>> Signed-off-by: Anthony Liguori 
>>
>> Even if the previous logic was never worked, new logic breaks stuff -
>> namely,
>>
>>  qemu -enable-kvm -nographic -kernel /boot/vmlinuz-$(uname -r) -append 
>> console=ttyS0 -serial pty
>>
>> the above command will cause the virtual machine to stuck at startup
>> using 100% CPU till one connects to the pty and sends any char to it.
>>
>> Note this is rather typical invocation for various headless virtual
>> machines by libvirt.
>>
>> So revert this change for now, till a better solution will be found.
>>
>> Signed-off-by: Michael Tokarev 
>> ---
>>  hw/serial.c |4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/serial.c b/hw/serial.c
>> index a421d1e..df54de2 100644
>> --- a/hw/serial.c
>> +++ b/hw/serial.c
>> @@ -327,8 +327,6 @@ static void serial_xmit(void *opaque)
>>  s->tsr = fifo_get(s,XMIT_FIFO);
>>  if (!s->xmit_fifo.count)
>>  s->lsr |= UART_LSR_THRE;
>> -} else if ((s->lsr & UART_LSR_THRE)) {
>> -return;
>>  } else {
>>  s->tsr = s->thr;
>>  s->lsr |= UART_LSR_THRE;
>> @@ -340,7 +338,7 @@ static void serial_xmit(void *opaque)
>>  /* in loopback mode, say that we just received a char */
>>  serial_receive1(s, &s->tsr, 1);
>>  } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) {
>> -if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
>> +if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
>>  s->tsr_retry++;
>>  qemu_mod_timer(s->transmit_timer,  new_xmit_ts + 
>> s->char_transmit_time);
>>  return;
> 
> 




[Qemu-devel] [PATCH 3/7] nbd: accept URIs

2012-11-12 Thread Paolo Bonzini
The URI syntax is consistent with the Gluster syntax.  Export names
are specified in the path, preceded by one or more (otherwise unused)
slashes.

Signed-off-by: Paolo Bonzini 
---
 block/nbd.c   | 98 ++-
 qemu-doc.texi | 25 ++-
 2 file modificati, 114 inserzioni(+), 9 rimozioni(-)

diff --git a/block/nbd.c b/block/nbd.c
index 48bbeca..e87c248 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -28,6 +28,7 @@
 
 #include "qemu-common.h"
 #include "nbd.h"
+#include "uri.h"
 #include "block_int.h"
 #include "module.h"
 #include "qemu_socket.h"
@@ -69,6 +70,69 @@ typedef struct BDRVNBDState {
 char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
+static int nbd_parse_uri(BDRVNBDState *s, const char *filename)
+{
+URI *uri;
+const char *p;
+QueryParams *qp = NULL;
+int ret = 0;
+
+uri = uri_parse(filename);
+if (!uri) {
+return -EINVAL;
+}
+
+/* transport */
+if (!strcmp(uri->scheme, "nbd")) {
+s->is_unix = false;
+} else if (!strcmp(uri->scheme, "nbd+tcp")) {
+s->is_unix = false;
+} else if (!strcmp(uri->scheme, "nbd+unix")) {
+s->is_unix = true;
+} else {
+ret = -EINVAL;
+goto out;
+}
+
+p = uri->path ? uri->path : "/";
+p += strspn(p, "/");
+if (p[0]) {
+s->export_name = g_strdup(p);
+}
+
+qp = query_params_parse(uri->query);
+if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
+ret = -EINVAL;
+goto out;
+}
+
+if (s->is_unix) {
+/* nbd+unix:///export?socket=path */
+if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
+ret = -EINVAL;
+goto out;
+}
+s->host_spec = g_strdup(qp->p[0].value);
+} else {
+/* nbd[+tcp]://host:port/export */
+if (!uri->server) {
+ret = -EINVAL;
+goto out;
+}
+if (!uri->port) {
+uri->port = NBD_DEFAULT_PORT;
+}
+s->host_spec = g_strdup_printf("%s:%d", uri->server, uri->port);
+}
+
+out:
+if (qp) {
+query_params_free(qp);
+}
+uri_free(uri);
+return ret;
+}
+
 static int nbd_config(BDRVNBDState *s, const char *filename)
 {
 char *file;
@@ -77,6 +141,10 @@ static int nbd_config(BDRVNBDState *s, const char *filename)
 const char *unixpath;
 int err = -EINVAL;
 
+if (strstr(filename, "://")) {
+return nbd_parse_uri(s, filename);
+}
+
 file = g_strdup(filename);
 
 export_name = strstr(file, EN_OPTSTR);
@@ -495,6 +563,33 @@ static int64_t nbd_getlength(BlockDriverState *bs)
 
 static BlockDriver bdrv_nbd = {
 .format_name = "nbd",
+.protocol_name   = "nbd",
+.instance_size   = sizeof(BDRVNBDState),
+.bdrv_file_open  = nbd_open,
+.bdrv_co_readv   = nbd_co_readv,
+.bdrv_co_writev  = nbd_co_writev,
+.bdrv_close  = nbd_close,
+.bdrv_co_flush_to_os = nbd_co_flush,
+.bdrv_co_discard = nbd_co_discard,
+.bdrv_getlength  = nbd_getlength,
+};
+
+static BlockDriver bdrv_nbd_tcp = {
+.format_name = "nbd",
+.protocol_name   = "nbd+tcp",
+.instance_size   = sizeof(BDRVNBDState),
+.bdrv_file_open  = nbd_open,
+.bdrv_co_readv   = nbd_co_readv,
+.bdrv_co_writev  = nbd_co_writev,
+.bdrv_close  = nbd_close,
+.bdrv_co_flush_to_os = nbd_co_flush,
+.bdrv_co_discard = nbd_co_discard,
+.bdrv_getlength  = nbd_getlength,
+};
+
+static BlockDriver bdrv_nbd_unix = {
+.format_name = "nbd",
+.protocol_name   = "nbd+unix",
 .instance_size   = sizeof(BDRVNBDState),
 .bdrv_file_open  = nbd_open,
 .bdrv_co_readv   = nbd_co_readv,
@@ -503,12 +598,13 @@ static BlockDriver bdrv_nbd = {
 .bdrv_co_flush_to_os = nbd_co_flush,
 .bdrv_co_discard = nbd_co_discard,
 .bdrv_getlength  = nbd_getlength,
-.protocol_name   = "nbd",
 };
 
 static void bdrv_nbd_init(void)
 {
 bdrv_register(&bdrv_nbd);
+bdrv_register(&bdrv_nbd_tcp);
+bdrv_register(&bdrv_nbd_unix);
 }
 
 block_init(bdrv_nbd_init);
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 35cabbc..d8fb2de 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -610,14 +610,14 @@ QEMU can access directly to block device exported using 
the Network Block Device
 protocol.
 
 @example
-qemu-system-i386 linux.img -hdb nbd:my_nbd_server.mydomain.org:1024
+qemu-system-i386 linux.img -hdb nbd://my_nbd_server.mydomain.org:1024/
 @end example
 
 If the NBD server is located on the same host, you can use an unix socket 
instead
 of an inet socket:
 
 @example
-qemu-system-i386 linux.img -hdb nbd:unix:/tmp/my_socket
+qemu-system-i386 linux.img -hdb nbd+unix://?socket=/tmp/my_socket
 @end example
 
 In this case, the block device must be exported using qemu-nbd:
@@ -631,

[Qemu-devel] [PATCH] ehci: fix compile error with EHCI_DEBUG enabled

2012-11-12 Thread Gabriel L. Somlo
This patch fixes a few debugging print statements whose arguments fell
out of sync over time with changes being made to the active code base.

Signed-off-by: Gabriel Somlo 
---

On Mon, Nov 12, 2012 at 11:26:18AM +0100, Gerd Hoffmann wrote:
> Seems macos doesn't like something in our ehci emulation ...
> Can you send a trace with all ehci tracepoints enabled?

Turning EHCI_DEBUG on gave me compile errors. This is my best guess
as to what the DPRINTF arguments *should* be, hope I guessed right :)

This patch works against both master and the q35 branch, BTW.

I'll reply to your email again with the debug output, figured I'd get
this out of the way first...

Thanks,
  Gabriel

 hw/usb/hcd-ehci.c |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index d9dc576..e3ccb59 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -1188,7 +1188,7 @@ static void ehci_execute_complete(EHCIQueue *q)
p->async == EHCI_ASYNC_FINISHED);
 
 DPRINTF("execute_complete: qhaddr 0x%x, next %x, qtdaddr 0x%x, status 
%d\n",
-q->qhaddr, q->qh.next, q->qtdaddr, q->usb_status);
+q->qhaddr, q->qh.next, q->qtdaddr, p->usb_status);
 
 if (p->usb_status < 0) {
 switch (p->usb_status) {
@@ -1305,8 +1305,8 @@ static int ehci_execute(EHCIPacket *p, const char *action)
 trace_usb_ehci_packet_action(p->queue, p, action);
 ret = usb_handle_packet(p->queue->dev, &p->packet);
 DPRINTF("submit: qh %x next %x qtd %x pid %x len %zd endp %x ret %d\n",
-q->qhaddr, q->qh.next, q->qtdaddr, q->pid,
-q->packet.iov.size, endp, ret);
+p->queue->qhaddr, p->queue->qh.next, p->queue->qtdaddr, p->pid,
+p->packet.iov.size, endp, ret);
 
 if (ret > BUFF_SIZE) {
 fprintf(stderr, "ret from usb_handle_packet > BUFF_SIZE\n");
-- 
1.7.7.6




Re: [Qemu-devel] [PATCH] slirp: Don't crash on packets from 0.0.0.0/8.

2012-11-12 Thread Nickolai Zeldovich
On Mon, Nov 12, 2012 at 4:37 AM, Jan Kiszka  wrote:
> On 2012-11-12 01:59, Nickolai Zeldovich wrote:
>> LWIP can generate packets with a source of 0.0.0.0, which triggers an
>> assertion failure in arp_table_add().  Instead of crashing, simply return
>> to avoid adding an invalid ARP table entry.
>
> I would prefer to filter out such invalid packets at a different level.
> Did you analyzed which path it takes through the stack?

The particular packet that crashed qemu for me was a gratuitous ARP,
though it looks like all three calls to arp_table_add() in arp_input()
can trigger this.

Popping up one level, I'm not sure why arp_table_add() and
arp_table_search() need a special case for 0.0.0.0/8 in the first
place.  I couldn't find any other code that assumes the ARP table
cannot contain 0.0.0.0/8 entries.  Would anything break if the check
for 0.0.0.0/8 was removed from arp_table_add() and arp_table_search()
altogether?

Nickolai.



[Qemu-devel] [PATCH 5/7] nbd: force read-only export for read-only devices

2012-11-12 Thread Paolo Bonzini
This is the desired behavior for HMP, but it is a better choice for QMP as well.

Signed-off-by: Paolo Bonzini 
---
 blockdev-nbd.c | 7 +++
 1 file modificato, 7 inserzioni(+)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 274fba6..e362572 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -93,6 +93,13 @@ void qmp_nbd_server_add(const char *device, bool 
has_writable, bool writable,
 return;
 }
 
+if (!has_writable) {
+writable = true;
+}
+if (bdrv_is_read_only(bs)) {
+writable = false;
+}
+
 exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
  nbd_server_put_ref);
 
-- 
1.7.12.1





[Qemu-devel] [PATCH 4/7] nbd: fix nbd_server_stop crash when no server was running

2012-11-12 Thread Paolo Bonzini
This failed on the new assertion of qemu_set_fd_handler2:

qemu-system-x86_64: /home/pbonzini/work/upstream/qemu/iohandler.c:60: 
qemu_set_fd_handler2: Assertion `fd >= 0' failed.

Signed-off-by: Paolo Bonzini 
---
 blockdev-nbd.c | 8 +---
 1 file modificato, 5 inserzioni(+), 3 rimozioni(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 8031813..274fba6 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -113,7 +113,9 @@ void qmp_nbd_server_stop(Error **errp)
 nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
 }
 
-qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
-close(server_fd);
-server_fd = -1;
+if (server_fd != -1) {
+qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+close(server_fd);
+server_fd = -1;
+}
 }
-- 
1.7.12.1





[Qemu-devel] [PATCH 2/5] virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting events

2012-11-12 Thread Paolo Bonzini
Suggested by Laszlo Ersek.

Signed-off-by: Paolo Bonzini 
---
 hw/virtio-scsi.c | 8 +---
 1 file modificato, 5 inserzioni(+), 3 rimozioni(-)

diff --git a/hw/virtio-scsi.c b/hw/virtio-scsi.c
index b54c789..30d3f8a 100644
--- a/hw/virtio-scsi.c
+++ b/hw/virtio-scsi.c
@@ -596,6 +596,10 @@ static void virtio_scsi_push_event(VirtIOSCSI *s, 
SCSIDevice *dev,
 VirtIOSCSIEvent *evt;
 int in_size;
 
+if (!(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+return;
+}
+
 if (!req) {
 s->events_dropped = true;
 return;
@@ -648,7 +652,6 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice 
*dev, SCSISense sense)
 VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
 
 if (((s->vdev.guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) &&
-(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK) &&
 dev->type != TYPE_ROM) {
 virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE,
sense.asc | (sense.ascq << 8));
@@ -659,8 +662,7 @@ static void virtio_scsi_hotplug(SCSIBus *bus, SCSIDevice 
*dev)
 {
 VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus);
 
-if (((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) &&
-(s->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK)) {
+if ((s->vdev.guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) {
 virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_TRANSPORT_RESET,
VIRTIO_SCSI_EVT_RESET_RESCAN);
 }
-- 
1.7.12.1





[Qemu-devel] [PATCH 6/7] nbd: disallow nbd-server-add before nbd-server-start

2012-11-12 Thread Paolo Bonzini
It works nicely with the QMP commands, but it adds useless complication
with HMP.  In particular, see the following:

(qemu) nbd_server_add -w scsi0-hd0
(qemu) nbd_server_start -a localhost:10809
NBD server already exporting device scsi0-hd0

Signed-off-by: Paolo Bonzini 
---
 blockdev-nbd.c | 5 +
 1 file modificato, 5 inserzioni(+)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index e362572..d1721a3 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -82,6 +82,11 @@ void qmp_nbd_server_add(const char *device, bool 
has_writable, bool writable,
 NBDExport *exp;
 NBDCloseNotifier *n;
 
+if (server_fd == -1) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
 if (nbd_export_find(device)) {
 error_setg(errp, "NBD server already exporting device '%s'", device);
 return;
-- 
1.7.12.1





[Qemu-devel] [PATCH 2/7] nbd: accept relative path to Unix socket

2012-11-12 Thread Paolo Bonzini
Adding the "is_unix" member now will simplify the parsing of NBD URIs.

Signed-off-by: Paolo Bonzini 
---
 block/nbd.c | 17 +++--
 1 file modificato, 7 inserzioni(+), 10 rimozioni(-)

diff --git a/block/nbd.c b/block/nbd.c
index 2bce47b..48bbeca 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -55,7 +55,6 @@ typedef struct BDRVNBDState {
 uint32_t nbdflags;
 off_t size;
 size_t blocksize;
-char *export_name; /* An NBD server may export several devices */
 
 CoMutex send_mutex;
 CoMutex free_sema;
@@ -65,13 +64,12 @@ typedef struct BDRVNBDState {
 Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
 struct nbd_reply reply;
 
-/* If it begins with  '/', this is a UNIX domain socket. Otherwise,
- * it's a string of the form :port
- */
+int is_unix;
 char *host_spec;
+char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
-static int nbd_config(BDRVNBDState *s, const char *filename, int flags)
+static int nbd_config(BDRVNBDState *s, const char *filename)
 {
 char *file;
 char *export_name;
@@ -98,11 +96,10 @@ static int nbd_config(BDRVNBDState *s, const char 
*filename, int flags)
 
 /* are we a UNIX or TCP socket? */
 if (strstart(host_spec, "unix:", &unixpath)) {
-if (unixpath[0] != '/') { /* We demand  an absolute path*/
-goto out;
-}
+s->is_unix = true;
 s->host_spec = g_strdup(unixpath);
 } else {
+s->is_unix = false;
 s->host_spec = g_strdup(host_spec);
 }
 
@@ -262,7 +259,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
 off_t size;
 size_t blocksize;
 
-if (s->host_spec[0] == '/') {
+if (s->is_unix) {
 sock = unix_socket_outgoing(s->host_spec);
 } else {
 sock = tcp_socket_outgoing_spec(s->host_spec);
@@ -320,7 +317,7 @@ static int nbd_open(BlockDriverState *bs, const char* 
filename, int flags)
 qemu_co_mutex_init(&s->free_sema);
 
 /* Pop the config into our state object. Exit if invalid. */
-result = nbd_config(s, filename, flags);
+result = nbd_config(s, filename);
 if (result != 0) {
 return result;
 }
-- 
1.7.12.1





[Qemu-devel] [PATCH 7/7] hmp: add NBD server commands

2012-11-12 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hmp-commands.hx | 45 ++
 hmp.c   | 76 +
 hmp.h   |  3 +++
 3 file modificati, 124 inserzioni(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index f916385..b74ef75 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1310,6 +1310,51 @@ Remove all matches from the access control list, and set 
the default
 policy back to @code{deny}.
 ETEXI
 
+{
+.name   = "nbd_server_start",
+.args_type  = "all:-a,writable:-w,uri:s",
+.params = "nbd_server_start [-a] [-w] host:port",
+.help   = "serve block devices on the given host and port",
+.mhandler.cmd = hmp_nbd_server_start,
+},
+STEXI
+@item nbd_server_start @var{host}:@var{port}
+@findex nbd_server_start
+Start an NBD server on the given host and/or port.  If the @option{-a}
+option is included, all of the virtual machine's block devices that
+have an inserted media on them are automatically exported; in this case,
+the @option{-w} option makes the devices writable too.
+ETEXI
+
+{
+.name   = "nbd_server_add",
+.args_type  = "writable:-w,device:B",
+.params = "nbd_server_add [-w] device",
+.help   = "export a block device via NBD",
+.mhandler.cmd = hmp_nbd_server_add,
+},
+STEXI
+@item nbd_server_add @var{device}
+@findex nbd_server_add
+Export a block device through QEMU's NBD server, which must be started
+beforehand with @command{nbd_server_start}.  The @option{-w} option makes the
+exported device writable too.
+ETEXI
+
+{
+.name   = "nbd_server_stop",
+.args_type  = "",
+.params = "nbd_server_stop",
+.help   = "stop serving block devices using the NBD protocol",
+.mhandler.cmd = hmp_nbd_server_stop,
+},
+STEXI
+@item nbd_server_stop
+@findex nbd_server_stop
+Stop the QEMU embedded NBD server.
+ETEXI
+
+
 #if defined(TARGET_I386)
 
 {
diff --git a/hmp.c b/hmp.c
index 895a343..180ba2b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -18,6 +18,7 @@
 #include "qemu-option.h"
 #include "qemu-timer.h"
 #include "qmp-commands.h"
+#include "qemu_socket.h"
 #include "monitor.h"
 #include "console.h"
 
@@ -1259,3 +1260,78 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
 qmp_screendump(filename, &err);
 hmp_handle_error(mon, &err);
 }
+
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
+{
+const char *uri = qdict_get_str(qdict, "uri");
+int writable = qdict_get_try_bool(qdict, "writable", 0);
+int all = qdict_get_try_bool(qdict, "all", 0);
+Error *local_err = NULL;
+BlockInfoList *block_list, *info;
+SocketAddress *addr;
+
+if (writable && !all) {
+error_setg(&local_err, "-w only valid together with -a");
+goto exit;
+}
+
+/* First check if the address is valid and start the server.  */
+addr = socket_parse(uri, &local_err);
+if (local_err != NULL) {
+goto exit;
+}
+
+qmp_nbd_server_start(addr, &local_err);
+qapi_free_SocketAddress(addr);
+if (local_err != NULL) {
+goto exit;
+}
+
+if (!all) {
+return;
+}
+
+/* Then try adding all block devices.  If one fails, close all and
+ * exit.
+ */
+block_list = qmp_query_block(NULL);
+
+for (info = block_list; info; info = info->next) {
+if (!info->value->has_inserted) {
+continue;
+}
+
+qmp_nbd_server_add(info->value->device, true, writable, &local_err);
+
+if (local_err != NULL) {
+qmp_nbd_server_stop(NULL);
+break;
+}
+}
+
+qapi_free_BlockInfoList(block_list);
+
+exit:
+hmp_handle_error(mon, &local_err);
+}
+
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
+{
+const char *device = qdict_get_str(qdict, "device");
+int writable = qdict_get_try_bool(qdict, "writable", 0);
+Error *local_err = NULL;
+
+qmp_nbd_server_add(device, true, writable, &local_err);
+
+if (local_err != NULL) {
+hmp_handle_error(mon, &local_err);
+}
+}
+
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
+{
+Error *errp = NULL;
+
+qmp_nbd_server_stop(&errp);
+hmp_handle_error(mon, &errp);
+}
diff --git a/hmp.h b/hmp.h
index 34eb2b3..0ab03be 100644
--- a/hmp.h
+++ b/hmp.h
@@ -77,5 +77,8 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 
 #endif
-- 
1.7.12.1




[Qemu-devel] [PATCH 1/7] qemu-nbd: initialize main loop before block layer

2012-11-12 Thread Paolo Bonzini
qemu-nbd was broken because they initialized the block layer while
qemu_aio_context was still NULL.

Signed-off-by: Paolo Bonzini 
---
 qemu-nbd.c | 2 +-
 1 file modificato, 1 inserzione(+). 1 rimozione(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 15bcd08..80f08d8 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -539,6 +539,7 @@ int main(int argc, char **argv)
 snprintf(sockpath, 128, SOCKET_PATH, basename(device));
 }
 
+qemu_init_main_loop();
 bdrv_init();
 atexit(bdrv_close_all);
 
@@ -584,7 +585,6 @@ int main(int argc, char **argv)
 memset(&client_thread, 0, sizeof(client_thread));
 }
 
-qemu_init_main_loop();
 qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
  (void *)(uintptr_t)fd);
 
-- 
1.7.12.1





[Qemu-devel] [PULL 1.3 0/7] NBD updates for 2012-11-12

2012-11-12 Thread Paolo Bonzini
Anthony,

The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:

  tcg: properly check that op's output needs to be synced to memory (2012-11-11 
16:06:46 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git nbd-next

for you to fetch changes up to 4057725f35abe00ea371f85c6e27dd25eafd9ddf:

  hmp: add NBD server commands (2012-11-12 14:38:29 +0100)

The main change is making the NBD server available via HMP.  This prompted
a few minor usability improvements in the QMP interface too.  Also,
NBD is made accessible via URIs, similar to Gluster (the change is
backwards-compatible).


Paolo Bonzini (7):
  qemu-nbd: initialize main loop before block layer
  nbd: accept relative path to Unix socket
  nbd: accept URIs
  nbd: fix nbd_server_stop crash when no server was running
  nbd: force read-only export for read-only devices
  nbd: disallow nbd-server-add before nbd-server-start
  hmp: add NBD server commands

 block/nbd.c | 115 ++--
 blockdev-nbd.c  |  20 --
 hmp-commands.hx |  45 ++
 hmp.c   |  76 +
 hmp.h   |   3 ++
 qemu-doc.texi   |  25 
 qemu-nbd.c  |   2 +-
 7 file modificati, 263 inserzioni(+), 23 rimozioni(-)
-- 
1.7.12.1




[Qemu-devel] [PATCH 5/5] scsi-disk: flush cache after disabling it

2012-11-12 Thread Paolo Bonzini
SBC says that "if an application client changes the WCE bit from one to
zero via a MODE SELECT command, then the device server shall write
any data in volatile cache to non-volatile medium before completing
the command".

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c | 9 +
 1 file modificato, 9 inserzioni(+)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index d15f891..49b5686 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1387,6 +1387,7 @@ invalid_param_len:
 
 static void scsi_disk_emulate_mode_select(SCSIDiskReq *r, uint8_t *inbuf)
 {
+SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);
 uint8_t *p = inbuf;
 int cmd = r->req.cmd.buf[0];
 int len = r->req.cmd.xfer;
@@ -1423,6 +1424,14 @@ static void scsi_disk_emulate_mode_select(SCSIDiskReq 
*r, uint8_t *inbuf)
 return;
 }
 }
+if (!bdrv_enable_write_cache(s->qdev.conf.bs)) {
+/* The request is used as the AIO opaque value, so add a ref.  */
+scsi_req_ref(&r->req);
+bdrv_acct_start(s->qdev.conf.bs, &r->acct, 0, BDRV_ACCT_FLUSH);
+r->req.aiocb = bdrv_aio_flush(s->qdev.conf.bs, scsi_aio_complete, r);
+return;
+}
+
 scsi_req_complete(&r->req, GOOD);
 return;
 
-- 
1.7.12.1




[Qemu-devel] [PATCH] kvm: Actually remove software breakpoints from list on cleanup

2012-11-12 Thread Jan Kiszka
So far we only removed them from the guest, leaving its states in the
list. This made it impossible for gdb to re-enable breakpoints on the
same address after re-attaching.

Signed-off-by: Jan Kiszka 
---
 kvm-all.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index b6d0483..3bc3347 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1905,6 +1905,8 @@ void kvm_remove_all_breakpoints(CPUArchState *current_env)
 }
 }
 }
+QTAILQ_REMOVE(&s->kvm_sw_breakpoints, bp, entry);
+g_free(bp);
 }
 kvm_arch_remove_all_hw_breakpoints();
 
-- 
1.7.3.4



[Qemu-devel] [PATCH 4/5] megasas: do not include block_int.h

2012-11-12 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini 
---
 hw/megasas.c | 1 -
 1 file modificato. 1 rimozione(-)

diff --git a/hw/megasas.c b/hw/megasas.c
index 7a2036e..b845ea7 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -25,7 +25,6 @@
 #include "iov.h"
 #include "scsi.h"
 #include "scsi-defs.h"
-#include "block_int.h"
 #include "trace.h"
 
 #include "mfi.h"
-- 
1.7.12.1





[Qemu-devel] [PATCH 3/5] scsi: remove superfluous call to scsi_device_set_ua

2012-11-12 Thread Paolo Bonzini
Suggested by Laszlo Ersek.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c | 1 -
 1 file modificato. 1 rimozione(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 098558d..d15f891 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -1964,7 +1964,6 @@ static void scsi_disk_resize_cb(void *opaque)
  * direct-access devices.
  */
 if (s->qdev.type == TYPE_DISK) {
-scsi_device_set_ua(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
 scsi_device_report_change(&s->qdev, SENSE_CODE(CAPACITY_CHANGED));
 }
 }
-- 
1.7.12.1





[Qemu-devel] [PATCH 1/5] scsi: do not return short responses for emulated commands

2012-11-12 Thread Paolo Bonzini
The inquiry command, for the case of VPD=1, was returning short
responses; the number of returned bytes was just the number of bytes
in the request, without padding to the specified allocation length
with zero bytes.  This is usually harmless, but it is a violation
of the SCSI specification.

To fix this, always pad with zero bytes to r->cmd.xfer in
scsi_disk_emulate_command, and return at most r->buflen bytes
(the size of the buffer for command data) rather than at most
buflen bytes (the number of bytes that was filled in).

Before this patch, "strace sg_inq -p0x83 /dev/sda" would report a
non-zero resid value.  After this patch, it reports resid=0.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi-disk.c | 34 ++
 1 file modificato, 18 inserzioni(+), 16 rimozioni(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index 1b0afa6..098558d 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -652,7 +652,6 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, 
uint8_t *outbuf)
 if (buflen > SCSI_MAX_INQUIRY_LEN) {
 buflen = SCSI_MAX_INQUIRY_LEN;
 }
-memset(outbuf, 0, buflen);
 
 outbuf[0] = s->qdev.type & 0x1f;
 outbuf[1] = (s->features & (1 << SCSI_DISK_F_REMOVABLE)) ? 0x80 : 0;
@@ -1596,24 +1595,26 @@ static int32_t scsi_disk_emulate_command(SCSIRequest 
*req, uint8_t *buf)
 break;
 }
 
+/*
+ * FIXME: we shouldn't return anything bigger than 4k, but the code
+ * requires the buffer to be as big as req->cmd.xfer in several
+ * places.  So, do not allow CDBs with a very large ALLOCATION
+ * LENGTH.  The real fix would be to modify scsi_read_data and
+ * dma_buf_read, so that they return data beyond the buflen
+ * as all zeros.
+ */
+if (req->cmd.xfer > 65536) {
+goto illegal_request;
+}
+r->buflen = MAX(4096, req->cmd.xfer);
+
 if (!r->iov.iov_base) {
-/*
- * FIXME: we shouldn't return anything bigger than 4k, but the code
- * requires the buffer to be as big as req->cmd.xfer in several
- * places.  So, do not allow CDBs with a very large ALLOCATION
- * LENGTH.  The real fix would be to modify scsi_read_data and
- * dma_buf_read, so that they return data beyond the buflen
- * as all zeros.
- */
-if (req->cmd.xfer > 65536) {
-goto illegal_request;
-}
-r->buflen = MAX(4096, req->cmd.xfer);
 r->iov.iov_base = qemu_blockalign(s->qdev.conf.bs, r->buflen);
 }
 
 buflen = req->cmd.xfer;
 outbuf = r->iov.iov_base;
+memset(outbuf, 0, r->buflen);
 switch (req->cmd.buf[0]) {
 case TEST_UNIT_READY:
 assert(!s->tray_open && bdrv_is_inserted(s->qdev.conf.bs));
@@ -1694,12 +1695,14 @@ static int32_t scsi_disk_emulate_command(SCSIRequest 
*req, uint8_t *buf)
 outbuf[5] = 0;
 outbuf[6] = s->qdev.blocksize >> 8;
 outbuf[7] = 0;
-buflen = 8;
 break;
 case REQUEST_SENSE:
 /* Just return "NO SENSE".  */
 buflen = scsi_build_sense(NULL, 0, outbuf, r->buflen,
   (req->cmd.buf[1] & 1) == 0);
+if (buflen < 0) {
+goto illegal_request;
+}
 break;
 case MECHANISM_STATUS:
 buflen = scsi_emulate_mechanism_status(s, outbuf);
@@ -1770,7 +1773,6 @@ static int32_t scsi_disk_emulate_command(SCSIRequest 
*req, uint8_t *buf)
 }
 
 /* Protection, exponent and lowest lba field left blank. */
-buflen = req->cmd.xfer;
 break;
 }
 DPRINTF("Unsupported Service Action In\n");
@@ -1827,7 +1829,7 @@ static int32_t scsi_disk_emulate_command(SCSIRequest 
*req, uint8_t *buf)
 return 0;
 }
 assert(!r->req.aiocb);
-r->iov.iov_len = MIN(buflen, req->cmd.xfer);
+r->iov.iov_len = MIN(r->buflen, req->cmd.xfer);
 if (r->iov.iov_len == 0) {
 scsi_req_complete(&r->req, GOOD);
 }
-- 
1.7.12.1





[Qemu-devel] [PULL 1.3 0/5] SCSI updates for 2012-11-12

2012-11-12 Thread Paolo Bonzini
Anthony,

The following changes since commit 3c5645fab3c4b65d0cffbe1aaafc787e4be63d0f:

  tcg: properly check that op's output needs to be synced to memory (2012-11-11 
16:06:46 +0100)

are available in the git repository at:

  git://github.com/bonzini/qemu.git scsi-next

for you to fetch changes up to accfeb2dd32ece73350b06cee1b2403f47e86fe3:

  scsi-disk: flush cache after disabling it (2012-11-12 15:00:27 +0100)


Paolo Bonzini (5):
  scsi: do not return short responses for emulated commands
  virtio-scsi: factor checks for VIRTIO_SCSI_S_DRIVER_OK when reporting 
events
  scsi: remove superfluous call to scsi_device_set_ua
  megasas: do not include block_int.h
  scsi-disk: flush cache after disabling it

 hw/megasas.c |  1 -
 hw/scsi-disk.c   | 44 +++-
 hw/virtio-scsi.c |  8 +---
 3 file modificati, 32 inserzioni(+), 21 rimozioni(-)
-- 
1.7.12.1




Re: [Qemu-devel] [PATCH V2] qemu-sockets: Fix parsing of the inet option 'to'.

2012-11-12 Thread Markus Armbruster
Anthony PERARD  writes:

> Having a qemu command line argument like "-vnc 127.0.0.1:0,to=99" is broken.
> This have been break with commit 879e45c72da1569e07fbbc6a1aa2a708ea796044.
>
> Signed-off-by: Anthony PERARD 

Thanks for taking the time to identify the commit that broke it.

Reviewed-by: Markus Armbruster 



Re: [Qemu-devel] [PATCH V19 2/7] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

2012-11-12 Thread Stefan Berger

On 11/08/2012 10:39 AM, Corey Bryant wrote:

Thanks for your responses.  I have a few comments below.

On 10/24/2012 02:46 PM, Stefan Berger wrote:

On 09/27/2012 10:22 AM, Corey Bryant wrote:



On 06/04/2012 03:37 PM, Stefan Berger wrote:



+/* check for ongoing seize by a higher locality */
+for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; 
l++) {
+if ((tis->loc[l].access & 
TPM_TIS_ACCESS_SEIZE)) {

+break;


Were you intending to break from the for loop or the while?



Right. I am setting a flag here now to then leave the while loop.



Are you setting the flag or testing it?  I'm not sure this code is 
serving any purpose the way it is, since it is testing the flag and 
then breaking from the for loop if it's on.  That's why I was 
wondering if you meant to break from the while loop instead.




Here's how the patch looks now:


+if ((val & TPM_TIS_ACCESS_SEIZE)) {
+/*
+ * allow seize if a locality is active and the requesting
+ * locality is higher than the one that's active
+ * OR
+ * allow seize for requesting locality if no locality is
+ * active
+ */
+while ((TPM_TIS_IS_VALID_LOCTY(tis->active_locty) &&
+locty > tis->active_locty) ||
+!TPM_TIS_IS_VALID_LOCTY(tis->active_locty)) {
+bool higher_seize = FALSE;
+
+/* already a pending SEIZE ? */
+if ((tis->loc[locty].access & TPM_TIS_ACCESS_SEIZE)) {
+break;
+}
+
+/* check for ongoing seize by a higher locality */
+for (l = locty + 1; l < TPM_TIS_NUM_LOCALITIES; l++) {
+if ((tis->loc[l].access & TPM_TIS_ACCESS_SEIZE)) {
+higher_seize = TRUE;
+break;
+}
+}
+
+if (higher_seize) {
+break;
+}
+
+/* cancel any seize by a lower locality */
+for (l = 0; l < locty - 1; l++) {
+tis->loc[l].access &= ~TPM_TIS_ACCESS_SEIZE;
+}
[...]



+case TPM_TIS_STATUS_EXECUTION:
+case TPM_TIS_STATUS_RECEPTION:
+/* abort currently running command */
+dprintf("tpm_tis: %s: Initiating abort.\n",
+__func__);
+tpm_tis_prep_abort(s, locty, locty);
+break;
+
+case TPM_TIS_STATUS_COMPLETION:


Does this path need to abort if TPM_STS_x.dataAvail is on? This
comment is based on "Table 19: State Transition Table." from the TPM
TIS document.



If TPM_TIS_STATUS_COMPLETION is the current state, then independent of
the TPM_TIS_STS_DATA_AVAILABLE flag the state transition is to idle
(states 30 and 37 in the spec). Following state 0.B in the spec, we
implement a TPM without idle state and so we transition to READY state
immediately. The data available flag should be reset, though.



Ok.  But row 30 in the table also says it aborts the command in the 
"Action Taken" column.




Row 30 describes that abort for while the TPM is in 'Completion' state, 
meaning the TPM has delivered the response from the TPM to the TIS and 
now an application can pick up the response. The abort in this case is 
achieved through changing the state and resetting the (response) buffer 
pointers so that the application will not receive more response bytes.


Regards,
Stefan




Re: [Qemu-devel] [PATCH V19 1/7] Support for TPM command line options

2012-11-12 Thread Stefan Berger

On 11/08/2012 10:52 AM, Corey Bryant wrote:



On 10/24/2012 03:06 PM, Stefan Berger wrote:

On 09/27/2012 10:12 AM, Corey Bryant wrote:



On 06/04/2012 03:37 PM, Stefan Berger wrote:



+if (!QLIST_EMPTY(&tpm_backends)) {
+error_report("Only one TPM is allowed.\n");
+return 1;
+}


A list of tpm_backends is maintained and walked in a few places, but
only one is allowed to be added to the list.  Will it ever make sense
to enable multiple backends at one time?



A list is also returned through the monitor. This list can at the moment
only have maximum of one entry. I would keep that list there unless
someone else opposes. It may be possible to create different types of
hardware emulation interfaces or simply replicate the TPM TIS at
different addresses. So I cannot say whether it will 'ever make sense'
to do that but I'd rather keep the opportunity there than close it and
with that also let the monitor return a list of items rather than a
single item.

I removed the processing of the lists in this part of the code at least.



Ok and it doesn't hurt to keep the list processing.  In that case you 
might as well keep the list processing code everywhere that you 
already have it.




I was only going to keep it in the monitor part now...
+ */

+int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+{
+QemuOpts *opts;
+
+if (strcmp("none", optarg) != 0) {


What's the point of supporting "-tpmdev none"?



Removed.



There must have been a reason you added it in the first place that I'm 
just not aware of.  Did someone else suggest adding it?


Not that I can remember.The option would have been useful if every VM by 
default had a TPM, similar to the physical world today, but it's 
unlikely that this will happen.



Regards,
Stefan




[Qemu-devel] KVM call agenda for 2012-11-12

2012-11-12 Thread Juan Quintela

Hi

Please send in any agenda topics you are interested in.

Later, Juan.



Re: [Qemu-devel] [PATCH] coroutine-sigaltstack.c: Use stack_t, not struct sigaltstack

2012-11-12 Thread Stefan Hajnoczi
On Sat, Nov 10, 2012 at 10:47 PM, Peter Maydell
 wrote:
> Use the POSIX-specified stack_t type as the argument to sigaltstack()
> rather than the legacy struct sigaltstack. This allows us to compile
> on MacOSX with --with-coroutine=sigaltstack.
>
> Signed-off-by: Peter Maydell 
> ---
> Tested on Linux and MacOSX.
>
>  coroutine-sigaltstack.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi 



Re: [Qemu-devel] [Bug 1077514] [NEW] *** buffer overflow detected ***: qemu-system-x86_64 terminated with nowait enabled

2012-11-12 Thread Paolo Bonzini
Il 12/11/2012 13:47, Stefan Hajnoczi ha scritto:
> > qemu-system-x86_64 -m 1024 -nographic -cpu coreduo -icount auto -hdachs 
> > 980,16,32 -kernel asa842-vmlinuz -initrd asa842-initrd.gz -append 
> > "ide_generic.probe_mask=0x01 ide_core.chs=0.0:980,16,32 auto nousb 
> > console=ttyS0,9600 bigphysarea=65536 no-hlt" -net nic -serial 
> > telnet::3020,server,nowait
> > failed to initialize KVM: Device or resource busy
> > Back to tcg accelerator.
> > QEMU 1.2.0 monitor - type 'help' for more information
> > (qemu) Warning: vlan 0 is not connected to host network
> > *** buffer overflow detected ***: qemu-system-x86_64 terminated
> 
> I'm unable to reproduce this on Fedora 18 with qemu.git/master.

Me neither.

Paolo



Re: [Qemu-devel] [Bug 1077514] [NEW] *** buffer overflow detected ***: qemu-system-x86_64 terminated with nowait enabled

2012-11-12 Thread Stefan Hajnoczi
On Sun, Nov 11, 2012 at 12:24 AM, Kaare Baastrup  wrote:
> Public bug reported:
>
> qemu-system-x86_64 -m 1024 -nographic -cpu coreduo -icount auto -hdachs 
> 980,16,32 -kernel asa842-vmlinuz -initrd asa842-initrd.gz -append 
> "ide_generic.probe_mask=0x01 ide_core.chs=0.0:980,16,32 auto nousb 
> console=ttyS0,9600 bigphysarea=65536 no-hlt" -net nic -serial 
> telnet::3020,server,nowait
> failed to initialize KVM: Device or resource busy
> Back to tcg accelerator.
> QEMU 1.2.0 monitor - type 'help' for more information
> (qemu) Warning: vlan 0 is not connected to host network
> *** buffer overflow detected ***: qemu-system-x86_64 terminated

I'm unable to reproduce this on Fedora 18 with qemu.git/master.

> === Backtrace: =
> /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7fd9f04b882c]
> /lib/x86_64-linux-gnu/libc.so.6(+0x109700)[0x7fd9f04b7700]
> /lib/x86_64-linux-gnu/libc.so.6(+0x10a7be)[0x7fd9f04b87be]
> qemu-system-x86_64(+0xf1b5d)[0x7fd9f4bb1b5d]
> qemu-system-x86_64(+0x18f148)[0x7fd9f4c4f148]
> qemu-system-x86_64(main+0xfe3)[0x7fd9f4b35353]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xed)[0x7fd9f03cf76d]
> qemu-system-x86_64(+0x796e9)[0x7fd9f4b396e9]

The backtrace is missing symbol information.  Please install the
debuginfo packages and repost the backtrace:
https://wiki.ubuntu.com/DebuggingProgramCrash#Debug_Symbol_Packages

Alternatively, please build qemu.git/master from source (it has symbol
information) and try to reproduce the bug.

Stefan



Re: [Qemu-devel] qemu-kvm not listed by command "virsh list --all"

2012-11-12 Thread Stefan Hajnoczi
On Mon, Nov 12, 2012 at 9:01 AM, Peter Cheung  wrote:
> Dear All
>Run a VM by qemu-kvm, and then i cannot list it by command "virsh list
> --all". Why?
> I am sure the VM is running because i can vnc to it.

Libvirt does not manage qemu-kvm processes that were started outside
its control by default.

If you want to do this, please look at the virsh qemu-attach  command.

Stefan



[Qemu-devel] [PATCH] aio: fix aio_ctx_prepare with idle bottom halves

2012-11-12 Thread Paolo Bonzini
Commit ed2aec4867f0d5f5de496bb765347b5d0cfe113d changed the return
value of aio_ctx_prepare from false to true when only idle bottom
halves are available.  This broke PC old-style DMA, which uses them.
Fix this by making aio_ctx_prepare return true only when non-idle
bottom halves are scheduled to run.

Reported-by: malc 
Signed-off-by: Paolo Bonzini 
---
Untested, I don't have an image that can do SB16.
malc, can you test and perhaps apply?

 async.c | 6 ++
 1 file modificato, 2 inserzioni(+), 4 rimozioni(-)

diff --git a/async.c b/async.c
index 04f9dcb..3f0e8f3 100644
--- a/async.c
+++ b/async.c
@@ -122,11 +122,9 @@ aio_ctx_prepare(GSource *source, gint*timeout)
 {
 AioContext *ctx = (AioContext *) source;
 QEMUBH *bh;
-bool scheduled = false;
 
 for (bh = ctx->first_bh; bh; bh = bh->next) {
 if (!bh->deleted && bh->scheduled) {
-scheduled = true;
 if (bh->idle) {
 /* idle bottom halves will be polled at least
  * every 10ms */
@@ -135,12 +133,12 @@ aio_ctx_prepare(GSource *source, gint*timeout)
 /* non-idle bottom halves will be executed
  * immediately */
 *timeout = 0;
-break;
+return true;
 }
 }
 }
 
-return scheduled;
+return false;
 }
 
 static gboolean
-- 
1.7.12.1




Re: [Qemu-devel] [PATCH] qemu-nbd: Initialise main loop earlier

2012-11-12 Thread Paolo Bonzini
Il 12/11/2012 13:23, Kevin Wolf ha scritto:
> Since the latest AIO changes qemu-nbd would segfault because
> bdrv_init() requires qemu_aio_context to be initialised.

Thanks... I had this in my NBD queue, going to send it out later.

Paolo

> Signed-off-by: Kevin Wolf 
> ---
>  qemu-nbd.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 15bcd08..80f08d8 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -539,6 +539,7 @@ int main(int argc, char **argv)
>  snprintf(sockpath, 128, SOCKET_PATH, basename(device));
>  }
>  
> +qemu_init_main_loop();
>  bdrv_init();
>  atexit(bdrv_close_all);
>  
> @@ -584,7 +585,6 @@ int main(int argc, char **argv)
>  memset(&client_thread, 0, sizeof(client_thread));
>  }
>  
> -qemu_init_main_loop();
>  qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
>   (void *)(uintptr_t)fd);
>  
> 




Re: [Qemu-devel] [PATCH] add mac address collision checking for device_add & pci_add

2012-11-12 Thread Stefan Hajnoczi
On Mon, Nov 12, 2012 at 12:49 PM, Lin Ma  wrote:
 Paolo Bonzini  11/12/12 7:27 PM >>>
>
> Il 12/11/2012 12:18, Daniel P. Berrange ha scritto:
>>> > QEMU doesn't check if there are mac collisions when adding nics.
>>> > It causes mac address collisions in guest if adding the nics which
>>> > include existing physical address.
>>> > This patch fixes the issue.
>> I understand the issue, but are there not use cases where it is
>> reasonable to have multiple NICs with the same MAC address ? To
>> me this kind of policy enforcement belongs at a higher level in
>> the mgmt stack.
>
I agree.
>
> Yes, Users won't intentionally add multiple NICs with the same MAC address.
> But what if there is typos in command-line or upper layer applications pass
> a conflicting mac option to qemu?
> We can't fully trust other applications to do this. So qemu shouldn't depend
> on other applications for the verifying mac.
> No matter upper layer applications verify mac or not, qemu should check the
> mac itself again to ensure the correctness.

It's perfectly okay to use the same MAC address on multiple cards from
a hardware emulation perspective (which is the scope of QEMU).

Higher level tools can perform this check if they want this policy.

Stefan



Re: [Qemu-devel] [PATCH] tap: reset vnet header size on open

2012-11-12 Thread Stefan Hajnoczi
On Mon, Nov 12, 2012 at 09:19:18AM +0200, Michael S. Tsirkin wrote:
> For tap, we currently assume the vnet header size is 10
> (the default value) but that might not be the case
> if tap is persistent and has been used by qemu previously.
> To fix, set vnet header size correctly on open.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
> 
> This fixes the issue reported by Alexander Graf on the kvm forum.
> Alexander, could you confirm please?
> Thanks,
> 
>  net/tap-linux.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index c6521be..3eaedc4 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -39,6 +39,7 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, 
> int vnet_hdr_required
>  {
>  struct ifreq ifr;
>  int fd, ret;
> +int len = sizeof(struct virtio_net_hdr);

We probably want the same behavior in the tap,fd= codepath.  How about a
call to tap_set_vnet_hdr_len() in net_init_tap() instead?

Stefan



  1   2   >