Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 02/05/2012 00:18, Anthony Liguori ha scritto:
> anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device
> rtl8139,?
> 

I don't think this is a fair comparison, or makes sense at all.  The cause
of the bug in master is a cut-and-paste typo:

@@ -157,7 +157,7 @@ int qdev_device_help(QemuOpts *opts)
   * for removal.  This conditional should be removed along with
   * it.
   */
-if (!prop->info->parse) {
+if (!prop->info->set) {
  continue;   /* no way to set it, don't show */
  }
  error_printf("%s.%s=%s\n", driver, prop->name,
@@ -165,7 +165,7 @@ int qdev_device_help(QemuOpts *opts)
  }
  if (info->bus_info) {
  for (prop = info->bus_info->props; prop&&  prop->name; prop++) {
-if (!prop->info->parse) {
+if (!prop->info->set) {
  continue;   /* no way to set it, don't show */
  }
  error_printf("%s.%s=%s\n", driver, prop->name,

while here the problem is due to a half-baked change in this series.

Since I redid the same change in my properties series, and I did it
correctly, the only sensible solution is to rebase these patches on
that one.

> So my series makes the situation better and I think it's easier to fix
> the full problem.  I also don't view the current bug as a -rc0 blocker
> (although it's obviously a release blocker).  I can send a proper patch
> later in the week but I'd still like to commit the bus changes before -rc0.

Please don't.  I have barely started reviewing this series and I have
quite a few questions, though some may be trivial.  In the meanwhile,
I'll separate the early pieces of my series and rebase this one on top.

Paolo

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[George Wilson]

Anthony Liguori  wrote on 05/01/2012 06:45:47 PM:

> Anthony Liguori 
> 05/01/2012 06:45 PM
>
> To
>
> George Wilson/Austin/IBM@IBMUS
>
> cc
>
> Paul Moore , qemu-devel@nongnu.org
>
> Subject
>
> Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication
> (security type 2) when in FIPS mode
>
> On 05/01/2012 06:43 PM, George Wilson wrote:
> >
> > Anthony Liguori  wrote on 05/01/2012 06:26:05
PM:
> >
> >> Anthony Liguori
> >> 05/01/2012 06:26 PM
> >>
> >> To
> >>
> >> Paul Moore
> >>
> >> cc
> >>
> >> qemu-devel@nongnu.org, George Wilson/Austin/IBM@IBMUS
> >>
> >> Subject
> >>
> >> Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication
> >> (security type 2) when in FIPS mode
> >>
> >> On 05/01/2012 04:20 PM, Paul Moore wrote:
> >>> FIPS 140-2 requires disabling certain ciphers, including DES, which
is
> > used
> >>> by VNC to obscure passwords when they are sent over the network.  The
> >>> solution for FIPS users is to disable the use of VNC password auth
when
> > the
> >>> host system is operating in FIPS mode.
> >>
> >> Sorry, what?
> >>
> >> Does FIPS really require software to detect when FIPS is enabled
> > andactively
> >> disable features???  That's absurd.
> >>
> >> Can you point to another software package that does something like
this?
> >
> > Yes, it's true that only FIPS-approved algorithms are permitted for use
in
> > FIPS
> > mode.  The kernel and all other FIPS 140-2 validated crypto modules
like
> > OpenSSL
> > and NSS are required to restrict algorithms to the approved set.  The
> > kernel
> > sets /proc/sys/crypto/fips_enabled so that programs can detect FIPS
mode
> > and
> > behave in accordance with the standard.
>
> But this is nonsensical. It would allow no-password to be configured
> for the VNC
> server but not DES?  Why is that okay?  It's not like we enable DES
> passwords by
> default.  A user has to explicitly configure it.

Because the standard says so :-)  If you're going to encrypt and need to
be FIPS 140-2 compliant, choose a FIPS-approved algorithm like AES.  And
adhere to approved key sizes and modes.  And make sure the algorithm does
self tests.  And so on.  It's best call into a FIPS-compliant library.
If the passwords are sent over an untrusted network, it's not OK not to
encrypt them from a security POV.

>
> Is there an open source app that actually keys off of fips_enabled?

libgcrypt is one example:

$strings /lib64/libgcrypt.so.11.5.3 | grep fips_enabled
/etc/gcrypt/fips_enabled
/proc/sys/crypto/fips_enabled

info libgcrypt has some details on FIPS mode.

>
> Regards,
>
> Anthony Liguori
>
> >
> >>
> >> Regards,
> >>
> >> Anthony Liguori
> >>
> >>>
> >>> This patch causes qemu to emits a syslog entry indicating that VNC
> > password
> >>> auth is disabled when it detects the host is running in FIPS mode,
and
> >>> unless a VNC password was specified on the command line it continues
> >>> normally.  However, if a VNC password was given on the command line,
> > qemu
> >>> fails with an error message to stderr explaining that that VNC
password
> >>> auth is not allowed in FIPS mode.
> >>>
> >>> Signed-off-by: Paul Moore
> >>> ---
> >>>qemu-doc.texi |8 +---
> >>>ui/vnc.c  |   32 
> >>>ui/vnc.h  |1 +
> >>>3 files changed, 38 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/qemu-doc.texi b/qemu-doc.texi
> >>> index e5d7ac4..f9b113e 100644
> >>> --- a/qemu-doc.texi
> >>> +++ b/qemu-doc.texi
> >>> @@ -1124,9 +1124,11 @@ the protocol limits passwords to 8
> >> characters it should not be considered
> >>>to provide high security. The password can be fairly easily
> >> brute-forced by
> >>>a client making repeat connections. For this reason, a VNC
> >> server using password
> >>>authentication should be restricted to only listen on the
> >> loopback interface
> >>> -or UNIX domain sockets. Password authentication is requested with
> >> the @code{password}
> >>> -option, and then once QEMU is running the password is set with
> >> the monitor. Until
> >>> -the monitor is used to set the password all clients will be
rejected.
> >>> +or UNIX domain sockets. Password authentication is not supported
> >> when operating
> >>> +in FIPS 140-2 compliance mode as it requires the use of the DES
> >> cipher. Password
> >>> +authentication is requested with the @code{password} option, and
> >> then once QEMU
> >>> +is running the password is set with the monitor. Until the
> >> monitor is used to
> >>> +set the password all clients will be rejected.
> >>>
> >>>@example
> >>>qemu [...OPTIONS...] -vnc :1,password -monitor stdio
> >>> diff --git a/ui/vnc.c b/ui/vnc.c
> >>> index deb9ecd..620791e 100644
> >>> --- a/ui/vnc.c
> >>> +++ b/ui/vnc.c
> >>> @@ -32,6 +32,7 @@
> >>>#include "acl.h"
> >>>#include "qemu-objects.h"
> >>>#include "qmp-commands.h"
> >>> +#include
> >>>
> >>>#define VNC_REFRESH_INTERVAL_BASE 30
> >>>#define VNC_REFRESH_INTERVAL_INC  50
> >>> @

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[George Wilson]

Anthony Liguori  wrote on 05/01/2012 06:26:05 PM:

> Anthony Liguori 
> 05/01/2012 06:26 PM
>
> To
>
> Paul Moore 
>
> cc
>
> qemu-devel@nongnu.org, George Wilson/Austin/IBM@IBMUS
>
> Subject
>
> Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication
> (security type 2) when in FIPS mode
>
> On 05/01/2012 04:20 PM, Paul Moore wrote:
> > FIPS 140-2 requires disabling certain ciphers, including DES, which is
used
> > by VNC to obscure passwords when they are sent over the network.  The
> > solution for FIPS users is to disable the use of VNC password auth when
the
> > host system is operating in FIPS mode.
>
> Sorry, what?
>
> Does FIPS really require software to detect when FIPS is enabled
andactively
> disable features???  That's absurd.
>
> Can you point to another software package that does something like this?

Yes, it's true that only FIPS-approved algorithms are permitted for use in
FIPS
mode.  The kernel and all other FIPS 140-2 validated crypto modules like
OpenSSL
and NSS are required to restrict algorithms to the approved set.  The
kernel
sets /proc/sys/crypto/fips_enabled so that programs can detect FIPS mode
and
behave in accordance with the standard.

>
> Regards,
>
> Anthony Liguori
>
> >
> > This patch causes qemu to emits a syslog entry indicating that VNC
password
> > auth is disabled when it detects the host is running in FIPS mode, and
> > unless a VNC password was specified on the command line it continues
> > normally.  However, if a VNC password was given on the command line,
qemu
> > fails with an error message to stderr explaining that that VNC password
> > auth is not allowed in FIPS mode.
> >
> > Signed-off-by: Paul Moore
> > ---
> >   qemu-doc.texi |8 +---
> >   ui/vnc.c  |   32 
> >   ui/vnc.h  |1 +
> >   3 files changed, 38 insertions(+), 3 deletions(-)
> >
> > diff --git a/qemu-doc.texi b/qemu-doc.texi
> > index e5d7ac4..f9b113e 100644
> > --- a/qemu-doc.texi
> > +++ b/qemu-doc.texi
> > @@ -1124,9 +1124,11 @@ the protocol limits passwords to 8
> characters it should not be considered
> >   to provide high security. The password can be fairly easily
> brute-forced by
> >   a client making repeat connections. For this reason, a VNC
> server using password
> >   authentication should be restricted to only listen on the
> loopback interface
> > -or UNIX domain sockets. Password authentication is requested with
> the @code{password}
> > -option, and then once QEMU is running the password is set with
> the monitor. Until
> > -the monitor is used to set the password all clients will be rejected.
> > +or UNIX domain sockets. Password authentication is not supported
> when operating
> > +in FIPS 140-2 compliance mode as it requires the use of the DES
> cipher. Password
> > +authentication is requested with the @code{password} option, and
> then once QEMU
> > +is running the password is set with the monitor. Until the
> monitor is used to
> > +set the password all clients will be rejected.
> >
> >   @example
> >   qemu [...OPTIONS...] -vnc :1,password -monitor stdio
> > diff --git a/ui/vnc.c b/ui/vnc.c
> > index deb9ecd..620791e 100644
> > --- a/ui/vnc.c
> > +++ b/ui/vnc.c
> > @@ -32,6 +32,7 @@
> >   #include "acl.h"
> >   #include "qemu-objects.h"
> >   #include "qmp-commands.h"
> > +#include
> >
> >   #define VNC_REFRESH_INTERVAL_BASE 30
> >   #define VNC_REFRESH_INTERVAL_INC  50
> > @@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
> >   static int vnc_cursor_define(VncState *vs);
> >   static void vnc_release_modifiers(VncState *vs);
> >
> > +static int fips_enabled(void)
> > +{
> > +int enabled = 0;
> > +char value;
> > +FILE *fds;
> > +
> > +fds = fopen("/proc/sys/crypto/fips_enabled", "r");
> > +if (fds == NULL) {
> > +return 0;
> > +}
> > +if (fread(&value, sizeof(value), 1, fds) == 1&&  value == '1') {
> > +enabled = 1;
> > +}
> > +fclose(fds);
> > +
> > +return enabled;
> > +}
> > +
> >   static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
> >   {
> >   #ifdef _VNC_DEBUG
> > @@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
> >   dcl->idle = 1;
> >   vnc_display = vs;
> >
> > +vs->fips = fips_enabled();
> > +VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
> > +if (vs->fips) {
> > +syslog(LOG_NOTICE, "Disabling VNC password auth due to
> FIPS mode\n");
> > +}
> > +
> >   vs->lsock = -1;
> >
> >   vs->ds = ds;
> > @@ -2892,6 +2917,13 @@ int vnc_display_open(DisplayState *ds,
> const char *display)
> >   while ((options = strchr(options, ','))) {
> >   options++;
> >   if (strncmp(options, "password", 8) == 0) {
> > +if (vs->fips) {
> > +fprintf(stderr,
> > +"VNC password auth disabled due to FIPS mode
\n");
> > +g_free(vs->display);
> > +vs->display = NULL;
> > +

Re: [Qemu-devel] [V6 PATCH 0/4] Send gratuitous packets by guest

[Jason Wang]

Hi Anthony:

Any more comments on the series?

Thanks

- Original Message -
> This an update of series that let guest and qemu to be co-operated to
> send gratuitous packets when needed (e.g after migration).
> 
> As it's hard for qemu to track the network configuration in guest
> such
> as bondings, vlans or ipv6s. Current gratuitous (RARP packets for
> primary mac address) may not work under those situations. The better
> method is to allow guest to send them when they can.
> 
> The series first introduce a model specific function in order to let
> nic models to use a device specific way to announce the link
> presence. With this, virtio-net backend were modified to notify the
> guest (through config update interrupt) and let guest send the
> gratuitous packet when needed.
> 
> The first user would be virtio-net.
> 
> Changes from V5:
> 
> - use a global variable to decide whether an announcement is needed
> after migration
> - align with virtio spec and let guest ack the announcement
> notification through control vq instead of config status writing
> 
> Changes from V4:
> 
> - keep the old behavior that send the gratuitous packets only after
>   migration
> - decide whether to send gratuitous packets by previous runstate
>   instead of a dedicated parameter
> - check virtio_net_started() instead of VIRTIO_NET_S_LINK_UP before
>   issue the config update interrupt
> - move VIRTIO_NET_S_ANNOUNCE to 0x100 and supress guest config write
> to RO bits
> - cleanups suggested by Michael
> 
> ---
> 
> Jason Wang (4):
>   net: announce self after vm start
>   net: model specific announcing support
>   virtio-net: notify guest to annouce itself
>   virtio-net: compat guest announce support
> 
> 
>  hw/pc_piix.c|   35 +++
>  hw/virtio-net.c |   29 +
>  hw/virtio-net.h |   14 ++
>  migration.c |2 +-
>  migration.h |2 ++
>  net.h   |2 ++
>  savevm.c|8 +---
>  vl.c|5 +
>  8 files changed, 93 insertions(+), 4 deletions(-)
> 
> --
> Jason Wang
> 
> 

[Qemu-devel] [PATCH V17 6/7] Introduce --enable-tpm-passthrough configure option

[Stefan Berger]

Introduce --enable-tpm-passthrough configure option.

Signed-off-by: Stefan Berger 
---
 configure |   16 +++-
 1 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 1b31811..c05d018 100755
--- a/configure
+++ b/configure
@@ -195,6 +195,7 @@ guest_agent="yes"
 libiscsi=""
 coroutine=""
 tpm="no"
+tpm_passthrough="no"
 
 # parse CC options first
 for opt do
@@ -827,11 +828,20 @@ for opt do
   ;;
   --enable-tpm) tpm="yes"
   ;;
+  --enable-tpm-passthrough) tpm_passthrough="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
 done
 
+if test "$tpm" = "no" ; then
+if test "$tpm_passthrough" = "yes"; then
+echo "ERROR: --enable-tpm-passthrough requires --enable-tpm"
+exit 1
+fi
+fi
+
 #
 # If cpu ~= sparc and  sparc_cpu hasn't been defined, plug in the right
 # QEMU_CFLAGS/LDFLAGS (assume sparc_v8plus for 32-bit and sparc_v9 for 64-bit)
@@ -1114,6 +1124,7 @@ echo "  --enable-guest-agent enable building of the 
QEMU Guest Agent"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "   gthread, ucontext, sigaltstack, windows"
 echo "  --enable-tpm enable TPM support"
+echo "  --enable-tpm-passthrough enable TPM passthrough driver"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -3013,6 +3024,7 @@ echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "coroutine backend $coroutine_backend"
 echo "TPM support   $tpm"
+echo "TPM passthrough   $tpm_passthrough"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3894,7 +3906,9 @@ fi
 if test "$tpm" = "yes"; then
   if test "$target_softmmu" = "yes" ; then
 if test "$linux" = "yes" ; then
-  echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
+  if test "$tpm_passthrough" = "yes" ; then
+echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
+  fi
 fi
 echo "CONFIG_TPM=y" >> $config_host_mak
   fi
-- 
1.7.6.5

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

[Stefan Berger]

This patch adds the main code of the TPM frontend driver, the TPM TIS
interface, to Qemu. The code is largely based on the previous implementation
for Xen but has been significantly extended to meet the standard's
requirements, such as the support for changing of localities and all the
functionality of the available flags.

Communication with the backend (i.e., for Xen or the libtpms-based one)
is cleanly separated through an interface which the backend driver needs
to implement.

The TPM TIS driver's backend was previously chosen in the code added
to arch_init. The frontend holds a pointer to the chosen backend (interface).

Communication with the backend is largely based on signals and conditions.
Whenever the frontend has collected a complete packet, it will signal
the backend, which then starts processing the command. Once the result
has been returned, the backend invokes a callback function
(tis_tpm_receive_cb()).

The one tricky part is support for VM suspend while the TPM is processing
a command. In this case the frontend driver is waiting for the backend
to return the result of the last command before shutting down. It waits
on a condition for a signal from the backend, which is delivered in
tis_tpm_receive_cb().

Testing the proper functioning of the different flags and localities
cannot be done from user space when running in Linux for example, since
access to the address space of the TPM TIS interface is not possible. Also
the Linux driver itself does not exercise all functionality. So, for
testing there is a fairly extensive test suite as part of the SeaBIOS patches
since from within the BIOS one can have full access to all the TPM's registers.


Signed-off-by: Stefan Berger 
---
 hw/tpm_tis.c |  822 ++
 1 files changed, 822 insertions(+), 0 deletions(-)
 create mode 100644 hw/tpm_tis.c

diff --git a/hw/tpm_tis.c b/hw/tpm_tis.c
new file mode 100644
index 000..02b9c2e
--- /dev/null
+++ b/hw/tpm_tis.c
@@ -0,0 +1,822 @@
+/*
+ * tpm_tis.c - QEMU's TPM TIS interface emulator
+ *
+ * Copyright (C) 2006,2010,2011 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger 
+ *  David Safford 
+ *
+ * Xen 4 support: Andrease Niederl 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputiggroup.org.
+ * In the developers menu choose the PC Client section then find the TIS
+ * specification.
+ */
+
+#include "tpm.h"
+#include "block.h"
+#include "exec-memory.h"
+#include "hw/hw.h"
+#include "hw/pc.h"
+#include "hw/pci_ids.h"
+#include "hw/tpm_tis.h"
+#include "qemu-error.h"
+#include "qemu-common.h"
+
+/*#define DEBUG_TIS */
+
+#ifdef DEBUG_TIS
+#define dprintf(fmt, ...) \
+do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#else
+#define dprintf(fmt, ...) \
+do { } while (0)
+#endif
+
+/* whether the STS interrupt is supported */
+/*#define RAISE_STS_IRQ */
+
+/* tis registers */
+#define TPM_TIS_REG_ACCESS0x00
+#define TPM_TIS_REG_INT_ENABLE0x08
+#define TPM_TIS_REG_INT_VECTOR0x0c
+#define TPM_TIS_REG_INT_STATUS0x10
+#define TPM_TIS_REG_INTF_CAPABILITY   0x14
+#define TPM_TIS_REG_STS   0x18
+#define TPM_TIS_REG_DATA_FIFO 0x24
+#define TPM_TIS_REG_DID_VID   0xf00
+#define TPM_TIS_REG_RID   0xf04
+
+#define TPM_TIS_STS_VALID (1 << 7)
+#define TPM_TIS_STS_COMMAND_READY (1 << 6)
+#define TPM_TIS_STS_TPM_GO(1 << 5)
+#define TPM_TIS_STS_DATA_AVAILABLE(1 << 4)
+#define TPM_TIS_STS_EXPECT(1 << 3)
+#define TPM_TIS_STS_RESPONSE_RETRY(1 << 1)
+
+#define TPM_TIS_ACCESS_TPM_REG_VALID_STS  (1 << 7)
+#define TPM_TIS_ACCESS_ACTIVE_LOCALITY(1 << 5)
+#define TPM_TIS_ACCESS_BEEN_SEIZED(1 << 4)
+#define TPM_TIS_ACCESS_SEIZE  (1 << 3)
+#define TPM_TIS_ACCESS_PENDING_REQUEST(1 << 2)
+#define TPM_TIS_ACCESS_REQUEST_USE(1 << 1)
+#define TPM_TIS_ACCESS_TPM_ESTABLISHMENT  (1 << 0)
+
+#define TPM_TIS_INT_ENABLED   (1 << 31)
+#define TPM_TIS_INT_DATA_AVAILABLE(1 << 0)
+#define TPM_TIS_INT_STS_VALID (1 << 1)
+#define TPM_TIS_INT_LOCALITY_CHANGED  (1 << 2)
+#define TPM_TIS_INT_COMMAND_READY (1 << 7)
+
+#ifndef RAISE_STS_IRQ
+
+#define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
+  TPM_TIS_INT_DATA_AVAILABLE   | \
+  TPM_TIS_INT_COMMAND_READY)
+
+#else
+
+#define TPM_TIS_INTERRUPTS_SUPPORTED (TPM_TIS_INT_LOCALITY_CHANGED | \
+  TPM_TIS_INT_DATA_AVAILABLE   | \
+  TPM_TIS_INT_STS_VALID | \
+  TPM_TIS_INT_COMMAND_READY)
+
+#endif
+
+#defi

[Qemu-devel] [PATCH V17 1/7] Support for TPM command line options

[Stefan Berger]

This patch adds support for TPM command line options.
The command line options supported here are

./qemu-... -tpmdev passthrough,path=,id=
   -device tpm-tis,tpmdev=

and

./qemu-... -tpmdev ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends (for example 'passthrough').

Using the type parameter, the backend is chosen, i.e., 'passthrough' for the
passthrough driver. The interpretation of the other parameters along
with determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'create' and return a TPMDriver structure if the VM can be started or 'NULL'
if not enough or bad parameters were provided.

Monitor support for 'info tpm' has been added. It for example prints the
following:

(qemu) info tpm
TPM devices:
 tpm0: model=tpm-tis
  \ tpm0: type=passthrough,path=/dev/tpm0

Signed-off-by: Stefan Berger 
---
 hmp-commands.hx  |2 +
 hmp.c|   28 +++
 hmp.h|1 +
 hw/tpm_tis.h |   78 
 monitor.c|8 ++
 qapi-schema.json |   29 
 qemu-config.c|   20 +
 qemu-options.hx  |   33 +
 tpm.c|  213 ++
 tpm.h|   81 +
 vl.c |   17 +
 11 files changed, 510 insertions(+), 0 deletions(-)
 create mode 100644 hw/tpm_tis.h
 create mode 100644 tpm.c
 create mode 100644 tpm.h

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 18cb415..08f6942 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1401,6 +1401,8 @@ show device tree
 show qdev device model list
 @item info roms
 show roms
+@item info tpm
+show the TPM device
 @end table
 ETEXI
 
diff --git a/hmp.c b/hmp.c
index eb96618..db42834 100644
--- a/hmp.c
+++ b/hmp.c
@@ -546,6 +546,34 @@ void hmp_info_block_jobs(Monitor *mon)
 }
 }
 
+void hmp_info_tpm(Monitor *mon)
+{
+TPMInfoList *info_list, *info;
+Error *err = NULL;
+unsigned int c = 0;
+
+info_list = qmp_query_tpm(&err);
+if (err) {
+monitor_printf(mon, "TPM device not supported\n");
+error_free(err);
+return;
+}
+
+monitor_printf(mon, "TPM device:\n");
+
+for (info = info_list; info; info = info->next) {
+TPMInfo *ti = info->value;
+monitor_printf(mon, " tpm%d: model=%s\n",
+   c, ti->model);
+monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
+   ti->id, ti->type,
+   ti->has_path ? ",path=" : "",
+   ti->has_path ? ti->path : "");
+c++;
+}
+qapi_free_TPMInfoList(info_list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
 monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 443b812..8e2a858 100644
--- a/hmp.h
+++ b/hmp.h
@@ -33,6 +33,7 @@ void hmp_info_spice(Monitor *mon);
 void hmp_info_balloon(Monitor *mon);
 void hmp_info_pci(Monitor *mon);
 void hmp_info_block_jobs(Monitor *mon);
+void hmp_info_tpm(Monitor *mon);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/hw/tpm_tis.h b/hw/tpm_tis.h
new file mode 100644
index 000..5e1f731
--- /dev/null
+++ b/hw/tpm_tis.h
@@ -0,0 +1,78 @@
+/*
+ * tpm_tis.c - QEMU's TPM TIS interface emulator
+ *
+ * Copyright (C) 2006,2010,2011 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger 
+ *  David Safford 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputiggroup.org
+ *
+ */
+#ifndef HW_TPM_TIS_H
+#define HW_TPM_TIS_H
+
+#include "isa.h"
+#include "qemu-common.h"
+
+#define TPM_TIS_ADDR_BASE   0xFED4
+
+#define TPM_TIS_NUM_LOCALITIES  5 /* per spec */
+#define TPM_TIS_LOCALITY_SHIFT  12
+#define TPM_TIS_NO_LOCALITY 0xff
+
+#define TPM_TIS_IS_VALID_LOCTY(x)   ((x) < TPM_TIS_NUM_LOCALITIES)
+
+#define TPM_TIS_IRQ 5
+
+#define TPM_TIS_BUFFER_MAX  4096
+
+
+typedef struct TPMSizedBuffer {
+uint32_t size;
+uint8_t  *buffer;
+} TPMSizedBuffer;
+
+typedef enum {
+TPM_TIS_STATUS_IDLE = 0,
+TPM_TIS_STATUS_READY,
+TPM_TIS_STATUS_COMPLETION,
+TPM_TIS_STATUS_EXECUTION,
+TPM_TIS_STATUS_RECEPTION,
+} TPMTISStatus;
+
+/* locality data  -- all fields are persisted */
+typedef struct TPMLocality {
+TPMTISStatus status;
+uint8_t access;
+uint8_t sts;
+uint32_t inte;
+uint32_t ints;
+
+uint16_t w_offset;
+uint16_t r_offset;
+TPMSizedBuffer w_buffer;
+TPMSizedBuffer r_buffer;
+} TPMLocality;
+
+typedef struct TPMTISState {
+QEMUBH *bh;
+uint32_t offset;
+uint8_t buf[TPM_TIS_BUFFER_MAX];
+
+uint8_t active_locty;
+uint8_t aborting_locty;
+uin

[Qemu-devel] [PATCH V17 5/7] Add a TPM Passthrough backend driver implementation

[Stefan Berger]

>From Andreas Niederl's original posting with adaptations where necessary:

This patch is based of off version 9 of Stefan Berger's patch series
  "Qemu Trusted Platform Module (TPM) integration"
and adds a new backend driver for it.

This patch adds a passthrough backend driver for passing commands sent to the
emulated TPM device directly to a TPM device opened on the host machine.

Thus it is possible to use a hardware TPM device in a system running on QEMU,
providing the ability to access a TPM in a special state (e.g. after a Trusted
Boot).

This functionality is being used in the acTvSM Trusted Virtualization Platform
which is available on [1].

Usage example:
  qemu-system-x86_64 -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \
 -device tpm-tis,tpmdev=tpm0 \
 -cdrom test.iso -boot d

Some notes about the host TPM:
The TPM needs to be enabled and activated. If that's not the case one
has to go through the BIOS/UEFI and enable and activate that TPM for TPM
commands to work as expected.
It may be necessary to boot the kernel using tpm_tis.force=1 in the boot
command line or 'modprobe tpm_tis force=1' in case of using it as a module.

Regards,
Andreas Niederl, Stefan Berger

[1] http://trustedjava.sourceforge.net/

Signed-off-by: Andreas Niederl 
Signed-off-by: Stefan Berger 
---
 Makefile.target  |3 +-
 configure|3 +
 hw/tpm_backend.c |   58 
 hw/tpm_backend.h |   43 ++
 hw/tpm_passthrough.c |  398 ++
 qemu-options.hx  |   38 +-
 tpm.c|   18 +++
 tpm.h|   33 
 vl.c |2 +
 9 files changed, 594 insertions(+), 2 deletions(-)
 create mode 100644 hw/tpm_backend.c
 create mode 100644 hw/tpm_backend.h
 create mode 100644 hw/tpm_passthrough.c

diff --git a/Makefile.target b/Makefile.target
index cdf108a..ef063c8 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -193,7 +193,8 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o cputlb.o
 obj-y += tpm.o
-obj-$(CONFIG_TPM) += tpm_tis.o
+obj-$(CONFIG_TPM) += tpm_tis.o tpm_backend.o
+obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o
diff --git a/configure b/configure
index 1773713..1b31811 100755
--- a/configure
+++ b/configure
@@ -3893,6 +3893,9 @@ fi
 
 if test "$tpm" = "yes"; then
   if test "$target_softmmu" = "yes" ; then
+if test "$linux" = "yes" ; then
+  echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak
+fi
 echo "CONFIG_TPM=y" >> $config_host_mak
   fi
 fi
diff --git a/hw/tpm_backend.c b/hw/tpm_backend.c
new file mode 100644
index 000..4cf0809
--- /dev/null
+++ b/hw/tpm_backend.c
@@ -0,0 +1,58 @@
+/*
+ *  common TPM backend driver functions
+ *
+ *  Copyright (c) 2012 IBM Corporation
+ *  Authors:
+ *Stefan Berger 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "tpm.h"
+#include "qemu-thread.h"
+#include "hw/tpm_backend.h"
+
+void tpm_backend_thread_deliver_request(TPMBackendThread *tbt)
+{
+   g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, NULL);
+}
+
+void tpm_backend_thread_create(TPMBackendThread *tbt,
+   GFunc func, gpointer user_data)
+{
+if (!tbt->pool) {
+tbt->pool = g_thread_pool_new(func, user_data, 1, TRUE, NULL);
+g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL);
+}
+}
+
+void tpm_backend_thread_end(TPMBackendThread *tbt)
+{
+if (tbt->pool) {
+g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_END, NULL);
+g_thread_pool_free(tbt->pool, FALSE, TRUE);
+tbt->pool = NULL;
+}
+}
+
+void tpm_backend_thread_tpm_reset(TPMBackendThread *tbt,
+  GFunc func, gpointer user_data)
+{
+if (!tbt->pool) {
+tpm_backend_thread_create(tbt, func, user_data);
+} else {
+g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_TPM_RESET,
+   NULL);
+}
+}
diff --git a/hw/tpm_backend.h b/hw/tpm_backend.h
new file mode 100644
index 000..f5fe198
--- /dev/null
+++ b/hw/tpm_backend.h
@@ -0,0 +1,43 @@
+/*
+ *  common TPM backend driver functions
+ *
+ *  Copyright (c) 2012 IBM Corporation
+ *  Authors:
+ *Stefan 

[Qemu-devel] [PATCH V17 7/7] Add fd parameter for TPM passthrough driver

[Stefan Berger]

Enable the passing of a file descriptor via fd=<..> to access the host's
TPM device using the TPM passthrough driver.

Signed-off-by: Stefan Berger 
---
 hmp.c|7 +-
 hw/tpm_passthrough.c |   57 +++--
 qapi-schema.json |4 ++-
 qemu-config.c|5 
 qemu-options.hx  |   11 +++--
 tpm.c|4 +++
 tpm.h|2 +
 7 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/hmp.c b/hmp.c
index db42834..e01a8d5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -565,10 +565,15 @@ void hmp_info_tpm(Monitor *mon)
 TPMInfo *ti = info->value;
 monitor_printf(mon, " tpm%d: model=%s\n",
c, ti->model);
-monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
+monitor_printf(mon, "  \\ %s: type=%s%s%s",
ti->id, ti->type,
ti->has_path ? ",path=" : "",
ti->has_path ? ti->path : "");
+if (ti->has_fd) {
+monitor_printf(mon, ",fd=%" PRId64,
+   ti->fd);
+}
+monitor_printf(mon, "\n");
 c++;
 }
 qapi_free_TPMInfoList(info_list);
diff --git a/hw/tpm_passthrough.c b/hw/tpm_passthrough.c
index 8914705..224fd25 100644
--- a/hw/tpm_passthrough.c
+++ b/hw/tpm_passthrough.c
@@ -305,26 +305,58 @@ static int tpm_passthrough_test_tpmdev(int fd)
 static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb)
 {
 const char *value;
+struct stat statbuf;
+size_t bufsize;
+
+value = qemu_opt_get(opts, "fd");
+if (value) {
+if (qemu_opt_get(opts, "path")) {
+error_report("fd= is invalid with path=");
+goto err_exit;
+}
 
-value = qemu_opt_get(opts, "path");
-if (!value) {
-value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
-}
+tb->s.tpm_pt->tpm_fd = qemu_parse_fd(value);
+if (tb->s.tpm_pt->tpm_fd < 0) {
+error_report("Illegal file descriptor for TPM device.\n");
+goto err_exit;
+}
+
+bufsize = sizeof(stringify(INT_MAX)) + 1;
+
+tb->has_tpm_fd = true;
+tb->tpm_fd = tb->s.tpm_pt->tpm_fd;
+} else {
+value = qemu_opt_get(opts, "path");
+if (!value) {
+value = TPM_PASSTHROUGH_DEFAULT_DEVICE;
+}
 
-tb->s.tpm_pt->tpm_dev = g_strdup(value);
+tb->s.tpm_pt->tpm_dev = g_strdup(value);
 
-tb->path = g_strdup(tb->s.tpm_pt->tpm_dev);
+tb->path = g_strdup(value);
+
+tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
+if (tb->s.tpm_pt->tpm_fd < 0) {
+error_report("Cannot access TPM device using '%s'.\n",
+ tb->s.tpm_pt->tpm_dev);
+goto err_free_parameters;
+}
+}
+
+if (fstat(tb->s.tpm_pt->tpm_fd, &statbuf) != 0) {
+error_report("Cannot determine file descriptor type for TPM "
+ "device: %s", strerror(errno));
+goto err_close_tpmdev;
+}
 
-tb->s.tpm_pt->tpm_fd = open(tb->s.tpm_pt->tpm_dev, O_RDWR);
-if (tb->s.tpm_pt->tpm_fd < 0) {
-error_report("Cannot access TPM device using '%s'.\n",
- tb->s.tpm_pt->tpm_dev);
+/* only allow character devices for now */
+if (!S_ISCHR(statbuf.st_mode)) {
+error_report("TPM file descriptor is not a character device");
 goto err_free_parameters;
 }
 
 if (tpm_passthrough_test_tpmdev(tb->s.tpm_pt->tpm_fd)) {
-error_report("'%s' is not a TPM device.\n",
- tb->s.tpm_pt->tpm_dev);
+error_report("Device is not a TPM.\n");
 goto err_close_tpmdev;
 }
 
@@ -341,6 +373,7 @@ static int tpm_passthrough_handle_device_opts(QemuOpts 
*opts, TPMBackend *tb)
 g_free(tb->s.tpm_pt->tpm_dev);
 tb->s.tpm_pt->tpm_dev = NULL;
 
+ err_exit:
 return 1;
 }
 
diff --git a/qapi-schema.json b/qapi-schema.json
index f302a36..7f2b945 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1742,10 +1742,12 @@
 #
 # @path: #optional Path to the TPM backend device
 #
+# @fd: #optional File descriptor for the TPM backend device
 # Since: 1.1
 ##
 { 'type': 'TPMInfo',
-  'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str' } }
+  'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str',
+   '*fd' : 'int' } }
 
 ##
 # @query-tpm
diff --git a/qemu-config.c b/qemu-config.c
index edc8d5d..a5e2677 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -628,6 +628,11 @@ static QemuOptsList qemu_tpmdev_opts = {
 .type = QEMU_OPT_STRING,
 .help = "Persistent storage for TPM state",
 },
+{
+.name = "fd",
+.type = QEMU_OPT_STRING,
+.help = "Filedescriptor for accessing the TPM",
+},
 { /* end of list */ }
 },
 };
diff --git a/qemu-options.hx b/qemu-options.hx
index b9920da..67

[Qemu-devel] [PATCH V17 0/7] Qemu Trusted Platform Module (TPM) integration

[Stefan Berger]

The following series of patches adds TPM (Trusted Platform Module) support
to Qemu. An emulator for the TIS (TPM Interface Spec) interface is
added that provides the basis for accessing a 'backend' implementing the actual
TPM functionality. The TIS emulator serves as a 'frontend' enabling for
example Linux's TPM TIS (tpm_tis) driver.

In this series I am posting a backend implementation that makes use of the
host's TPM through a passthrough driver, which on Linux is accessed
using /dev/tpm0.

v17:
 - applies to checkout of 6507470 (Apr 30)
 - split up path and fd into two optional parameters

v16:
 - applied to checkout of 42fe1c2 (Apr 27)
 - followed Anthony's suggestions for v15
 - changed qemu-options.hx and vl.c to not show anything TPM-related if
   --enable-tpm-passthrough was not used on configure line

v15:
 - applies to checkout of 8a22565 (Mar 27)
 - replacing g_malloc's with g_new; no more checks for NULL after allocs
 - introducing usage of bottom half in TIS frontend to deliver responses
 - get rid of locks since global lock is held by all threads entering TIS
   code
 - cleanups

v14:
 - applies to checkout of da5361c (Dec 12)
 - implemented Anthony Liguori's suggestions
 - dropping the version log on individual patches

v13:
 - applies to checkout of 61a5872 (Dec 12)
 - only allowing character devices as fd parameter
 - fixing error path in tpm_tis_init

v12:
 - applies to checkout of ebffe2a (Oct 11)
 - added documentation for fd parameter
 - nits

v11:
 - applies to checkout of 46f3069 (Sep 28)
 - some filing on the documentation
 - small nits fixed

v10:
 - applies to checkout of 1ce9ce6 (Sep 27)
 - addressed Michael Tsirkin's comments on v9

v9:
 - addressed Michael Tsirkin's and other reviewers' comments
 - only posting Andreas Niederl's passthrough driver as the backend driver

v8:
 - applies to checkout of f0fb8b7 (Aug 30)
 - fixing compilation error pointed out by Andreas Niederl
 - adding patch that allows to feed an initial state into the libtpms TPM
 - following memory API changes (glib) where necessary

v7:
 - applies to checkout of b9c6cbf (Aug 9)
 - measuring the modules if multiboot is used
 - coding style fixes

v6:
 - applies to checkout of 75ef849 (July 2nd)
 - some fixes and improvements to existing patches; see individual patches
 - added a patch with a null driver responding to all TPM requests with
   a response indicating failure; this backend has no dependencies and
   can alwayy be built;
 - added a patch to support the hashing of kernel, ramfs and command line
   if those were passed to Qemu using -kernel, -initrd and -append
   respectively. Measurements are taken, logged, and passed to SeaBIOS using
   the firmware interface.
 - libtpms revision 7 now requires 83kb of block storage due to having more
   NVRAM space

v5:
 - applies to checkout of 1fddfba1
 - adding support for split command line using the -tpmdev ... -device ...
   options while keeping the -tpm option
 - support for querying the device models using -tpm model=?
 - support for monitor 'info tpm'
 - adding documentation of command line options for man page and web page
 - increasing room for ACPI tables that qemu reserves to 128kb (from 64kb)
 - adding (experimental) support for block migration
 - adding (experimental) support for taking measurements when kernel,
   initrd and kernel command line are directly passed to Qemu

v4:
 - applies to checkout of d2d979c6
 - more coding style fixes
 - adding patch for supporting blob encryption (in addition to the existing
   QCoW2-level encryption)
   - this allows for graceful termination of a migration if the target
 is detected to have a wrong key
   - tested with big and little endian hosts
 - main thread releases mutex while checking for work to do on behalf of
   backend
 - introducing file locking (fcntl) on the block layer for serializing access
   to shared (QCoW2) files (used during migration)

v3:
 - Building a null driver at patch 5/8 that responds to all requests
   with an error response; subsequently this driver is transformed to the
   libtpms-based driver for real TPM functionality
 - Reworked the threading; dropped the patch for qemu_thread_join; the
   main thread synchronizing with the TPM thread termination may need
   to write data to the block storage while waiting for the thread to 
   terminate; did not previously show a problem but is safer
 - A lot of testing based on recent git checkout 4b4a72e5 (4/10):
   - migration of i686 VM from x86_64 host to i686 host to ppc64 host while
 running tests inside the VM
   - tests with S3 suspend/resume
   - tests with snapshots
   - multiple-hour tests with VM suspend/resume (using virsh save/restore)
 while running a TPM test suite inside the VM
   All tests passed; [not all of them were done on the ppc64 host]

v2:
 - splitting some of the patches into smaller ones for easier review
 - fixes in individual patches

Regards,
Stefan

Stefan Berger (7):
  Support for TPM command

[Qemu-devel] [PATCH V17 4/7] Build the TPM frontend code

[Stefan Berger]

Build the TPM frontend code that has been added so far.

Signed-off-by: Stefan Berger 
---
 Makefile.target |2 ++
 configure   |   11 +++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 1582904..cdf108a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -192,6 +192,8 @@ obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o cputlb.o
+obj-y += tpm.o
+obj-$(CONFIG_TPM) += tpm_tis.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o
diff --git a/configure b/configure
index 3c72fa0..1773713 100755
--- a/configure
+++ b/configure
@@ -194,6 +194,7 @@ zlib="yes"
 guest_agent="yes"
 libiscsi=""
 coroutine=""
+tpm="no"
 
 # parse CC options first
 for opt do
@@ -824,6 +825,8 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --enable-tpm) tpm="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1110,6 +1113,7 @@ echo "  --disable-guest-agentdisable building of the 
QEMU Guest Agent"
 echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "   gthread, ucontext, sigaltstack, windows"
+echo "  --enable-tpm enable TPM support"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -3008,6 +3012,7 @@ echo "OpenGL support$opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "coroutine backend $coroutine_backend"
+echo "TPM support   $tpm"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3886,6 +3891,12 @@ if test "$gprof" = "yes" ; then
   fi
 fi
 
+if test "$tpm" = "yes"; then
+  if test "$target_softmmu" = "yes" ; then
+echo "CONFIG_TPM=y" >> $config_host_mak
+  fi
+fi
+
 if test "$ARCH" = "tci"; then
   linker_script=""
 else
-- 
1.7.6.5

[Qemu-devel] [PATCH V17 3/7] Add a debug register

[Stefan Berger]

This patch uses the possibility to add a vendor-specific register and
adds a debug register useful for dumping the TIS's internal state. This
register is only active in a debug build (#define DEBUG_TIS).

Signed-off-by: Stefan Berger 
---
 hw/tpm_tis.c |   70 ++
 1 files changed, 70 insertions(+), 0 deletions(-)

diff --git a/hw/tpm_tis.c b/hw/tpm_tis.c
index 02b9c2e..5f8899d 100644
--- a/hw/tpm_tis.c
+++ b/hw/tpm_tis.c
@@ -52,6 +52,9 @@
 #define TPM_TIS_REG_DID_VID   0xf00
 #define TPM_TIS_REG_RID   0xf04
 
+/* vendor-specific registers */
+#define TPM_TIS_REG_DEBUG 0xf90
+
 #define TPM_TIS_STS_VALID (1 << 7)
 #define TPM_TIS_STS_COMMAND_READY (1 << 6)
 #define TPM_TIS_STS_TPM_GO(1 << 5)
@@ -97,6 +100,11 @@
 
 #define TPM_TIS_NO_DATA_BYTE  0xff
 
+/* local prototypes */
+
+static uint64_t tpm_tis_mmio_read(void *opaque, target_phys_addr_t addr,
+  unsigned size);
+
 /* utility functions */
 
 static uint8_t tpm_tis_locality_from_addr(target_phys_addr_t addr)
@@ -331,6 +339,63 @@ static uint32_t tpm_tis_data_read(TPMState *s, uint8_t 
locty)
 return ret;
 }
 
+#ifdef DEBUG_TIS
+static void tpm_tis_dump_state(void *opaque, target_phys_addr_t addr)
+{
+static const unsigned regs[] = {
+TPM_TIS_REG_ACCESS,
+TPM_TIS_REG_INT_ENABLE,
+TPM_TIS_REG_INT_VECTOR,
+TPM_TIS_REG_INT_STATUS,
+TPM_TIS_REG_INTF_CAPABILITY,
+TPM_TIS_REG_STS,
+TPM_TIS_REG_DID_VID,
+TPM_TIS_REG_RID,
+0xfff};
+int idx;
+uint8_t locty = tpm_tis_locality_from_addr(addr);
+target_phys_addr_t base = addr & ~0xfff;
+TPMState *s = opaque;
+TPMTISState *tis = &s->s.tis;
+
+dprintf("tpm_tis: active locality  : %d\n"
+"tpm_tis: state of locality %d : %d\n"
+"tpm_tis: register dump:\n",
+tis->active_locty,
+locty, tis->loc[locty].status);
+
+for (idx = 0; regs[idx] != 0xfff; idx++) {
+dprintf("tpm_tis: 0x%04x : 0x%08x\n", regs[idx],
+(uint32_t)tpm_tis_mmio_read(opaque, base + regs[idx], 4));
+}
+
+dprintf("tpm_tis: read offset   : %d\n"
+"tpm_tis: result buffer : ",
+tis->loc[locty].r_offset);
+for (idx = 0;
+ idx < tpm_tis_get_size_from_buffer(&tis->loc[locty].r_buffer);
+ idx++) {
+dprintf("%c%02x%s",
+tis->loc[locty].r_offset == idx ? '>' : ' ',
+tis->loc[locty].r_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+dprintf("\n"
+"tpm_tis: write offset  : %d\n"
+"tpm_tis: request buffer: ",
+tis->loc[locty].w_offset);
+for (idx = 0;
+ idx < tpm_tis_get_size_from_buffer(&tis->loc[locty].w_buffer);
+ idx++) {
+dprintf("%c%02x%s",
+tis->loc[locty].w_offset == idx ? '>' : ' ',
+tis->loc[locty].w_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+dprintf("\n");
+}
+#endif
+
 /*
  * Read a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -400,6 +465,11 @@ static uint64_t tpm_tis_mmio_read(void *opaque, 
target_phys_addr_t addr,
 case TPM_TIS_REG_RID:
 val = TPM_TIS_TPM_RID;
 break;
+#ifdef DEBUG_TIS
+case TPM_TIS_REG_DEBUG:
+tpm_tis_dump_state(opaque, addr);
+break;
+#endif
 }
 
 if (shift) {
-- 
1.7.6.5

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[Anthony Liguori]

On 05/01/2012 06:43 PM, George Wilson wrote:


Anthony Liguori  wrote on 05/01/2012 06:26:05 PM:


Anthony Liguori
05/01/2012 06:26 PM

To

Paul Moore

cc

qemu-devel@nongnu.org, George Wilson/Austin/IBM@IBMUS

Subject

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication
(security type 2) when in FIPS mode

On 05/01/2012 04:20 PM, Paul Moore wrote:

FIPS 140-2 requires disabling certain ciphers, including DES, which is

used

by VNC to obscure passwords when they are sent over the network.  The
solution for FIPS users is to disable the use of VNC password auth when

the

host system is operating in FIPS mode.


Sorry, what?

Does FIPS really require software to detect when FIPS is enabled

andactively

disable features???  That's absurd.

Can you point to another software package that does something like this?


Yes, it's true that only FIPS-approved algorithms are permitted for use in
FIPS
mode.  The kernel and all other FIPS 140-2 validated crypto modules like
OpenSSL
and NSS are required to restrict algorithms to the approved set.  The
kernel
sets /proc/sys/crypto/fips_enabled so that programs can detect FIPS mode
and
behave in accordance with the standard.


But this is nonsensical. It would allow no-password to be configured for the VNC 
server but not DES?  Why is that okay?  It's not like we enable DES passwords by 
default.  A user has to explicitly configure it.


Is there an open source app that actually keys off of fips_enabled?

Regards,

Anthony Liguori





Regards,

Anthony Liguori



This patch causes qemu to emits a syslog entry indicating that VNC

password

auth is disabled when it detects the host is running in FIPS mode, and
unless a VNC password was specified on the command line it continues
normally.  However, if a VNC password was given on the command line,

qemu

fails with an error message to stderr explaining that that VNC password
auth is not allowed in FIPS mode.

Signed-off-by: Paul Moore
---
   qemu-doc.texi |8 +---
   ui/vnc.c  |   32 
   ui/vnc.h  |1 +
   3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index e5d7ac4..f9b113e 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1124,9 +1124,11 @@ the protocol limits passwords to 8

characters it should not be considered

   to provide high security. The password can be fairly easily

brute-forced by

   a client making repeat connections. For this reason, a VNC

server using password

   authentication should be restricted to only listen on the

loopback interface

-or UNIX domain sockets. Password authentication is requested with

the @code{password}

-option, and then once QEMU is running the password is set with

the monitor. Until

-the monitor is used to set the password all clients will be rejected.
+or UNIX domain sockets. Password authentication is not supported

when operating

+in FIPS 140-2 compliance mode as it requires the use of the DES

cipher. Password

+authentication is requested with the @code{password} option, and

then once QEMU

+is running the password is set with the monitor. Until the

monitor is used to

+set the password all clients will be rejected.

   @example
   qemu [...OPTIONS...] -vnc :1,password -monitor stdio
diff --git a/ui/vnc.c b/ui/vnc.c
index deb9ecd..620791e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -32,6 +32,7 @@
   #include "acl.h"
   #include "qemu-objects.h"
   #include "qmp-commands.h"
+#include

   #define VNC_REFRESH_INTERVAL_BASE 30
   #define VNC_REFRESH_INTERVAL_INC  50
@@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
   static int vnc_cursor_define(VncState *vs);
   static void vnc_release_modifiers(VncState *vs);

+static int fips_enabled(void)
+{
+int enabled = 0;
+char value;
+FILE *fds;
+
+fds = fopen("/proc/sys/crypto/fips_enabled", "r");
+if (fds == NULL) {
+return 0;
+}
+if (fread(&value, sizeof(value), 1, fds) == 1&&   value == '1') {
+enabled = 1;
+}
+fclose(fds);
+
+return enabled;
+}
+
   static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
   {
   #ifdef _VNC_DEBUG
@@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
   dcl->idle = 1;
   vnc_display = vs;

+vs->fips = fips_enabled();
+VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
+if (vs->fips) {
+syslog(LOG_NOTICE, "Disabling VNC password auth due to

FIPS mode\n");

+}
+
   vs->lsock = -1;

   vs->ds = ds;
@@ -2892,6 +2917,13 @@ int vnc_display_open(DisplayState *ds,

const char *display)

   while ((options = strchr(options, ','))) {
   options++;
   if (strncmp(options, "password", 8) == 0) {
+if (vs->fips) {
+fprintf(stderr,
+"VNC password auth disabled due to FIPS mode

\n");

+g_free(vs->display);
+vs->display = NULL;
+return -1;
+ 

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

[Anthony Liguori]

On 04/30/2012 07:16 AM, Stefan Berger wrote:

This patch adds support for TPM command line options.
The command line options supported here are

./qemu-... -tpmdev passthrough,path=,id=
-device tpm-tis,tpmdev=

and

./qemu-... -tpmdev ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends (for example 'passthrough').

Using the type parameter, the backend is chosen, i.e., 'passthrough' for the
passthrough driver. The interpretation of the other parameters along
with determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'create' and return a TPMDriver structure if the VM can be started or 'NULL'
if not enough or bad parameters were provided.

Monitor support for 'info tpm' has been added. It for example prints the
following:

(qemu) info tpm
TPM devices:
  tpm0: model=tpm-tis
   \ tpm0: type=passthrough,path=/dev/tpm0

Signed-off-by: Stefan Berger
---
  hmp-commands.hx  |2 +
  hmp.c|   28 +++
  hmp.h|1 +
  hw/tpm_tis.h |   78 
  monitor.c|8 ++
  qapi-schema.json |   29 
  qemu-config.c|   20 +
  qemu-options.hx  |   33 +
  tpm.c|  213 ++
  tpm.h|   81 +
  vl.c |   17 +
  11 files changed, 510 insertions(+), 0 deletions(-)
  create mode 100644 hw/tpm_tis.h
  create mode 100644 tpm.c
  create mode 100644 tpm.h

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 18cb415..08f6942 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1401,6 +1401,8 @@ show device tree
  show qdev device model list
  @item info roms
  show roms
+@item info tpm
+show the TPM device
  @end table
  ETEXI

diff --git a/hmp.c b/hmp.c
index eb96618..7e130c5 100644
--- a/hmp.c
+++ b/hmp.c
@@ -546,6 +546,34 @@ void hmp_info_block_jobs(Monitor *mon)
  }
  }

+void hmp_info_tpm(Monitor *mon)
+{
+TPMInfoList *info_list, *info;
+Error *err = NULL;
+unsigned int c = 0;
+
+info_list = qmp_query_tpm(&err);
+if (err) {
+monitor_printf(mon, "TPM device not supported\n");
+error_free(err);
+return;
+}
+
+monitor_printf(mon, "TPM device:\n");
+
+for (info = info_list; info; info = info->next) {
+TPMInfo *ti = info->value;
+monitor_printf(mon, " tpm%d: model=%s\n",
+   c, ti->model);
+monitor_printf(mon, "  \\ %s: type=%s%s%s\n",
+   ti->id, ti->type,
+   ti->parameters ? "," : "",
+   ti->parameters ? ti->parameters : "");
+c++;
+}
+qapi_free_TPMInfoList(info_list);
+}
+
  void hmp_quit(Monitor *mon, const QDict *qdict)
  {
  monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 443b812..8e2a858 100644
--- a/hmp.h
+++ b/hmp.h
@@ -33,6 +33,7 @@ void hmp_info_spice(Monitor *mon);
  void hmp_info_balloon(Monitor *mon);
  void hmp_info_pci(Monitor *mon);
  void hmp_info_block_jobs(Monitor *mon);
+void hmp_info_tpm(Monitor *mon);
  void hmp_quit(Monitor *mon, const QDict *qdict);
  void hmp_stop(Monitor *mon, const QDict *qdict);
  void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/hw/tpm_tis.h b/hw/tpm_tis.h
new file mode 100644
index 000..5e1f731
--- /dev/null
+++ b/hw/tpm_tis.h
@@ -0,0 +1,78 @@
+/*
+ * tpm_tis.c - QEMU's TPM TIS interface emulator
+ *
+ * Copyright (C) 2006,2010,2011 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger
+ *  David Safford
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputiggroup.org
+ *
+ */
+#ifndef HW_TPM_TIS_H
+#define HW_TPM_TIS_H
+
+#include "isa.h"
+#include "qemu-common.h"
+
+#define TPM_TIS_ADDR_BASE   0xFED4
+
+#define TPM_TIS_NUM_LOCALITIES  5 /* per spec */
+#define TPM_TIS_LOCALITY_SHIFT  12
+#define TPM_TIS_NO_LOCALITY 0xff
+
+#define TPM_TIS_IS_VALID_LOCTY(x)   ((x)<  TPM_TIS_NUM_LOCALITIES)
+
+#define TPM_TIS_IRQ 5
+
+#define TPM_TIS_BUFFER_MAX  4096
+
+
+typedef struct TPMSizedBuffer {
+uint32_t size;
+uint8_t  *buffer;
+} TPMSizedBuffer;
+
+typedef enum {
+TPM_TIS_STATUS_IDLE = 0,
+TPM_TIS_STATUS_READY,
+TPM_TIS_STATUS_COMPLETION,
+TPM_TIS_STATUS_EXECUTION,
+TPM_TIS_STATUS_RECEPTION,
+} TPMTISStatus;
+
+/* locality data  -- all fields are persisted */
+typedef struct TPMLocality {
+TPMTISStatus status;
+uint8_t access;
+uint8_t sts;
+uint32_t inte;
+uint32_t ints;
+
+uint16_t w_offset;
+uint16_t r_offset;
+TPMSizedBuffer w_buffer;
+TPMSizedBuffer r_buffer;
+} TPMLocality;
+
+typedef struct TPMTISState {
+QEMUBH *bh;
+uint32_t offset;
+uint8_t buf[TPM_TI

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[Anthony Liguori]

On 05/01/2012 04:20 PM, Paul Moore wrote:

FIPS 140-2 requires disabling certain ciphers, including DES, which is used
by VNC to obscure passwords when they are sent over the network.  The
solution for FIPS users is to disable the use of VNC password auth when the
host system is operating in FIPS mode.


Sorry, what?

Does FIPS really require software to detect when FIPS is enabled and actively 
disable features???  That's absurd.


Can you point to another software package that does something like this?

Regards,

Anthony Liguori



This patch causes qemu to emits a syslog entry indicating that VNC password
auth is disabled when it detects the host is running in FIPS mode, and
unless a VNC password was specified on the command line it continues
normally.  However, if a VNC password was given on the command line, qemu
fails with an error message to stderr explaining that that VNC password
auth is not allowed in FIPS mode.

Signed-off-by: Paul Moore
---
  qemu-doc.texi |8 +---
  ui/vnc.c  |   32 
  ui/vnc.h  |1 +
  3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index e5d7ac4..f9b113e 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1124,9 +1124,11 @@ the protocol limits passwords to 8 characters it should 
not be considered
  to provide high security. The password can be fairly easily brute-forced by
  a client making repeat connections. For this reason, a VNC server using 
password
  authentication should be restricted to only listen on the loopback interface
-or UNIX domain sockets. Password authentication is requested with the 
@code{password}
-option, and then once QEMU is running the password is set with the monitor. 
Until
-the monitor is used to set the password all clients will be rejected.
+or UNIX domain sockets. Password authentication is not supported when operating
+in FIPS 140-2 compliance mode as it requires the use of the DES cipher. 
Password
+authentication is requested with the @code{password} option, and then once QEMU
+is running the password is set with the monitor. Until the monitor is used to
+set the password all clients will be rejected.

  @example
  qemu [...OPTIONS...] -vnc :1,password -monitor stdio
diff --git a/ui/vnc.c b/ui/vnc.c
index deb9ecd..620791e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -32,6 +32,7 @@
  #include "acl.h"
  #include "qemu-objects.h"
  #include "qmp-commands.h"
+#include

  #define VNC_REFRESH_INTERVAL_BASE 30
  #define VNC_REFRESH_INTERVAL_INC  50
@@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
  static int vnc_cursor_define(VncState *vs);
  static void vnc_release_modifiers(VncState *vs);

+static int fips_enabled(void)
+{
+int enabled = 0;
+char value;
+FILE *fds;
+
+fds = fopen("/proc/sys/crypto/fips_enabled", "r");
+if (fds == NULL) {
+return 0;
+}
+if (fread(&value, sizeof(value), 1, fds) == 1&&  value == '1') {
+enabled = 1;
+}
+fclose(fds);
+
+return enabled;
+}
+
  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
  {
  #ifdef _VNC_DEBUG
@@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
  dcl->idle = 1;
  vnc_display = vs;

+vs->fips = fips_enabled();
+VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
+if (vs->fips) {
+syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
+}
+
  vs->lsock = -1;

  vs->ds = ds;
@@ -2892,6 +2917,13 @@ int vnc_display_open(DisplayState *ds, const char 
*display)
  while ((options = strchr(options, ','))) {
  options++;
  if (strncmp(options, "password", 8) == 0) {
+if (vs->fips) {
+fprintf(stderr,
+"VNC password auth disabled due to FIPS mode\n");
+g_free(vs->display);
+vs->display = NULL;
+return -1;
+}
  password = 1; /* Require password auth */
  } else if (strncmp(options, "reverse", 7) == 0) {
  reverse = 1;
diff --git a/ui/vnc.h b/ui/vnc.h
index a851ebd..8746a98 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -160,6 +160,7 @@ struct VncDisplay
  char *display;
  char *password;
  time_t expires;
+int fips;
  int auth;
  bool lossy;
  bool non_adaptive;

Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[Andreas Färber]

Am 01.05.2012 23:20, schrieb Paul Moore:
> FIPS 140-2 requires disabling certain ciphers, including DES, which is used
> by VNC to obscure passwords when they are sent over the network.  The
> solution for FIPS users is to disable the use of VNC password auth when the
> host system is operating in FIPS mode.
> 
> This patch causes qemu to emits a syslog entry indicating that VNC password

"to emit"

> auth is disabled when it detects the host is running in FIPS mode, and
> unless a VNC password was specified on the command line it continues
> normally.  However, if a VNC password was given on the command line, qemu
> fails with an error message to stderr explaining that that VNC password

"explaining that VNC"

> auth is not allowed in FIPS mode.
> 
> Signed-off-by: Paul Moore 

Interesting feature. :)

> diff --git a/ui/vnc.c b/ui/vnc.c
> index deb9ecd..620791e 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -32,6 +32,7 @@
>  #include "acl.h"
>  #include "qemu-objects.h"
>  #include "qmp-commands.h"
> +#include 

syslog.h is POSIX, but it'll need a guard for mingw32.

> @@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
>  static int vnc_cursor_define(VncState *vs);
>  static void vnc_release_modifiers(VncState *vs);
>  
> +static int fips_enabled(void)
> +{
> +int enabled = 0;
> +char value;
> +FILE *fds;
> +
> +fds = fopen("/proc/sys/crypto/fips_enabled", "r");

How standardized is this? Should we limit this to __linux__ or something?

> +if (fds == NULL) {
> +return 0;
> +}
> +if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
> +enabled = 1;
> +}
> +fclose(fds);
> +
> +return enabled;
> +}

bool would seem nicer as return type and field type below.

Andreas

> +
>  static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
>  {
>  #ifdef _VNC_DEBUG

> diff --git a/ui/vnc.h b/ui/vnc.h
> index a851ebd..8746a98 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -160,6 +160,7 @@ struct VncDisplay
>  char *display;
>  char *password;
>  time_t expires;
> +int fips;
>  int auth;
>  bool lossy;
>  bool non_adaptive;

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

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Andreas Färber]

Am 02.05.2012 00:24, schrieb Anthony Liguori:
> On 05/01/2012 02:34 PM, Andreas Färber wrote:
>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
>>
>> Move macro to preceding patch?
> 
> Do you mean an independent patch?

Sorry, I reviewed the patches in mail reception order. ;)

I meant 08/14 (qdev: convert busses to QEMU object model) where macros
for other bus types were introduced. Seemed like an oversight.

Andreas

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

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Anthony Liguori]

On 05/01/2012 02:34 PM, Andreas Färber wrote:

Am 01.05.2012 20:18, schrieb Anthony Liguori:

It should have never been a bus method.

Signed-off-by: Anthony Liguori
---

[...]

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 4a468f8..5044018 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,22 +25,13 @@

  /* - */

-static char *idebus_get_fw_dev_path(DeviceState *dev);
-
  #define TYPE_IDE_BUS "IDE"
-
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
-BusClass *k = BUS_CLASS(klass);
-
-k->get_fw_dev_path = idebus_get_fw_dev_path;
-}
+#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)


Move macro to preceding patch?

Otherwise looks good.


Do you mean an independent patch?

Regards,

Anthony Liguori



/-F

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 05:18 PM, Andreas Färber wrote:

Am 01.05.2012 23:47, schrieb Paolo Bonzini:

I think it's too late for this series to go into 1.1.


In that case this is calling for a qom-next branch...


I really prefer to do this for 1.1.  We've got a large enough testing window and 
these patches have been around for a while.


Regards,

Anthony Liguori



Andreas

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 05:12 PM, Paolo Bonzini wrote:

Il 02/05/2012 00:01, Anthony Liguori ha scritto:


There's magic in the qdev layer now to decide whether a Property in the
array of properties becomes a legacy or static property (it's only ever
exposed as one type).


I don't think this is true: a legacy property always has a static
counterpart.  The magic (really just a fallback) is to decide whether
-device,=  will use the legacy property or a string
visitor + a static property.


Yes, I mispoke.  All legacy properties are static properties but not all static 
properties are legacy (IIUC).


The exception is PROP_PTR which is always a legacy but never a static.

So there is quite a bit of magic.

Regards,

Anthony Liguori



Paolo

Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Anthony Liguori]

On 05/01/2012 05:15 PM, Eric Blake wrote:

On 05/01/2012 03:53 PM, Anthony Liguori wrote:


I think (correct me if I'm wrong) libvirt should be aware of any file
that qemu
asks it to open. So from a security point of view, libvirt can prevent
opening a
file if it isn't affiliated with the guest.


Right, libvirt can maintain a whitelist of files QEMU is allowed to open
(which is already has because it needs to label these files).


Indeed.


  The only
complexity is that it's not a straight strcmp().  The path needs to be
(carefully) broken into components with '.' and '..' handled
appropriately.  But this shouldn't be that difficult to do.


Libvirt would probably canonicalize path names, both when sticking them
in the whitelist, and in validating the requests from qemu - agreed that
it's not difficult.

More importantly, libvirt needs to start tracking the backing chain of
any qcow2 or qed file as part of the domain XML; and operations like
'block-stream' would update not only the chain, but also the whitelist.
  In the drive-reopen case, this means that libvirt would have to be
careful when to change labeling


Would you give QEMU open access or change the way you label to only allow 
read/write access?  I think the later is probably the better approach.


So presumably, you'll need to adjust the sVirt policy too...

You'll need to detect if a file is on NFS too and figure out what the default 
label is that was given so you can build the rules correctly.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 04:47 PM, Paolo Bonzini wrote:

Il 01/05/2012 22:46, Anthony Liguori ha scritto:



So I think we can safely break it :-)


Does this work with compat properties set on a bus?


No :-(

This is pretty easy to fix though.  The attached should do the trick,
I'll update and send out.

   Even if it does,

perhaps it's better to avoid the cleverness and wait for my series that
moves bus properties to the appropriate abstract superclass.


This series does that FWIW (patch 6/14).


Yeah, it should---modulo bisectability of course.

It's a fairly different approach WRT my series (using
qdev_add_properties instead of klass->props).  In theory I like it, but
I fail to see right now whether it breaks "-device foo,?" somehow.  I
think it does:

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.mac=macaddr
rtl8139.vlan=vlan
rtl8139.netdev=netdev
rtl8139.bootindex=int32
rtl8139.addr=pci-devfn<<<  here start bus props
rtl8139.romfile=string
rtl8139.rombar=uint32
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

I think it's too late for this series to go into 1.1.


No, this all works as expected (well, almost):

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.vlan=vlan
rtl8139.addr=pci-devfn
rtl8139.romfile=string
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

But there is stuff missing...

The problem is that qdev_device_help() only prints out legacy properties and 
munges them to look the way they used to.  But when you refactored some of the 
legacy types to be non-legacy, it meant that the legacy- version disappeared and 
those options disappeared from -device ,?


So we need to fix this either way.  I think the simple solution is to store 
property info in a list instead of directly printing it.  Then we can walk the 
list and remove non-legacy properties for given legacy properties and munge the 
names in place.


But this is true even with master (but much, much worse):

anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device rtl8139,?


This is because we only look at DeviceClass::props which only contains what 
happens to only contain static properties for rtl8139.  We totally ignore static 
properties in current master.


So my series makes the situation better and I think it's easier to fix the full 
problem.  I also don't view the current bug as a -rc0 blocker (although it's 
obviously a release blocker).  I can send a proper patch later in the week but 
I'd still like to commit the bus changes before -rc0.


We have a month before 1.1.  I think that's plenty of time to address any fall 
out here..


Regards,

Anthony Liguori



Paolo

[Qemu-devel] [PATCH 2/2] Declare state directory in smb.conf

[Jan Kiszka]

From: Nikolaus Rath 

The smb.conf generated by the userspace networking does not include a state 
directory
directive. Samba therefore falls back to the default value. Since the user 
generally
does not have write access to this path, smbd immediately crashes.

The "state directory" option was added in Samba 3.4.0 (commit
http://gitweb.samba.org/?p=samba.git;a=commit;h=7b02e05eb64f3ffd7aa1cf027d10a7343c0da757).

This patch adds the missing option.

Signed-off-by: Nikolaus Rath 
Signed-off-by: Jan Kiszka 
---
 net/slirp.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index f49043b..96f5032 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -510,6 +510,7 @@ static int slirp_smb(SlirpState* s, const char 
*exported_dir,
 "socket address=127.0.0.1\n"
 "pid directory=%s\n"
 "lock directory=%s\n"
+"state directory=%s\n"
 "log file=%s/log.smbd\n"
 "smb passwd file=%s/smbpasswd\n"
 "security = share\n"
@@ -522,6 +523,7 @@ static int slirp_smb(SlirpState* s, const char 
*exported_dir,
 s->smb_dir,
 s->smb_dir,
 s->smb_dir,
+s->smb_dir,
 exported_dir
 );
 fclose(f);
-- 
1.7.3.4

[Qemu-devel] [PATCH 1/2] slirp: don't use "smb ports = 0" option

[Jan Kiszka]

From: Nikolaus Rath 

The "smb ports = 0" option causes recent samba versions to crash. It was
introduced in commit 15ef3e with log message "Samba 3 support".
However, a value of 0 has never been officially supported by smb and is
also not necessary: if stdin is a socket, smb does not try to listen on
any ports and uses just stdin. This is necessary to support inetd based
operation (otherwise smbd would always fail when called from inetd,
because inetd already listens on the SMB port). Since samba has
supported inetd operation since pre-3.x, it should be safe to rely on
this feature. I have tested it with Samba 3.6.4 -- communication works
fine, and smbd is not listening on any ports.

I suspect the "smb ports = 0" hack may have been introduced when someone
tested the qemu generated samba config from the command line with "smbd
-i" and found it to fail (because then stdin isn't a socket).

Signed-off-by: Nikolaus Rath 
Signed-off-by: Jan Kiszka 
---
 net/slirp.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 18e07ba..f49043b 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -507,7 +507,6 @@ static int slirp_smb(SlirpState* s, const char 
*exported_dir,
 fprintf(f,
 "[global]\n"
 "private dir=%s\n"
-"smb ports=0\n"
 "socket address=127.0.0.1\n"
 "pid directory=%s\n"
 "lock directory=%s\n"
-- 
1.7.3.4

[Qemu-devel] [PATCH 0/2] [PULL] slirp: Fixes for latest smbd versions

[Jan Kiszka]

The following changes since commit 65074706b9353bae7307fcfcbbf63a36f6896aa7:

  linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts (2012-05-01 23:44:43 
+0400)

are available in the git repository at:
  git://git.kiszka.org/qemu.git queues/slirp


CC: Nikolaus Rath 

Nikolaus Rath (2):
  slirp: don't use "smb ports = 0" option
  Declare state directory in smb.conf

 net/slirp.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Andreas Färber]

Am 01.05.2012 23:47, schrieb Paolo Bonzini:
> I think it's too late for this series to go into 1.1.

In that case this is calling for a qom-next branch...

Andreas

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

Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Eric Blake]

On 05/01/2012 03:53 PM, Anthony Liguori wrote:

>> I think (correct me if I'm wrong) libvirt should be aware of any file
>> that qemu
>> asks it to open. So from a security point of view, libvirt can prevent
>> opening a
>> file if it isn't affiliated with the guest.
> 
> Right, libvirt can maintain a whitelist of files QEMU is allowed to open
> (which is already has because it needs to label these files).

Indeed.

>  The only
> complexity is that it's not a straight strcmp().  The path needs to be
> (carefully) broken into components with '.' and '..' handled
> appropriately.  But this shouldn't be that difficult to do.

Libvirt would probably canonicalize path names, both when sticking them
in the whitelist, and in validating the requests from qemu - agreed that
it's not difficult.

More importantly, libvirt needs to start tracking the backing chain of
any qcow2 or qed file as part of the domain XML; and operations like
'block-stream' would update not only the chain, but also the whitelist.
 In the drive-reopen case, this means that libvirt would have to be
careful when to change labeling - provide access to the new files before
drive-reopen, then revoke access to files after drive-reopen completes.
 In other words, having the -open-hook-fd client pass a command to
libvirt at the time it is closing an fd would help libvirt know when
qemu has quit using a file, which might make it easier to revoke SELinux
labels at that time.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Paolo Bonzini]

Il 02/05/2012 00:01, Anthony Liguori ha scritto:
> 
> There's magic in the qdev layer now to decide whether a Property in the
> array of properties becomes a legacy or static property (it's only ever
> exposed as one type).

I don't think this is true: a legacy property always has a static
counterpart.  The magic (really just a fallback) is to decide whether
-device ,= will use the legacy property or a string
visitor + a static property.

Paolo

[Qemu-devel] [PATCH RESEND for-1.1] linux-user: Clean up interim solution for exit syscall

[Andreas Färber]

After all target CPUs have been QOM'ified, we no longer need an #ifdef
to switch between object_delete() and g_free() in NPTL thread exit.

Signed-off-by: Andreas Färber 
---
 linux-user/syscall.c |4 
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 7128618..801b8ed 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5045,11 +5045,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 NULL, NULL, 0);
   }
   thread_env = NULL;
-#ifdef ENV_GET_CPU
   object_delete(OBJECT(ENV_GET_CPU(cpu_env)));
-#else
-  g_free(cpu_env);
-#endif
   g_free(ts);
   pthread_exit(NULL);
   }
-- 
1.7.7

[Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode

[Paul Moore]

FIPS 140-2 requires disabling certain ciphers, including DES, which is used
by VNC to obscure passwords when they are sent over the network.  The
solution for FIPS users is to disable the use of VNC password auth when the
host system is operating in FIPS mode.

This patch causes qemu to emits a syslog entry indicating that VNC password
auth is disabled when it detects the host is running in FIPS mode, and
unless a VNC password was specified on the command line it continues
normally.  However, if a VNC password was given on the command line, qemu
fails with an error message to stderr explaining that that VNC password
auth is not allowed in FIPS mode.

Signed-off-by: Paul Moore 
---
 qemu-doc.texi |8 +---
 ui/vnc.c  |   32 
 ui/vnc.h  |1 +
 3 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/qemu-doc.texi b/qemu-doc.texi
index e5d7ac4..f9b113e 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1124,9 +1124,11 @@ the protocol limits passwords to 8 characters it should 
not be considered
 to provide high security. The password can be fairly easily brute-forced by
 a client making repeat connections. For this reason, a VNC server using 
password
 authentication should be restricted to only listen on the loopback interface
-or UNIX domain sockets. Password authentication is requested with the 
@code{password}
-option, and then once QEMU is running the password is set with the monitor. 
Until
-the monitor is used to set the password all clients will be rejected.
+or UNIX domain sockets. Password authentication is not supported when operating
+in FIPS 140-2 compliance mode as it requires the use of the DES cipher. 
Password
+authentication is requested with the @code{password} option, and then once QEMU
+is running the password is set with the monitor. Until the monitor is used to
+set the password all clients will be rejected.
 
 @example
 qemu [...OPTIONS...] -vnc :1,password -monitor stdio
diff --git a/ui/vnc.c b/ui/vnc.c
index deb9ecd..620791e 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -32,6 +32,7 @@
 #include "acl.h"
 #include "qemu-objects.h"
 #include "qmp-commands.h"
+#include 
 
 #define VNC_REFRESH_INTERVAL_BASE 30
 #define VNC_REFRESH_INTERVAL_INC  50
@@ -48,6 +49,24 @@ static DisplayChangeListener *dcl;
 static int vnc_cursor_define(VncState *vs);
 static void vnc_release_modifiers(VncState *vs);
 
+static int fips_enabled(void)
+{
+int enabled = 0;
+char value;
+FILE *fds;
+
+fds = fopen("/proc/sys/crypto/fips_enabled", "r");
+if (fds == NULL) {
+return 0;
+}
+if (fread(&value, sizeof(value), 1, fds) == 1 && value == '1') {
+enabled = 1;
+}
+fclose(fds);
+
+return enabled;
+}
+
 static void vnc_set_share_mode(VncState *vs, VncShareMode mode)
 {
 #ifdef _VNC_DEBUG
@@ -2748,6 +2767,12 @@ void vnc_display_init(DisplayState *ds)
 dcl->idle = 1;
 vnc_display = vs;
 
+vs->fips = fips_enabled();
+VNC_DEBUG("FIPS mode %s\n", (vs->fips ? "enabled" : "disabled"));
+if (vs->fips) {
+syslog(LOG_NOTICE, "Disabling VNC password auth due to FIPS mode\n");
+}
+
 vs->lsock = -1;
 
 vs->ds = ds;
@@ -2892,6 +2917,13 @@ int vnc_display_open(DisplayState *ds, const char 
*display)
 while ((options = strchr(options, ','))) {
 options++;
 if (strncmp(options, "password", 8) == 0) {
+if (vs->fips) {
+fprintf(stderr,
+"VNC password auth disabled due to FIPS mode\n");
+g_free(vs->display);
+vs->display = NULL;
+return -1;
+}
 password = 1; /* Require password auth */
 } else if (strncmp(options, "reverse", 7) == 0) {
 reverse = 1;
diff --git a/ui/vnc.h b/ui/vnc.h
index a851ebd..8746a98 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -160,6 +160,7 @@ struct VncDisplay
 char *display;
 char *password;
 time_t expires;
+int fips;
 int auth;
 bool lossy;
 bool non_adaptive;

Re: [Qemu-devel] [PATCH v2 0/2] QEMU copyright update

[Andreas Färber]

Am 27.04.2012 12:08, schrieb Andreas Färber:
> Hello,
> 
> For 1.0 I had brought up the issue that the copyright statement reads 2008.

Ping for 1.1-rc0.

/-F

> Mentor Graphics have solved this for their Sourcery CodeBench fork by 
> printing:
> Copyright (c) 2003-2008 Fabrice Bellard, 2008-2011 Mentor Graphics
> 
> For the QEMU community it's less clear whom to assign the copyright to,
> thus my proposal is the appendix "and contributors".
> 
> v2 applies this change to softmmus, linux-user and bsd-user (pointed out by 
> PMM).
> darwin-user is intentionally left out as there is a pending PULL for its 
> removal.
> 
> Also included is a change to the bsd-user version banner.
> 
> CC'ing libvirt since it has been sensitive to changes there in the past.
> 
> Regards,
> Andreas
> 
> Cc: Anthony Liguori 
> Cc: Fabrice Bellard 
> Cc: Peter Maydell 
> Cc: Natalia Portillo 
> Cc: libvirt 
> 
> Andreas Färber (2):
>   Update copyright banners
>   bsd-user: Output package version
> 
>  bsd-user/main.c   |3 ++-
>  linux-user/main.c |2 +-
>  vl.c  |3 ++-
>  3 files changed, 5 insertions(+), 3 deletions(-)

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

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 03:57 PM, Peter Maydell wrote:

On 1 May 2012 21:48, Anthony Liguori  wrote:

Legacy properties != static properties.

qdev_add_properties adds both legacy and static properties.  I'm happy to
put static properties into object but not legacy properties.


So, er, how are you defining legacy and static properties?
I would have thought 'legacy property' was 'anything we happen
to already have'...


No, per the naming convention Paolo introduced, it's roughly:

1) legacy properties can be get/set as strings using the qdev parsing methods. 
Via qdev, they're exposed as -device ,=.


In QOM, they're exposed as:

/path/to/device/legacy- = 

2) static properties are well-behaved QOM properties that just happen to be 
defined by the DEFINE_PROP_XXX macros.  They use the get/set methods of the 
property types.  New code can (and probably should) make use of static properties.


There's magic in the qdev layer now to decide whether a Property in the array of 
properties becomes a legacy or static property (it's only ever exposed as one 
type).  When we move static properties to Object, I'd like to keep legacy 
strictly as a qdev concept so we don't accidentally introduce more legacy 
properties.



Base classes are free to call object_property_add_static directly and avoid
qdev_add_properties entirely but we need the later for backwards compat.


If qdev_add_properties is purely for back-compat it should be
clearly labelled so we don't use it in new code.


Yes, I expect it would be once we have Paolo's property series in.  For now, 
it's the only real interface we have for adding bulk properties.


Regards,

Anthony Liguori



-- PMM

Re: [Qemu-devel] [PATCH v5 0/7] add fixed-width visitors and serialization tests/fixes

[Andreas Färber]

Am 27.04.2012 22:21, schrieb Michael Roth:
> These patches apply on top of qemu.git master, and can also be obtained from:
> git://github.com/mdroth/qemu.git visitor-fixed-width-v5

I've tested that branch by running some random guests without noticeable
problems and by testing X86CPU level/xlevel simplifications on top
(attached).

NOTE: There is a v6 patch with fixed commit message hidden as reply
within this series but there is no matching -v6 branch pushed yet.

That being said, v5 series

Tested-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
>From b19a05c8dff628af5f0170cc53c8319af0074104 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andreas=20F=C3=A4rber?= 
Date: Tue, 1 May 2012 23:33:13 +0200
Subject: [PATCH] target-i386: Use uint32 visitor for [x]level properties
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This simplifies the code and resolves TODOs.

Signed-off-by: Andreas Färber 
---
 target-i386/cpu.c |   42 --
 1 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 65d9af6..af8e1f3 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -715,66 +715,32 @@ static void x86_cpuid_get_level(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
-int64_t value;
 
-value = cpu->env.cpuid_level;
-/* TODO Use visit_type_uint32() once available */
-visit_type_int(v, &value, name, errp);
+visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
 }
 
 static void x86_cpuid_set_level(Object *obj, Visitor *v, void *opaque,
 const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
-const int64_t min = 0;
-const int64_t max = UINT32_MAX;
-int64_t value;
-
-/* TODO Use visit_type_uint32() once available */
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
-return;
-}
-if (value < min || value > max) {
-error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
-  name ? name : "null", value, min, max);
-return;
-}
 
-cpu->env.cpuid_level = value;
+visit_type_uint32(v, &cpu->env.cpuid_level, name, errp);
 }
 
 static void x86_cpuid_get_xlevel(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
-int64_t value;
 
-value = cpu->env.cpuid_xlevel;
-/* TODO Use visit_type_uint32() once available */
-visit_type_int(v, &value, name, errp);
+visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
 }
 
 static void x86_cpuid_set_xlevel(Object *obj, Visitor *v, void *opaque,
  const char *name, Error **errp)
 {
 X86CPU *cpu = X86_CPU(obj);
-const int64_t min = 0;
-const int64_t max = UINT32_MAX;
-int64_t value;
-
-/* TODO Use visit_type_uint32() once available */
-visit_type_int(v, &value, name, errp);
-if (error_is_set(errp)) {
-return;
-}
-if (value < min || value > max) {
-error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE, "",
-  name ? name : "null", value, min, max);
-return;
-}
 
-cpu->env.cpuid_xlevel = value;
+visit_type_uint32(v, &cpu->env.cpuid_xlevel, name, errp);
 }
 
 static char *x86_cpuid_get_vendor(Object *obj, Error **errp)
-- 
1.7.7

Re: [Qemu-devel] [PATCH 7/7] qdev: switch property accessors to fixed-width visitor interfaces

[Andreas Färber]

Am 27.04.2012 22:21, schrieb Michael Roth:
> Signed-off-by: Michael Roth 

Reviewed-by: Andreas Färber 

/-F

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

Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Anthony Liguori]

On 05/01/2012 04:45 PM, Corey Bryant wrote:



On 05/01/2012 04:25 PM, Anthony Liguori wrote:

Thanks for sending this out Stefan.

On 05/01/2012 10:31 AM, Stefan Hajnoczi wrote:

Libvirt can take advantage of SELinux to restrict the QEMU process and
prevent
it from opening files that it should not have access to. This improves
security because it prevents the attacker from escaping the QEMU
process if
they manage to gain control.

NFS has been a pain point for SELinux because it does not support
labels (which
I believe are stored in extended attributes). In other words, it's not
possible to use SELinux goodness on QEMU when image files are located
on NFS.
Today we have to allow QEMU access to any file on the NFS export
rather than
restricting specifically to the image files that the guest requires.

File descriptor passing is a solution to this problem and might also
come in
handy elsewhere. Libvirt or another external process chooses files
which QEMU
is allowed to access and provides just those file descriptors - QEMU
cannot
open the files itself.

This series adds the -open-hook-fd command-line option. Whenever QEMU
needs to
open an image file it sends a request over the given UNIX domain
socket. The
response includes the file descriptor or an errno on failure. Please
see the
patches for details on the protocol.

The -open-hook-fd approach allows QEMU to support file descriptor passing
without changing -drive. It also supports snapshot_blkdev and other
commands
that re-open image files.

Anthony Liguori wrote most of these patches. I
added a
demo -open-hook-fd server and added some small fixes. Since Anthony is
traveling right now I'm sending the RFC for discussion.


What I like about this approach is that it's useful outside the block
layer and is conceptionally simple from a QEMU PoV. We simply delegate
open() to libvirt and let libvirt enforce whatever rules it wants.

This is not meant to be an alternative to blockdev, but even with
blockdev, I think we still want to use a mechanism like this even with
blockdev.

Regards,

Anthony Liguori



I like it too and I think it's a better solution than the fd: protocol approach.

I think (correct me if I'm wrong) libvirt should be aware of any file that qemu
asks it to open. So from a security point of view, libvirt can prevent opening a
file if it isn't affiliated with the guest.


Right, libvirt can maintain a whitelist of files QEMU is allowed to open (which 
is already has because it needs to label these files).  The only complexity is 
that it's not a straight strcmp().  The path needs to be (carefully) broken into 
components with '.' and '..' handled appropriately.  But this shouldn't be that 
difficult to do.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 6/7] qdev: use int32_t container for devfn property

[Andreas Färber]

Am 27.04.2012 22:21, schrieb Michael Roth:
> Valid range for devfn is -1 to 255 (-1 for automatic assignment). We do
> not currently validate this due to devfn being stored as a uint32_t.
> This can lead to segfaults and other strange behavior.
> 
> We could technically just cast it to int32_t to implement the checking,
> but this will not work for visitor-based setting where we may do additional
> bounds-checking based on target container type, which is int32_t for this
> case.
> 
> Signed-off-by: Michael Roth 

Reviewed-by: Andreas Färber 

Upper limit matches my limited PCI knowledge; cc'ing mst.

/-F

> ---
>  hw/pci.c |2 +-
>  hw/pci.h |2 +-
>  hw/qdev-properties.c |   11 ---
>  hw/qdev.h|2 +-
>  4 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index b706e69..7818c9b 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -1538,7 +1538,7 @@ PCIDevice *pci_create_multifunction(PCIBus *bus, int 
> devfn, bool multifunction,
>  DeviceState *dev;
>  
>  dev = qdev_create(&bus->qbus, name);
> -qdev_prop_set_uint32(dev, "addr", devfn);
> +qdev_prop_set_int32(dev, "addr", devfn);
>  qdev_prop_set_bit(dev, "multifunction", multifunction);
>  return PCI_DEVICE(dev);
>  }
> diff --git a/hw/pci.h b/hw/pci.h
> index 8d0aa49..3bc9218 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -193,7 +193,7 @@ struct PCIDevice {
>  
>  /* the following fields are read only */
>  PCIBus *bus;
> -uint32_t devfn;
> +int32_t devfn;
>  char name[64];
>  PCIIORegion io_regions[PCI_NUM_REGIONS];
>  
> diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
> index 98dd06a..36d0aa0 100644
> --- a/hw/qdev-properties.c
> +++ b/hw/qdev-properties.c
> @@ -822,7 +822,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void 
> *opaque,
>  {
>  DeviceState *dev = DEVICE(obj);
>  Property *prop = opaque;
> -uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
> +int32_t *ptr = qdev_get_prop_ptr(dev, prop);
>  unsigned int slot, fn, n;
>  Error *local_err = NULL;
>  char *str = (char *)"";
> @@ -855,7 +855,7 @@ invalid:
>  
>  static int print_pci_devfn(DeviceState *dev, Property *prop, char *dest, 
> size_t len)
>  {
> -uint32_t *ptr = qdev_get_prop_ptr(dev, prop);
> +int32_t *ptr = qdev_get_prop_ptr(dev, prop);
>  
>  if (*ptr == -1) {
>  return snprintf(dest, len, "");
> @@ -870,11 +870,8 @@ PropertyInfo qdev_prop_pci_devfn = {
>  .print = print_pci_devfn,
>  .get   = get_int32,
>  .set   = set_pci_devfn,
> -/* FIXME: this should be -1...255, but the address is stored
> - * into an uint32_t rather than int32_t.
> - */
> -.min   = 0,
> -.max   = 0xULL,
> +.min   = -1,
> +.max   = 255,
>  };
>  
>  /* --- blocksize --- */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 4e90119..d07da45 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -267,7 +267,7 @@ extern PropertyInfo qdev_prop_blocksize;
>  #define DEFINE_PROP_HEX64(_n, _s, _f, _d)   \
>  DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t)
>  #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d)   \
> -DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, uint32_t)
> +DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t)
>  
>  #define DEFINE_PROP_PTR(_n, _s, _f) \
>  DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*)


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

Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Anthony Liguori]

On 05/01/2012 03:56 PM, Eric Blake wrote:

On 05/01/2012 02:25 PM, Anthony Liguori wrote:

Thanks for sending this out Stefan.


Indeed.



This series adds the -open-hook-fd command-line option.  Whenever QEMU
needs to
open an image file it sends a request over the given UNIX domain
socket.  The
response includes the file descriptor or an errno on failure.  Please
see the
patches for details on the protocol.

The -open-hook-fd approach allows QEMU to support file descriptor passing
without changing -drive.  It also supports snapshot_blkdev and other
commands
that re-open image files.

Anthony Liguori   wrote most of these patches.  I
added a
demo -open-hook-fd server and added some small fixes.  Since Anthony is
traveling right now I'm sending the RFC for discussion.


What I like about this approach is that it's useful outside the block
layer and is conceptionally simple from a QEMU PoV.  We simply delegate
open() to libvirt and let libvirt enforce whatever rules it wants.

This is not meant to be an alternative to blockdev, but even with
blockdev, I think we still want to use a mechanism like this even with
blockdev.


The overall series looks like it would be rather interesting.  What sort
of timing restrictions are there?  For example, the proposed
'drive-reopen' command (probably now delegated to qemu 1.2) would mean
that qemu would be calling back into libvirt in order to do the reopen.
  If libvirt takes its time in passing back an open fd, is it going to
starve qemu from answering unrelated monitor commands in the meantime?


s/libvirt/kernel/g and your concerns are equally valid.

Doing open() should never be done in a path that could block things.  There's 
always the possibility that we're on top of NFS and the open could timeout.


For something like drive_reopen, we should use an asynchronous open() that 
dispatched the open() in the posix-aio thread pool.


That's part of what's nice about this approach, we could still call file_open() 
in the posix-aio thread pool...



I definitely want to make sure we avoid deadlock where libvirt is
waiting on a monitor command, but the monitor command is waiting on
libvirt to pass an fd.

Is this also an opportunity to request whether a particular fd must be
seekable vs. acceptable as a one-pass read or write, perhaps by whether
the command is 1 (seekable open) or 2 (one-pass open)?


I'm not really sure where the distinction lies...

I want the RPC to behave exactly like open().  So if we're assuming that open() 
of a /dev/ file returns something that is ioctl()'able, then that's what libvirt 
should return.


If we want to sort of do fd-transformation where a special protocol is used for 
things like ioctl, that's fine, but it ought to be a different mechanism (that's 
probably not nearly as generic).



For example,
migration is one-pass (and therefore libvirt passes a pipe which is
hooked up to a helper app that uses O_DIRECT), while block devices must
be seekable.


But migration doesn't involve doing an open().  This is not a replacement for fd 
passing.  This is a replacement for open() to make up for the facts that (1) 
some management tools like libvirt cannot isolate guests with DAC and (2) 
SELinux cannot be used to isolate guests across all file systems.


I would really prefer that the kernel fix this problem for us, but from what I'm 
told, the problem lies in the NFS standards committee so short of forking the 
NFS protocol, there isn't much that the kernel can do.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 01/05/2012 22:46, Anthony Liguori ha scritto:
>>>
>>>
>>> So I think we can safely break it :-)
>>
>> Does this work with compat properties set on a bus?
> 
> No :-(
> 
> This is pretty easy to fix though.  The attached should do the trick,
> I'll update and send out.
> 
>   Even if it does,
>> perhaps it's better to avoid the cleverness and wait for my series that
>> moves bus properties to the appropriate abstract superclass.
> 
> This series does that FWIW (patch 6/14).

Yeah, it should---modulo bisectability of course.

It's a fairly different approach WRT my series (using
qdev_add_properties instead of klass->props).  In theory I like it, but
I fail to see right now whether it breaks "-device foo,?" somehow.  I
think it does:

$ qemu-system-x86_64 -device rtl8139,?
rtl8139.mac=macaddr
rtl8139.vlan=vlan
rtl8139.netdev=netdev
rtl8139.bootindex=int32
rtl8139.addr=pci-devfn   <<< here start bus props
rtl8139.romfile=string
rtl8139.rombar=uint32
rtl8139.multifunction=on/off
rtl8139.command_serr_enable=on/off

I think it's too late for this series to go into 1.1.

Paolo

Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Corey Bryant]

On 05/01/2012 04:25 PM, Anthony Liguori wrote:

Thanks for sending this out Stefan.

On 05/01/2012 10:31 AM, Stefan Hajnoczi wrote:

Libvirt can take advantage of SELinux to restrict the QEMU process and
prevent
it from opening files that it should not have access to. This improves
security because it prevents the attacker from escaping the QEMU
process if
they manage to gain control.

NFS has been a pain point for SELinux because it does not support
labels (which
I believe are stored in extended attributes). In other words, it's not
possible to use SELinux goodness on QEMU when image files are located
on NFS.
Today we have to allow QEMU access to any file on the NFS export
rather than
restricting specifically to the image files that the guest requires.

File descriptor passing is a solution to this problem and might also
come in
handy elsewhere. Libvirt or another external process chooses files
which QEMU
is allowed to access and provides just those file descriptors - QEMU
cannot
open the files itself.

This series adds the -open-hook-fd command-line option. Whenever QEMU
needs to
open an image file it sends a request over the given UNIX domain
socket. The
response includes the file descriptor or an errno on failure. Please
see the
patches for details on the protocol.

The -open-hook-fd approach allows QEMU to support file descriptor passing
without changing -drive. It also supports snapshot_blkdev and other
commands
that re-open image files.

Anthony Liguori wrote most of these patches. I
added a
demo -open-hook-fd server and added some small fixes. Since Anthony is
traveling right now I'm sending the RFC for discussion.


What I like about this approach is that it's useful outside the block
layer and is conceptionally simple from a QEMU PoV. We simply delegate
open() to libvirt and let libvirt enforce whatever rules it wants.

This is not meant to be an alternative to blockdev, but even with
blockdev, I think we still want to use a mechanism like this even with
blockdev.

Regards,

Anthony Liguori



I like it too and I think it's a better solution than the fd: protocol 
approach.


I think (correct me if I'm wrong) libvirt should be aware of any file 
that qemu asks it to open.  So from a security point of view, libvirt 
can prevent opening a file if it isn't affiliated with the guest.


--
Regards,
Corey



Anthony Liguori (3):
block: add open() wrapper that can be hooked by libvirt
block: add new command line parameter that and protocol description
block: plumb up open-hook-fd option

Stefan Hajnoczi (2):
osdep: add qemu_recvmsg() wrapper
Example -open-hook-fd server

block.c | 107 ++
block.h | 2 +
block/raw-posix.c | 18 +++
block/raw-win32.c | 2 +-
block/vdi.c | 2 +-
block/vmdk.c | 6 +--
block/vpc.c | 2 +-
block/vvfat.c | 4 +-
block_int.h | 12 +
osdep.c | 46 +
qemu-common.h | 2 +
qemu-options.hx | 42 +++
test-fd-passing.c | 147
+
vl.c | 3 ++
14 files changed, 378 insertions(+), 17 deletions(-)
create mode 100644 test-fd-passing.c

Re: [Qemu-devel] [PATCH v6 4/7] qapi: String visitor, use %f representation for floats

[Andreas Färber]

Am 30.04.2012 16:33, schrieb Michael Roth:
> Currently string-output-visitor formats floats as %g, which is nice in
> that trailing 0's are automatically truncated, but otherwise this causes
> some issues:
> 
>  - it uses 6 significant figures instead of 6 decimal places, which
>means something like 155777.5 (which even has an exact floating point
>representation) will be rounded to 155778 when converted to a string.
> 
>  - output will be presented in scientific notation when the normalized
>form requires a 10^x multiplier. Not a huge deal, but arguably less
>readable for command-line arguments.
> 
>  - due to using scientific notation for numbers requiring more than 6
>significant figures, instead of hard-defined decimal places, it
>fails a lot of the test-visitor-serialization unit tests for floats.
> 
> Instead, let's just use %f, which is what the QJSON and the QMP visitors
> use.
> 
> Signed-off-by: Michael Roth 

Reviewed-by: Andreas Färber 

/-F

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

Re: [Qemu-devel] [PATCH 1/7] qapi: add Visitor interfaces for uint*_t and int*_t

[Andreas Färber]

Am 27.04.2012 22:21, schrieb Michael Roth:
> This adds visitor interfaces for fixed-width integers types.
> Implementing these in visitors is optional, otherwise we fall back to
> visit_type_int() (int64_t) with some additional bounds checking to avoid
> integer overflows for cases where the value fetched exceeds the bounds
> of our target C type.
> 
> Signed-off-by: Michael Roth 

Reviewed-by: Andreas Färber 

/-F

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

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Andreas Färber]

Am 01.05.2012 20:50, schrieb Mark Cave-Ayland:
> On 01/05/12 08:10, Blue Swirl wrote:
>> The signal dma_enabled should be eventually replaced by a Pin.
> 
> And this is a sysbus concept, yes?

No, it'll be a general DeviceState concept.
SysBus is supposed to die out gradually.

Andreas

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

Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Andreas Färber]

Am 01.05.2012 22:51, schrieb Stefan Weil:
> Am 01.05.2012 21:50, schrieb Alexander Graf:
>> On my PPC host, HOST_LONG_SIZE is not defined even after
>> running configure. Use the normal C way of determining the
>> long size instead.
>>
>> Signed-off-by: Alexander Graf 
>> ---
>> thunk.h | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/thunk.h b/thunk.h
>> index 5be8f91..87025c3 100644
>> --- a/thunk.h
>> +++ b/thunk.h
>> @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype
>> *type_ptr, int is_host)
>> defined(HOST_PARISC) || defined(HOST_SPARC64)
>> return 4;
>> #elif defined(HOST_PPC)
>> - return HOST_LONG_SIZE;
>> + return sizeof(void *);
>> #else
>> return 2;
>> #endif
> 
> 
> Malc reverted his change and applied Alexander's previous patch,
> so this one is no longer needed (and would conflict with
> latest git master).

Since malc does seem to have committed Alex' exact patch, I would hope
that a PULL applies fine now, just like a rebase of a local branch onto
committed patch versions from master...

Andreas

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

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Andreas Färber]

Am 01.05.2012 22:37, schrieb Anthony Liguori:
> On 05/01/2012 02:05 PM, Andreas Färber wrote:
>> Am 01.05.2012 20:18, schrieb Anthony Liguori:
>>> This allows a base class to easily add properties.
>>>
>>> Signed-off-by: Anthony Liguori
>>
>> Implementation looks okay but /me not so happy with it: This conflicts
>> with the move of the qdev static property infrastructure from
>> DeviceState to Object.
>>
>> Consider rebasing this onto part of Paolo's series and call it
>> object_add_properties?
> 
> Eh?  There's nothing object_ about these properties and there's no way
> I'm willing to put legacy properties in object...
> 
> So I'm not quite sure what you're suggesting.

You just suggested to Peter using qdev_add_properties() in new QOM ARM
classes of his.

I'd rather not propagate using qdev_* functions in new QOM code because
either it remains forever or renaming becomes another touch-all series.

In Paolo's series the Property* concept is moved from qdev to QOM; thus
if it's in Object we usually use an object_ prefix.
Not too long ago you were willing to merge the large part of Paolo's
series which included this code movement, so if you don't want that
after all you should communicate that openly as a reply there. :)

Andreas

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

Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Eric Blake]

On 05/01/2012 02:25 PM, Anthony Liguori wrote:
> Thanks for sending this out Stefan.

Indeed.


>> This series adds the -open-hook-fd command-line option.  Whenever QEMU
>> needs to
>> open an image file it sends a request over the given UNIX domain
>> socket.  The
>> response includes the file descriptor or an errno on failure.  Please
>> see the
>> patches for details on the protocol.
>>
>> The -open-hook-fd approach allows QEMU to support file descriptor passing
>> without changing -drive.  It also supports snapshot_blkdev and other
>> commands
>> that re-open image files.
>>
>> Anthony Liguori  wrote most of these patches.  I
>> added a
>> demo -open-hook-fd server and added some small fixes.  Since Anthony is
>> traveling right now I'm sending the RFC for discussion.
> 
> What I like about this approach is that it's useful outside the block
> layer and is conceptionally simple from a QEMU PoV.  We simply delegate
> open() to libvirt and let libvirt enforce whatever rules it wants.
> 
> This is not meant to be an alternative to blockdev, but even with
> blockdev, I think we still want to use a mechanism like this even with
> blockdev.

The overall series looks like it would be rather interesting.  What sort
of timing restrictions are there?  For example, the proposed
'drive-reopen' command (probably now delegated to qemu 1.2) would mean
that qemu would be calling back into libvirt in order to do the reopen.
 If libvirt takes its time in passing back an open fd, is it going to
starve qemu from answering unrelated monitor commands in the meantime?
I definitely want to make sure we avoid deadlock where libvirt is
waiting on a monitor command, but the monitor command is waiting on
libvirt to pass an fd.

Is this also an opportunity to request whether a particular fd must be
seekable vs. acceptable as a one-pass read or write, perhaps by whether
the command is 1 (seekable open) or 2 (one-pass open)?  For example,
migration is one-pass (and therefore libvirt passes a pipe which is
hooked up to a helper app that uses O_DIRECT), while block devices must
be seekable.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Peter Maydell]

On 1 May 2012 21:48, Anthony Liguori  wrote:
> Legacy properties != static properties.
>
> qdev_add_properties adds both legacy and static properties.  I'm happy to
> put static properties into object but not legacy properties.

So, er, how are you defining legacy and static properties?
I would have thought 'legacy property' was 'anything we happen
to already have'...

> Base classes are free to call object_property_add_static directly and avoid
> qdev_add_properties entirely but we need the later for backwards compat.

If qdev_add_properties is purely for back-compat it should be
clearly labelled so we don't use it in new code.

-- PMM

Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Stefan Weil]

Am 01.05.2012 21:50, schrieb Alexander Graf:

On my PPC host, HOST_LONG_SIZE is not defined even after
running configure. Use the normal C way of determining the
long size instead.

Signed-off-by: Alexander Graf 
---
thunk.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/thunk.h b/thunk.h
index 5be8f91..87025c3 100644
--- a/thunk.h
+++ b/thunk.h
@@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype 
*type_ptr, int is_host)

defined(HOST_PARISC) || defined(HOST_SPARC64)
return 4;
#elif defined(HOST_PPC)
- return HOST_LONG_SIZE;
+ return sizeof(void *);
#else
return 2;
#endif



Malc reverted his change and applied Alexander's previous patch,
so this one is no longer needed (and would conflict with
latest git master).

Regards,

Stefan W.

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 03:43 PM, Andreas Färber wrote:

Am 01.05.2012 22:37, schrieb Anthony Liguori:

On 05/01/2012 02:05 PM, Andreas Färber wrote:

Am 01.05.2012 20:18, schrieb Anthony Liguori:

This allows a base class to easily add properties.

Signed-off-by: Anthony Liguori


Implementation looks okay but /me not so happy with it: This conflicts
with the move of the qdev static property infrastructure from
DeviceState to Object.

Consider rebasing this onto part of Paolo's series and call it
object_add_properties?


Eh?  There's nothing object_ about these properties and there's no way
I'm willing to put legacy properties in object...

So I'm not quite sure what you're suggesting.


You just suggested to Peter using qdev_add_properties() in new QOM ARM
classes of his.

I'd rather not propagate using qdev_* functions in new QOM code because
either it remains forever or renaming becomes another touch-all series.

In Paolo's series the Property* concept is moved from qdev to QOM; thus
if it's in Object we usually use an object_ prefix.
Not too long ago you were willing to merge the large part of Paolo's
series which included this code movement, so if you don't want that
after all you should communicate that openly as a reply there. :)


Legacy properties != static properties.

qdev_add_properties adds both legacy and static properties.  I'm happy to put 
static properties into object but not legacy properties.


So qdev_add_properties is going to stick around even after Paolo's changes.

Base classes are free to call object_property_add_static directly and avoid 
qdev_add_properties entirely but we need the later for backwards compat.


Regards,

Anthony Liguori



Andreas

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

On 05/01/2012 03:37 PM, Paolo Bonzini wrote:

Il 01/05/2012 20:18, Anthony Liguori ha scritto:

This is technically a compatibility breaker.  However:

1) libvirt does not rely on this (it always uses the driver name)

2) This behavior isn't actually documented anywhere (the docs just say driver).

3) I suspect there are less than three people on earth that even know this is
possible (minus the people reading this message).

So I think we can safely break it :-)


Does this work with compat properties set on a bus?


No :-(

This is pretty easy to fix though.  The attached should do the trick, I'll 
update and send out.


  Even if it does,

perhaps it's better to avoid the cleverness and wait for my series that
moves bus properties to the appropriate abstract superclass.


This series does that FWIW (patch 6/14).

Regards,

Anthony Liguori



Paolo



diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..be809db 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -378,7 +378,7 @@ static QEMUMachine pc_machine_v1_1 = {
 .property = "vapic",\
 .value= "off",\
 },{\
-.driver   = "USB",\
+.driver   = TYPE_USB_DEVICE,\
 .property = "full-path",\
 .value= "no",\
 }
@@ -451,7 +451,7 @@ static QEMUMachine pc_machine_v0_14 = {
 #define PC_COMPAT_0_13 \
 PC_COMPAT_0_14,\
 {\
-.driver   = "PCI",\
+.driver   = TYPE_PCI_DEVICE,\
 .property = "command_serr_enable",\
 .value= "off",\
 },{\
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 0c6dade..ddd7b25 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1158,10 +1158,14 @@ void qdev_prop_set_globals(DeviceState *dev)
 GlobalProperty *prop;
 
 QTAILQ_FOREACH(prop, &global_props, next) {
-if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0) {
+Object *obj;
+
+obj = object_dynamic_cast(OBJECT(dev), prop->driver);
+if (obj == NULL) {
 continue;
 }
-if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
+
+if (qdev_prop_parse(DEVICE(obj), prop->property, prop->value) != 0) {
 exit(1);
 }
 }

[Qemu-devel] [PULL 0/8] ppc patch queue 2012-05-01

[Alexander Graf]

Hi Anthony,

This is my current patch queue for ppc. Please pull.

Alex


The following changes since commit 42fe1c245f0239ebcdc084740a1777ac3699d071:
  Stefan Weil (1):
main-loop: Fix build for w32 and w64

are available in the git repository at:

  git://repo.or.cz/qemu/agraf.git ppc-for-upstream

Alexander Graf (2):
  PPC: Fix up e500 cache size setting
  linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

Bharat Bhushan (1):
  booke:Use MMU API for creating initial mapping for secondary cpus

David Gibson (2):
  pseries: Implement automatic PAPR VIO address allocation
  pseries: Use the same interrupt swizzling for host bridges as p2p bridges

François Revol (1):
  target-ppc: Some support for dumping TLB_EMB TLBs

Peter Portante (1):
  pseries: Fix use of global CPU state

Stefan Weil (1):
  ppce500_spin: Replace assert by hw_error (fixes compiler warning)

 hw/ppce500_spin.c   |3 +-
 hw/spapr.c  |7 ++---
 hw/spapr_hcall.c|2 +-
 hw/spapr_llan.c |5 +--
 hw/spapr_pci.c  |   49 ++
 hw/spapr_pci.h  |5 +--
 hw/spapr_vio.c  |   54 +++
 hw/spapr_vio.h  |   13 -
 hw/spapr_vscsi.c|5 +--
 hw/spapr_vty.c  |5 +--
 target-ppc/helper.c |   50 +++
 target-ppc/translate_init.c |   26 +++-
 thunk.h |2 +-
 13 files changed, 147 insertions(+), 79 deletions(-)

[Qemu-devel] [PATCH 7/8] ppce500_spin: Replace assert by hw_error (fixes compiler warning)

[Alexander Graf]

From: Stefan Weil 

The default case in function spin_read should never be reached,
therefore the old code used assert(0) to abort QEMU.

This does not work when QEMU is compiled with macro NDEBUG defined.
In this case (and also when the compiler does not know that assert
never returns), there is a compiler warning because of the missing
return value.

Using hw_error allows an improved error message and aborts always.

Signed-off-by: Stefan Weil 
[agraf: use __func__]
Signed-off-by: Alexander Graf 
---
 hw/ppce500_spin.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
index 95a2825..fddf219 100644
--- a/hw/ppce500_spin.c
+++ b/hw/ppce500_spin.c
@@ -179,7 +179,7 @@ static uint64_t spin_read(void *opaque, target_phys_addr_t 
addr, unsigned len)
 case 4:
 return ldl_p(spin_p);
 default:
-assert(0);
+hw_error("ppce500: unexpected %s with len = %u", __func__, len);
 }
 }
 
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 08/14] qdev: convert busses to QEMU Object Model

[Anthony Liguori]

On 05/01/2012 02:31 PM, Andreas Färber wrote:

Am 01.05.2012 20:18, schrieb Anthony Liguori:

This is far less interesting than it sounds.  We simply add an Object to each
BusInfo and then register the types appropriately.  Most of the interesting
refactoring will follow in the next patches.

Since we're changing fundamental type names (BusInfo ->  BusClass), it all needs
to convert at once.  Fortunately, not a lot of code is affected.

Signed-off-by: Anthony Liguori
---

[...]

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 9405f54..58497e5 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -29,16 +29,19 @@
  /* - */
  /* hda bus   */

-static struct BusInfo hda_codec_bus_info = {
-.name  = "HDA",
-.size  = sizeof(HDACodecBus),
+#define TYPE_HDA_BUS "HDA"


I stumbled over this being a pretty generic term for a type name.
"HDA-codec-bus" maybe?


It's unfortunately part of the ABI, so it cannot be changed :-(


diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index f5ee166..97673dd 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c

[...]

@@ -432,16 +433,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
  if (!bus) {
  return NULL;
  }
-if (bus->info != k->bus_info) {
+if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) != 0){


Space before {.


Ack.



  struct BusState {
+Object obj;
  DeviceState *parent;
-BusInfo *info;
  const char *name;
  int allow_hotplug;
  int qdev_allocated;


Indicate what is /*<  private>*/ and what not?


Ack.




@@ -321,7 +323,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, 
DeviceState *dev,
  char *qdev_get_fw_dev_path(DeviceState *dev);

  /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
-extern struct BusInfo system_bus_info;
+#define TYPE_SYSTEM_BUS "System"


"System-bus"?

Regarding the bus names, these have been merely moved here but I'm
raising the topic because they are now in the global name space where
they will potentially conflict with devices and other types.


The problem is they are already part of our existing ABI.  If we change it, it 
will break existing bus ids and qdev paths.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

On 05/01/2012 02:05 PM, Andreas Färber wrote:

Am 01.05.2012 20:18, schrieb Anthony Liguori:

This allows a base class to easily add properties.

Signed-off-by: Anthony Liguori


Implementation looks okay but /me not so happy with it: This conflicts
with the move of the qdev static property infrastructure from
DeviceState to Object.

Consider rebasing this onto part of Paolo's series and call it
object_add_properties?


Eh?  There's nothing object_ about these properties and there's no way I'm 
willing to put legacy properties in object...


So I'm not quite sure what you're suggesting.

Regards,

Anthony Liguori



Andreas


---
  hw/qdev.c |   25 -
  hw/qdev.h |2 ++
  2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..e17a9ab 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
  static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
   Error **errp);

-void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+void qdev_add_properties(DeviceState *dev, Property *props)
  {
  Property *prop;

+for (prop = props; prop&&  prop->name; prop++) {
+qdev_property_add_legacy(dev, prop, NULL);
+qdev_property_add_static(dev, prop, NULL);
+}
+qdev_prop_set_defaults(dev, props);
+}
+
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+{
  if (qdev_hotplug) {
  assert(bus->allow_hotplug);
  }

  dev->parent_bus = bus;
  QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
-
-for (prop = qdev_get_bus_info(dev)->props; prop&&  prop->name; prop++) {
-qdev_property_add_legacy(dev, prop, NULL);
-qdev_property_add_static(dev, prop, NULL);
-}
-qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
+qdev_add_properties(dev, dev->parent_bus->info->props);
  }

  /* Create a new device.  This only initializes the device state structure
@@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
  dev->instance_id_alias = -1;
  dev->state = DEV_STATE_CREATED;

-for (prop = qdev_get_props(dev); prop&&  prop->name; prop++) {
-qdev_property_add_legacy(dev, prop, NULL);
-qdev_property_add_static(dev, prop, NULL);
-}
-
+qdev_add_properties(dev, qdev_get_props(dev));
  object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
-qdev_prop_set_defaults(dev, qdev_get_props(dev));
  }

  /* Unlink device from bus and free the structure.  */
diff --git a/hw/qdev.h b/hw/qdev.h
index 4e90119..ca8386a 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);

  extern int qdev_hotplug;

+void qdev_add_properties(DeviceState *dev, Property *props);
+
  #endif

Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Paolo Bonzini]

Il 01/05/2012 20:18, Anthony Liguori ha scritto:
> This is technically a compatibility breaker.  However:
> 
> 1) libvirt does not rely on this (it always uses the driver name)
> 
> 2) This behavior isn't actually documented anywhere (the docs just say 
> driver).
> 
> 3) I suspect there are less than three people on earth that even know this is
>possible (minus the people reading this message).
> 
> So I think we can safely break it :-)

Does this work with compat properties set on a bus?  Even if it does,
perhaps it's better to avoid the cleverness and wait for my series that
moves bus properties to the appropriate abstract superclass.

Paolo

[Qemu-devel] [PATCH 5/8] pseries: Use the same interrupt swizzling for host bridges as p2p bridges

[Alexander Graf]

From: David Gibson 

Currently the pseries PCI code uses a somewhat strange scheme of PCI irq
allocation - one per slot up to a maximum that's greater than the usual 4.
This scheme more or less worked, because we were able to tell the guest the
irq mapping in the device tree, however it's a bit odd and may break
assumptions in the future.  Worse, the array used to construct the dev
tree interrupt map was mis-sized, we got away with it only because it
happened that our SPAPR_PCI_NUM_LSI value was greater than 7.

This patch changes the pseries PCI code to use the same interrupt swizzling
scheme as is standardized for PCI to PCI bridges.  This makes for better
consistency, deals better with any devices which use multiple interrupt
pins and will make life easier in the future when we add passthrough of
what may be either a host bridge or a PCI to PCI bridge.  This won't break
existing guests, because they don't assume a particular mapping scheme for
host bridges, but just follow what we tell them in the device tree (also
updated to match, of course).  This patch also fixes the allocation of the
irq map.

Signed-off-by: David Gibson 
Signed-off-by: Alexander Graf 
---
 hw/spapr_pci.c |   49 -
 hw/spapr_pci.h |5 ++---
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/hw/spapr_pci.c b/hw/spapr_pci.c
index a564c00..25b400a 100644
--- a/hw/spapr_pci.c
+++ b/hw/spapr_pci.c
@@ -198,16 +198,20 @@ static void rtas_write_pci_config(sPAPREnvironment *spapr,
 finish_write_pci_config(spapr, 0, addr, size, val, rets);
 }
 
+static int pci_spapr_swizzle(int slot, int pin)
+{
+return (slot + pin) % PCI_NUM_PINS;
+}
+
 static int pci_spapr_map_irq(PCIDevice *pci_dev, int irq_num)
 {
 /*
  * Here we need to convert pci_dev + irq_num to some unique value
- * which is less than number of IRQs on the specific bus (now it
- * is 16).  At the moment irq_num == device_id (number of the
- * slot?)
- * FIXME: we should swizzle in fn and irq_num
+ * which is less than number of IRQs on the specific bus (4).  We
+ * use standard PCI swizzling, that is (slot number + pin number)
+ * % 4.
  */
-return (pci_dev->devfn >> 3) % SPAPR_PCI_NUM_LSI;
+return pci_spapr_swizzle(PCI_SLOT(pci_dev->devfn), irq_num);
 }
 
 static void pci_spapr_set_irq(void *opaque, int irq_num, int level)
@@ -304,13 +308,13 @@ static int spapr_phb_init(SysBusDevice *s)
phb->busname ? phb->busname : phb->dtbusname,
pci_spapr_set_irq, pci_spapr_map_irq, phb,
&phb->memspace, &phb->iospace,
-   PCI_DEVFN(0, 0), SPAPR_PCI_NUM_LSI);
+   PCI_DEVFN(0, 0), PCI_NUM_PINS);
 phb->host_state.bus = bus;
 
 QLIST_INSERT_HEAD(&spapr->phbs, phb, list);
 
 /* Initialize the LSI table */
-for (i = 0; i < SPAPR_PCI_NUM_LSI; i++) {
+for (i = 0; i < PCI_NUM_PINS; i++) {
 qemu_irq qirq;
 uint32_t num;
 
@@ -392,8 +396,7 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
uint32_t xics_phandle,
void *fdt)
 {
-PCIBus *bus = phb->host_state.bus;
-int bus_off, i;
+int bus_off, i, j;
 char nodename[256];
 uint32_t bus_range[] = { cpu_to_be32(0), cpu_to_be32(0xff) };
 struct {
@@ -415,8 +418,8 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
 };
 uint64_t bus_reg[] = { cpu_to_be64(phb->buid), 0 };
 uint32_t interrupt_map_mask[] = {
-cpu_to_be32(b_d(-1)|b_fff(0)), 0x0, 0x0, 0x0};
-uint32_t interrupt_map[bus->nirq][7];
+cpu_to_be32(b_d(-1)|b_fff(0)), 0x0, 0x0, cpu_to_be32(-1)};
+uint32_t interrupt_map[PCI_SLOT_MAX * PCI_NUM_PINS][7];
 
 /* Start populating the FDT */
 sprintf(nodename, "pci@%" PRIx64, phb->buid);
@@ -450,19 +453,23 @@ int spapr_populate_pci_devices(sPAPRPHBState *phb,
  */
 _FDT(fdt_setprop(fdt, bus_off, "interrupt-map-mask",
  &interrupt_map_mask, sizeof(interrupt_map_mask)));
-for (i = 0; i < 7; i++) {
-uint32_t *irqmap = interrupt_map[i];
-irqmap[0] = cpu_to_be32(b_d(i)|b_fff(0));
-irqmap[1] = 0;
-irqmap[2] = 0;
-irqmap[3] = 0;
-irqmap[4] = cpu_to_be32(xics_phandle);
-irqmap[5] = cpu_to_be32(phb->lsi_table[i % SPAPR_PCI_NUM_LSI].dt_irq);
-irqmap[6] = cpu_to_be32(0x8);
+for (i = 0; i < PCI_SLOT_MAX; i++) {
+for (j = 0; j < PCI_NUM_PINS; j++) {
+uint32_t *irqmap = interrupt_map[i*PCI_NUM_PINS + j];
+int lsi_num = pci_spapr_swizzle(i, j);
+
+irqmap[0] = cpu_to_be32(b_d(i)|b_fff(0));
+irqmap[1] = 0;
+irqmap[2] = 0;
+irqmap[3] = cpu_to_be32(j+1);
+irqmap[4] = cpu_to_be32(xics_phandle);
+irqmap[5] = cpu_to_be32(phb->lsi_table[lsi_num].dt_irq);
+

[Qemu-devel] [PATCH 8/8] target-ppc: Some support for dumping TLB_EMB TLBs

[Alexander Graf]

From: François Revol 

Add mmubooke_dump_mmu().

TODO: Add printing of individual flags.

Signed-off-by: François Revol 
[agraf: fix coding style]
Signed-off-by: Alexander Graf 
---
 target-ppc/helper.c |   50 ++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index c610ce3..e97e496 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -1466,6 +1466,53 @@ static const char *book3e_tsize_to_str[32] = {
 "1T", "2T"
 };
 
+static void mmubooke_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
+ CPUPPCState *env)
+{
+ppcemb_tlb_t *entry;
+int i;
+
+if (kvm_enabled() && !env->kvm_sw_tlb) {
+cpu_fprintf(f, "Cannot access KVM TLB\n");
+return;
+}
+
+cpu_fprintf(f, "\nTLB:\n");
+cpu_fprintf(f, "Effective  Physical   Size PID   Prot "
+"Attr\n");
+
+entry = &env->tlb.tlbe[0];
+for (i = 0; i < env->nb_tlb; i++, entry++) {
+target_phys_addr_t ea, pa;
+target_ulong mask;
+uint64_t size = (uint64_t)entry->size;
+char size_buf[20];
+
+/* Check valid flag */
+if (!(entry->prot & PAGE_VALID)) {
+continue;
+}
+
+mask = ~(entry->size - 1);
+ea = entry->EPN & mask;
+pa = entry->RPN & mask;
+#if (TARGET_PHYS_ADDR_BITS >= 36)
+/* Extend the physical address to 36 bits */
+pa |= (target_phys_addr_t)(entry->RPN & 0xF) << 32;
+#endif
+size /= 1024;
+if (size >= 1024) {
+snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "M", size / 1024);
+} else {
+snprintf(size_buf, sizeof(size_buf), "%3" PRId64 "k", size);
+}
+cpu_fprintf(f, "0x%016" PRIx64 " 0x%016" PRIx64 " %s %-5u %08x %08x\n",
+(uint64_t)ea, (uint64_t)pa, size_buf, (uint32_t)entry->PID,
+entry->prot, entry->attr);
+}
+
+}
+
 static void mmubooke206_dump_one_tlb(FILE *f, fprintf_function cpu_fprintf,
  CPUPPCState *env, int tlbn, int offset,
  int tlbsize)
@@ -1561,6 +1608,9 @@ static void mmubooks_dump_mmu(FILE *f, fprintf_function 
cpu_fprintf,
 void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUPPCState *env)
 {
 switch (env->mmu_model) {
+case POWERPC_MMU_BOOKE:
+mmubooke_dump_mmu(f, cpu_fprintf, env);
+break;
 case POWERPC_MMU_BOOKE206:
 mmubooke206_dump_mmu(f, cpu_fprintf, env);
 break;
-- 
1.6.0.2

[Qemu-devel] [PULL 0/5] Tracing patches for 1.1

[Stefan Hajnoczi]

This pull request is for QEMU 1.1 and fixes tracetool Python compatibility
issues.  This makes it possible to run all the way back to Python 2.4.  These
patches are required so that QEMU can be built with tracing on older systems.

The following changes since commit b754e4fc1e8e68af975c545c38ebc3b001ebc98f:

  Remove stray HOST_LONG_SIZE (2012-05-01 18:23:04 +0400)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tracing

for you to fetch changes up to e120d449e1b39ec508c297b963ce452628dd37c3:

  configure: check for supported Python 2.x versions (2012-05-01 20:15:31 +0100)


Stefan Hajnoczi (5):
  tracetool: use Python 2.4-compatible exception handling syntax
  tracetool: use Python 2.4-compatible __import__() arguments
  tracetool: avoid str.rpartition() Python 2.5 function
  tracetool: avoid pkgutil.iter_modules() Python 2.7 function
  configure: check for supported Python 2.x versions

 configure |7 ---
 scripts/tracetool.py  |4 ++--
 scripts/tracetool/__init__.py |   19 +++
 scripts/tracetool/backend/__init__.py |8 ++--
 scripts/tracetool/format/__init__.py  |8 ++--
 5 files changed, 29 insertions(+), 17 deletions(-)

-- 
1.7.10

Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd

[Anthony Liguori]

Thanks for sending this out Stefan.

On 05/01/2012 10:31 AM, Stefan Hajnoczi wrote:

Libvirt can take advantage of SELinux to restrict the QEMU process and prevent
it from opening files that it should not have access to.  This improves
security because it prevents the attacker from escaping the QEMU process if
they manage to gain control.

NFS has been a pain point for SELinux because it does not support labels (which
I believe are stored in extended attributes).  In other words, it's not
possible to use SELinux goodness on QEMU when image files are located on NFS.
Today we have to allow QEMU access to any file on the NFS export rather than
restricting specifically to the image files that the guest requires.

File descriptor passing is a solution to this problem and might also come in
handy elsewhere.  Libvirt or another external process chooses files which QEMU
is allowed to access and provides just those file descriptors - QEMU cannot
open the files itself.

This series adds the -open-hook-fd command-line option.  Whenever QEMU needs to
open an image file it sends a request over the given UNIX domain socket.  The
response includes the file descriptor or an errno on failure.  Please see the
patches for details on the protocol.

The -open-hook-fd approach allows QEMU to support file descriptor passing
without changing -drive.  It also supports snapshot_blkdev and other commands
that re-open image files.

Anthony Liguori  wrote most of these patches.  I added a
demo -open-hook-fd server and added some small fixes.  Since Anthony is
traveling right now I'm sending the RFC for discussion.


What I like about this approach is that it's useful outside the block layer and 
is conceptionally simple from a QEMU PoV.  We simply delegate open() to libvirt 
and let libvirt enforce whatever rules it wants.


This is not meant to be an alternative to blockdev, but even with blockdev, I 
think we still want to use a mechanism like this even with blockdev.


Regards,

Anthony Liguori



Anthony Liguori (3):
   block: add open() wrapper that can be hooked by libvirt
   block: add new command line parameter that and protocol description
   block: plumb up open-hook-fd option

Stefan Hajnoczi (2):
   osdep: add qemu_recvmsg() wrapper
   Example -open-hook-fd server

  block.c   |  107 ++
  block.h   |2 +
  block/raw-posix.c |   18 +++
  block/raw-win32.c |2 +-
  block/vdi.c   |2 +-
  block/vmdk.c  |6 +--
  block/vpc.c   |2 +-
  block/vvfat.c |4 +-
  block_int.h   |   12 +
  osdep.c   |   46 +
  qemu-common.h |2 +
  qemu-options.hx   |   42 +++
  test-fd-passing.c |  147 +
  vl.c  |3 ++
  14 files changed, 378 insertions(+), 17 deletions(-)
  create mode 100644 test-fd-passing.c

[Qemu-devel] [PATCH 1/8] booke:Use MMU API for creating initial mapping for secondary cpus

[Alexander Graf]

From: Bharat Bhushan 

Initial Mapping creation for secondary CPU in SMP was missing new MMU API.

Signed-off-by: Bharat Bhushan 
Signed-off-by: Alexander Graf 
---
 hw/ppce500_spin.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/ppce500_spin.c b/hw/ppce500_spin.c
index 960b7b0..95a2825 100644
--- a/hw/ppce500_spin.c
+++ b/hw/ppce500_spin.c
@@ -86,6 +86,7 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
 tlb->mas2 = (va & TARGET_PAGE_MASK) | MAS2_M;
 tlb->mas7_3 = pa & TARGET_PAGE_MASK;
 tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
+env->tlb_dirty = true;
 }
 
 static void spin_kick(void *data)
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Alexander Graf]

On my PPC host, HOST_LONG_SIZE is not defined even after
running configure. Use the normal C way of determining the
long size instead.

Signed-off-by: Alexander Graf 
---
 thunk.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/thunk.h b/thunk.h
index 5be8f91..87025c3 100644
--- a/thunk.h
+++ b/thunk.h
@@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype *type_ptr, 
int is_host)
   defined(HOST_PARISC) || defined(HOST_SPARC64)
 return 4;
 #elif defined(HOST_PPC)
-return HOST_LONG_SIZE;
+return sizeof(void *);
 #else
 return 2;
 #endif
-- 
1.6.0.2

[Qemu-devel] [PATCH 2/5] tracetool: use Python 2.4-compatible __import__() arguments

[Stefan Hajnoczi]

In Python 2.5 keyword arguments were added to __import__().  Avoid using
them to achieve Python 2.4 compatibility.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Lluís Vilanova 
---
 scripts/tracetool/__init__.py |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 74fe21b..49858c9 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -204,7 +204,7 @@ def try_import(mod_name, attr_name = None, attr_default = 
None):
 object or attribute value.
 """
 try:
-module = __import__(mod_name, fromlist=["__package__"])
+module = __import__(mod_name, globals(), locals(), ["__package__"])
 if attr_name is None:
 return True, module
 return True, getattr(module, str(attr_name), attr_default)
-- 
1.7.10

[Qemu-devel] [PATCH 3/5] tracetool: avoid str.rpartition() Python 2.5 function

[Stefan Hajnoczi]

The str.rpartition() function is related to str.split() and is used for
splitting strings.  It was introduced in Python 2.5 and therefore cannot
be used in tracetool as Python 2.4 compatibility is required.

Replace the code using str.rsplit().

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Lluís Vilanova 
---
 scripts/tracetool/__init__.py |   17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 49858c9..175df08 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -64,14 +64,17 @@ class Arguments:
 res = []
 for arg in arg_str.split(","):
 arg = arg.strip()
-parts = arg.split()
-head, sep, tail = parts[-1].rpartition("*")
-parts = parts[:-1]
-if tail == "void":
-assert len(parts) == 0 and sep == ""
+if arg == 'void':
 continue
-arg_type = " ".join(parts + [ " ".join([head, sep]).strip() 
]).strip()
-res.append((arg_type, tail))
+
+if '*' in arg:
+arg_type, identifier = arg.rsplit('*', 1)
+arg_type += '*'
+identifier = identifier.strip()
+else:
+arg_type, identifier = arg.rsplit(None, 1)
+
+res.append((arg_type, identifier))
 return Arguments(res)
 
 def __iter__(self):
-- 
1.7.10

[Qemu-devel] [PATCH 2/8] PPC: Fix up e500 cache size setting

[Alexander Graf]

When initializing the e500 code, we need to expose its
cache line size for user and system mode, while the mmu
details are only interesting for system emulation.

Split the 2 switch statements apart, allowing us to #ifdef
out the mmu parts for user mode emulation while keeping all
cache information consistent.

Signed-off-by: Alexander Graf 
---
 target-ppc/translate_init.c |   26 ++
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ba4b84d..6f61175 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4461,33 +4461,36 @@ static void init_proc_e500 (CPUPPCState *env, int 
version)
  &spr_read_spefscr, &spr_write_spefscr,
  &spr_read_spefscr, &spr_write_spefscr,
  0x);
+#if !defined(CONFIG_USER_ONLY)
 /* Memory management */
-#if defined(CONFIG_USER_ONLY)
-env->dcache_line_size = 32;
-env->icache_line_size = 32;
-#else /* !defined(CONFIG_USER_ONLY) */
 env->nb_pids = 3;
 env->nb_ways = 2;
 env->id_tlbs = 0;
 switch (version) {
 case fsl_e500v1:
-/* e500v1 */
 tlbncfg[0] = gen_tlbncfg(2, 1, 1, 0, 256);
 tlbncfg[1] = gen_tlbncfg(16, 1, 9, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16);
-env->dcache_line_size = 32;
-env->icache_line_size = 32;
 break;
 case fsl_e500v2:
-/* e500v2 */
 tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512);
 tlbncfg[1] = gen_tlbncfg(16, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 16);
-env->dcache_line_size = 32;
-env->icache_line_size = 32;
 break;
 case fsl_e500mc:
-/* e500mc */
 tlbncfg[0] = gen_tlbncfg(4, 1, 1, 0, 512);
 tlbncfg[1] = gen_tlbncfg(64, 1, 12, TLBnCFG_AVAIL | TLBnCFG_IPROT, 64);
+break;
+default:
+cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]);
+}
+#endif
+/* Cache sizes */
+switch (version) {
+case fsl_e500v1:
+case fsl_e500v2:
+env->dcache_line_size = 32;
+env->icache_line_size = 32;
+break;
+case fsl_e500mc:
 env->dcache_line_size = 64;
 env->icache_line_size = 64;
 l1cfg0 |= 0x100; /* 64 byte cache block size */
@@ -4495,7 +4498,6 @@ static void init_proc_e500 (CPUPPCState *env, int version)
 default:
 cpu_abort(env, "Unknown CPU: " TARGET_FMT_lx "\n", env->spr[SPR_PVR]);
 }
-#endif
 gen_spr_BookE206(env, 0x00DF, tlbncfg);
 /* XXX : not implemented */
 spr_register(env, SPR_HID0, "HID0",
-- 
1.6.0.2

[Qemu-devel] [PATCH 6/8] pseries: Fix use of global CPU state

[Alexander Graf]

From: Peter Portante 

Commit ed120055c7f9b26b5707d3ceabbe5a3f06aaf937 (Implement PAPR VPA
functions for pSeries shared processor partitions) introduced the
deregister_dtl() function and typo "emv" as name of its argument.
This went unnoticed because the code in that function can access the
global variable "env" so that no build failure resulted.

Fix the argument to read "env". Resolves LP#986241.

Signed-off-by: Peter Portante 
Acked-by: Andreas Färber 
[agraf: fixed typo in commit message]
Signed-off-by: Alexander Graf 
---
 hw/spapr_hcall.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
index 634763e..94bb504 100644
--- a/hw/spapr_hcall.c
+++ b/hw/spapr_hcall.c
@@ -482,7 +482,7 @@ static target_ulong register_dtl(CPUPPCState *env, 
target_ulong addr)
 return H_SUCCESS;
 }
 
-static target_ulong deregister_dtl(CPUPPCState *emv, target_ulong addr)
+static target_ulong deregister_dtl(CPUPPCState *env, target_ulong addr)
 {
 env->dispatch_trace_log = 0;
 env->dtl_size = 0;
-- 
1.6.0.2

[Qemu-devel] [PATCH 4/5] tracetool: avoid pkgutil.iter_modules() Python 2.7 function

[Stefan Hajnoczi]

The pkgutil.iter_modules() function provides a way to enumerate child
modules.  Unfortunately it's missing in Python <2.7 so we must implement
similar behavior ourselves.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Lluís Vilanova 
---
 scripts/tracetool/backend/__init__.py |8 ++--
 scripts/tracetool/format/__init__.py  |8 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/scripts/tracetool/backend/__init__.py 
b/scripts/tracetool/backend/__init__.py
index 34b7ed8..be43472 100644
--- a/scripts/tracetool/backend/__init__.py
+++ b/scripts/tracetool/backend/__init__.py
@@ -37,7 +37,7 @@ __maintainer__ = "Stefan Hajnoczi"
 __email__  = "stefa...@linux.vnet.ibm.com"
 
 
-import pkgutil
+import os
 
 import tracetool
 
@@ -45,7 +45,11 @@ import tracetool
 def get_list():
 """Get a list of (name, description) pairs."""
 res = [("nop", "Tracing disabled.")]
-for _, modname, _ in pkgutil.iter_modules(tracetool.backend.__path__):
+modnames = []
+for filename in os.listdir(tracetool.backend.__path__[0]):
+if filename.endswith('.py') and filename != '__init__.py':
+modnames.append(filename.rsplit('.', 1)[0])
+for modname in modnames:
 module = tracetool.try_import("tracetool.backend." + modname)
 
 # just in case; should never fail unless non-module files are put there
diff --git a/scripts/tracetool/format/__init__.py 
b/scripts/tracetool/format/__init__.py
index 0e4baf0..3c2a0d8 100644
--- a/scripts/tracetool/format/__init__.py
+++ b/scripts/tracetool/format/__init__.py
@@ -41,7 +41,7 @@ __maintainer__ = "Stefan Hajnoczi"
 __email__  = "stefa...@linux.vnet.ibm.com"
 
 
-import pkgutil
+import os
 
 import tracetool
 
@@ -49,7 +49,11 @@ import tracetool
 def get_list():
 """Get a list of (name, description) pairs."""
 res = []
-for _, modname, _ in pkgutil.iter_modules(tracetool.format.__path__):
+modnames = []
+for filename in os.listdir(tracetool.format.__path__[0]):
+if filename.endswith('.py') and filename != '__init__.py':
+modnames.append(filename.rsplit('.', 1)[0])
+for modname in modnames:
 module = tracetool.try_import("tracetool.format." + modname)
 
 # just in case; should never fail unless non-module files are put there
-- 
1.7.10

[Qemu-devel] [PATCH 5/5] configure: check for supported Python 2.x versions

[Stefan Hajnoczi]

The tracetool code requires Python 2.4, which was released in 2004.
Check for a supported Python version so we can give a clear error
message.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Lluís Vilanova 
---
 configure |7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 3c72fa0..b4f1379 100755
--- a/configure
+++ b/configure
@@ -1239,9 +1239,10 @@ fi
 
 # Note that if the Python conditional here evaluates True we will exit
 # with status 1 which is a shell 'false' value.
-if ! "$python" -c 'import sys; sys.exit(sys.version_info[0] >= 3)'; then
-  echo "Python 2 required but '$python' is version 3 or better."
-  echo "Use --python=/path/to/python to specify a Python 2."
+if ! "$python" -c 'import sys; sys.exit(sys.version_info < (2,4) or 
sys.version_info >= (3,))'; then
+  echo "Cannot use '$python', Python 2.4 or later is required."
+  echo "Note that Python 3 or later is not yet supported."
+  echo "Use --python=/path/to/python to specify a supported Python."
   exit 1
 fi
 
-- 
1.7.10

[Qemu-devel] [PATCH 4/8] pseries: Implement automatic PAPR VIO address allocation

[Alexander Graf]

From: David Gibson 

PAPR virtual IO (VIO) devices require a unique, but otherwise arbitrary,
"address" used as a token to the hypercalls which manipulate them.

Currently the pseries machine code does an ok job of allocating these
addresses when the legacy -net nic / -serial and so forth options are used
but will fail to allocate them properly when using -device.

Specifically, you can use -device if all addresses are explicitly assigned.
Without explicit assignment, only one VIO device of each type (network,
console, SCSI) will be assigned properly, any further ones will attempt
to take the same address leading to a fatal error.

This patch fixes the situation by adding a proper address allocator to the
VIO "bus" code.  This is used both by -device and the legacy options and
default devices.  Addresses can still be explicitly assigned with -device
options if desired.

This patch changes the (guest visible) numbering of VIO devices, but since
their addresses are discovered using the device tree and already differ
from the numbering found on existing PowerVM systems, this does not break
compatibility.

Signed-off-by: David Gibson 
Signed-off-by: Alexander Graf 
---
 hw/spapr.c   |7 +++
 hw/spapr_llan.c  |5 ++---
 hw/spapr_vio.c   |   54 ++
 hw/spapr_vio.h   |   13 ++---
 hw/spapr_vscsi.c |5 ++---
 hw/spapr_vty.c   |5 ++---
 6 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/hw/spapr.c b/hw/spapr.c
index bfaf260..cca20f9 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -631,8 +631,7 @@ static void ppc_spapr_init(ram_addr_t ram_size,
 
 for (i = 0; i < MAX_SERIAL_PORTS; i++) {
 if (serial_hds[i]) {
-spapr_vty_create(spapr->vio_bus, SPAPR_VTY_BASE_ADDRESS + i,
- serial_hds[i]);
+spapr_vty_create(spapr->vio_bus, serial_hds[i]);
 }
 }
 
@@ -650,14 +649,14 @@ static void ppc_spapr_init(ram_addr_t ram_size,
 }
 
 if (strcmp(nd->model, "ibmveth") == 0) {
-spapr_vlan_create(spapr->vio_bus, 0x1000 + i, nd);
+spapr_vlan_create(spapr->vio_bus, nd);
 } else {
 pci_nic_init_nofail(&nd_table[i], nd->model, NULL);
 }
 }
 
 for (i = 0; i <= drive_get_max_bus(IF_SCSI); i++) {
-spapr_vscsi_create(spapr->vio_bus, 0x2000 + i);
+spapr_vscsi_create(spapr->vio_bus);
 }
 
 if (rma_size < (MIN_RMA_SLOF << 20)) {
diff --git a/hw/spapr_llan.c b/hw/spapr_llan.c
index e18d2eb..8313043 100644
--- a/hw/spapr_llan.c
+++ b/hw/spapr_llan.c
@@ -204,12 +204,11 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
 return 0;
 }
 
-void spapr_vlan_create(VIOsPAPRBus *bus, uint32_t reg, NICInfo *nd)
+void spapr_vlan_create(VIOsPAPRBus *bus, NICInfo *nd)
 {
 DeviceState *dev;
 
 dev = qdev_create(&bus->bus, "spapr-vlan");
-qdev_prop_set_uint32(dev, "reg", reg);
 
 qdev_set_nic_properties(dev, nd);
 
@@ -480,7 +479,7 @@ static target_ulong h_multicast_ctrl(CPUPPCState *env, 
sPAPREnvironment *spapr,
 }
 
 static Property spapr_vlan_properties[] = {
-DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000, 0x1000),
+DEFINE_SPAPR_PROPERTIES(VIOsPAPRVLANDevice, sdev, 0x1000),
 DEFINE_NIC_PROPERTIES(VIOsPAPRVLANDevice, nicconf),
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/spapr_vio.c b/hw/spapr_vio.c
index fccf48b..315ab80 100644
--- a/hw/spapr_vio.c
+++ b/hw/spapr_vio.c
@@ -620,28 +620,22 @@ static void rtas_quiesce(sPAPREnvironment *spapr, 
uint32_t token,
 rtas_st(rets, 0, 0);
 }
 
-static int spapr_vio_check_reg(VIOsPAPRDevice *sdev)
+static VIOsPAPRDevice *reg_conflict(VIOsPAPRDevice *dev)
 {
-VIOsPAPRDevice *other_sdev;
+VIOsPAPRBus *bus = DO_UPCAST(VIOsPAPRBus, bus, dev->qdev.parent_bus);
 DeviceState *qdev;
-VIOsPAPRBus *sbus;
-
-sbus = DO_UPCAST(VIOsPAPRBus, bus, sdev->qdev.parent_bus);
+VIOsPAPRDevice *other;
 
 /*
- * Check two device aren't given clashing addresses by the user (or some
- * other mechanism). We have to open code this because we have to check
- * for matches with devices other than us.
+ * Check for a device other than the given one which is already
+ * using the requested address. We have to open code this because
+ * the given dev might already be in the list.
  */
-QTAILQ_FOREACH(qdev, &sbus->bus.children, sibling) {
-other_sdev = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
+QTAILQ_FOREACH(qdev, &bus->bus.children, sibling) {
+other = DO_UPCAST(VIOsPAPRDevice, qdev, qdev);
 
-if (other_sdev != sdev && other_sdev->reg == sdev->reg) {
-fprintf(stderr, "vio: %s and %s devices conflict at address %#x\n",
-object_get_typename(OBJECT(sdev)),
-object_get_typename(OBJECT(qdev)),
-sdev->reg);
-return -EEXIST;
+if (other != dev && other->reg == d

Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Alexander Graf]

On 01.05.2012, at 21:11, Peter Maydell wrote:

> On 1 May 2012 19:54, Alexander Graf  wrote:
>> 
>> On 01.05.2012, at 20:32, Andreas Färber wrote:
>> 
>>> Am 01.05.2012 10:58, schrieb Alexander Graf:
 On my PPC host, HOST_LONG_SIZE is not defined even after
 running configure. Use the normal C way of determining the
 long size instead.
 
 Signed-off-by: Alexander Graf 
 ---
 thunk.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/thunk.h b/thunk.h
 index 5be8f91..87025c3 100644
 --- a/thunk.h
 +++ b/thunk.h
 @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype 
 *type_ptr, int is_host)
   defined(HOST_PARISC) || defined(HOST_SPARC64)
 return 4;
 #elif defined(HOST_PPC)
 -return HOST_LONG_SIZE;
 +return sizeof(void *);
>>> 
>>> malc has committed a different fix (TARGET_ABI_BITS / 8) - thanks for
>>> fixing the build - but this PULL will now conflict.
>> 
>> :(.
>> 
>> Removed from my queue.
> 
> We need to put out a patch which converts commit b754e4fc1 to
> the fix as reviewed on the mailing list, because b754e4fc1 is
> incorrect: it is returning the size of a target type when it
> should be returning the size of a host type.

Yes, you're right. Added the below patch onto the queue. Sending a new pull 
request now.


Alex

From ddf1ddcf65866e2dd8e5d515bc636617fb2e15ee Mon Sep 17 00:00:00 2001
From: Alexander Graf 
Date: Mon, 30 Apr 2012 22:58:55 +
Subject: [PATCH 8/8] linux-user: Fix invalid TARGET_ABI_BITS usage on ppc hosts

When trying to evaluate the size of the _host_ type size for olddev_t,
we need to expose the host's pointer size, not the guest pointer size.

This usage got introduced accidently in commit b754e4fc1.

Fix things by not using TARGET_.*, but rather use host sizeof()
information, which gives us the correct size.

Reported-by: Peter Maydell 
Signed-off-by: Alexander Graf 
---
 thunk.h |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/thunk.h b/thunk.h
index c295766..87025c3 100644
--- a/thunk.h
+++ b/thunk.h
@@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype *type_ptr, 
int is_host)
   defined(HOST_PARISC) || defined(HOST_SPARC64)
 return 4;
 #elif defined(HOST_PPC)
-return TARGET_ABI_BITS / 8;
+return sizeof(void *);
 #else
 return 2;
 #endif
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 09/14] qdev: connect some links and move type to object (v2)

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> This makes sysbus part of the root hierarchy and all busses children of their
> respective parent DeviceState.
> 
> Signed-off-by: Anthony Liguori 

This conflicts with Paolo's patch set but since this one is less
intrusive (hardcoding the base init rather than renaming the base class
to non-null) and the other series appears not to get merged for 1.1,

Reviewed-by: Andreas Färber 

/-F

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

Re: [Qemu-devel] [PATCH 12/14] qbus: move print_dev to DeviceClass

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> It should have never been a bus method.
> 
> Signed-off-by: Anthony Liguori 

Looks good,

Reviewed-by: Andreas Färber 

/-F

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

Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> It should have never been a bus method.
> 
> Signed-off-by: Anthony Liguori 
> ---
[...]
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 4a468f8..5044018 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -25,22 +25,13 @@
>  
>  /* - */
>  
> -static char *idebus_get_fw_dev_path(DeviceState *dev);
> -
>  #define TYPE_IDE_BUS "IDE"
> -
> -static void ide_bus_class_init(ObjectClass *klass, void *data)
> -{
> -BusClass *k = BUS_CLASS(klass);
> -
> -k->get_fw_dev_path = idebus_get_fw_dev_path;
> -}
> +#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)

Move macro to preceding patch?

Otherwise looks good.

/-F

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

Re: [Qemu-devel] [PATCH 08/14] qdev: convert busses to QEMU Object Model

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> This is far less interesting than it sounds.  We simply add an Object to each
> BusInfo and then register the types appropriately.  Most of the interesting
> refactoring will follow in the next patches.
> 
> Since we're changing fundamental type names (BusInfo -> BusClass), it all 
> needs
> to convert at once.  Fortunately, not a lot of code is affected.
> 
> Signed-off-by: Anthony Liguori 
> ---
[...]
> diff --git a/hw/intel-hda.c b/hw/intel-hda.c
> index 9405f54..58497e5 100644
> --- a/hw/intel-hda.c
> +++ b/hw/intel-hda.c
> @@ -29,16 +29,19 @@
>  /* - */
>  /* hda bus   */
>  
> -static struct BusInfo hda_codec_bus_info = {
> -.name  = "HDA",
> -.size  = sizeof(HDACodecBus),
> +#define TYPE_HDA_BUS "HDA"

I stumbled over this being a pretty generic term for a type name.
"HDA-codec-bus" maybe?

> diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
> index f5ee166..97673dd 100644
> --- a/hw/qdev-monitor.c
> +++ b/hw/qdev-monitor.c
[...]
> @@ -432,16 +433,16 @@ DeviceState *qdev_device_add(QemuOpts *opts)
>  if (!bus) {
>  return NULL;
>  }
> -if (bus->info != k->bus_info) {
> +if (strcmp(object_get_typename(OBJECT(bus)), k->bus_type) != 0){

Space before {.

> diff --git a/hw/qdev.h b/hw/qdev.h
> index 6d2261f..8ac703e 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
[...]
> @@ -79,28 +79,30 @@ struct DeviceState {
>  int alias_required_for_version;
>  };
>  
> -typedef void (*bus_dev_printfn)(Monitor *mon, DeviceState *dev, int indent);
> -typedef char *(*bus_get_dev_path)(DeviceState *dev);
>  /*
>   * This callback is used to create Open Firmware device path in accordance 
> with
>   * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus 
> bindings
>   * can be found here http://playground.sun.com/1275/bindings/.
>   */
> -typedef char *(*bus_get_fw_dev_path)(DeviceState *dev);
> -typedef int (qbus_resetfn)(BusState *bus);
>  
> -struct BusInfo {
> -const char *name;
> -size_t size;
> -bus_dev_printfn print_dev;
> -bus_get_dev_path get_dev_path;
> -bus_get_fw_dev_path get_fw_dev_path;
> -qbus_resetfn *reset;
> +#define TYPE_BUS "bus"
> +#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
> +#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
> +#define BUS_GET_CLASS(obj) OBJECT_GET_CLASS(BusClass, (obj), TYPE_BUS)
> +
> +struct BusClass {
> +ObjectClass parent_class;
> +
> +/* FIXME first arg should be BusState */
> +void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
> +char *(*get_dev_path)(DeviceState *dev);
> +char *(*get_fw_dev_path)(DeviceState *dev);
> +int (*reset)(BusState *bus);
>  };
>  
>  struct BusState {
> +Object obj;
>  DeviceState *parent;
> -BusInfo *info;
>  const char *name;
>  int allow_hotplug;
>  int qdev_allocated;

Indicate what is /*< private >*/ and what not?

> @@ -321,7 +323,7 @@ void error_set_from_qdev_prop_error(Error **errp, int 
> ret, DeviceState *dev,
>  char *qdev_get_fw_dev_path(DeviceState *dev);
>  
>  /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
> -extern struct BusInfo system_bus_info;
> +#define TYPE_SYSTEM_BUS "System"

"System-bus"?

Regarding the bus names, these have been merely moved here but I'm
raising the topic because they are now in the global name space where
they will potentially conflict with devices and other types.

Otherwise looks okay, thanks for your work on this.

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 1/5] tracetool: use Python 2.4-compatible exception handling syntax

[Stefan Hajnoczi]

The newer "except  as :" syntax is not
supported by Python 2.4, we need to use "except ,
:".

Tested all trace backends with Python 2.4.

Reported-by: Andreas Färber 
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Lluís Vilanova 
---
 scripts/tracetool.py |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index cacfd99..c003cf6 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -70,7 +70,7 @@ def main(args):
 
 try:
 opts, args = getopt.getopt(args[1:], "", long_opts)
-except getopt.GetoptError as err:
+except getopt.GetoptError, err:
 error_opt(str(err))
 
 check_backend = False
@@ -131,7 +131,7 @@ def main(args):
 try:
 tracetool.generate(sys.stdin, arg_format, arg_backend,
binary = binary, probe_prefix = probe_prefix)
-except tracetool.TracetoolError as e:
+except tracetool.TracetoolError, e:
 error_opt(str(e))
 
 if __name__ == "__main__":
-- 
1.7.10

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 08:10, Blue Swirl wrote:


In your view, would a suitable fix be to change dma_memory_read,
dma_memory_write, dma_opaque, it_shift and dma_enabled to be qdev properties
and modify esp_init() to return the qdev reference so they can be set by the
caller?


There's an ongoing work to introduce IOMMUs by changing how DMA work
and this could simplify the DMA part. There's no clean way to use
function pointers in qdev.


In my current working tree, I have actually managed to get this working 
with a customised qdev type macro and the standard qdev pointer type - 
so it may not be particularly great, but it works.



For it_shift, a qdev or QOM property should be OK.

The signal dma_enabled should be eventually replaced by a Pin.


And this is a sysbus concept, yes?


ATB,

Mark.

Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Peter Maydell]

On 1 May 2012 19:54, Alexander Graf  wrote:
>
> On 01.05.2012, at 20:32, Andreas Färber wrote:
>
>> Am 01.05.2012 10:58, schrieb Alexander Graf:
>>> On my PPC host, HOST_LONG_SIZE is not defined even after
>>> running configure. Use the normal C way of determining the
>>> long size instead.
>>>
>>> Signed-off-by: Alexander Graf 
>>> ---
>>> thunk.h |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/thunk.h b/thunk.h
>>> index 5be8f91..87025c3 100644
>>> --- a/thunk.h
>>> +++ b/thunk.h
>>> @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype 
>>> *type_ptr, int is_host)
>>>       defined(HOST_PARISC) || defined(HOST_SPARC64)
>>>             return 4;
>>> #elif defined(HOST_PPC)
>>> -            return HOST_LONG_SIZE;
>>> +            return sizeof(void *);
>>
>> malc has committed a different fix (TARGET_ABI_BITS / 8) - thanks for
>> fixing the build - but this PULL will now conflict.
>
> :(.
>
> Removed from my queue.

We need to put out a patch which converts commit b754e4fc1 to
the fix as reviewed on the mailing list, because b754e4fc1 is
incorrect: it is returning the size of a target type when it
should be returning the size of a host type.

-- PMM

[Qemu-devel] [PATCH 10/14] qbus: move get_dev_path to DeviceState

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori 
---
 hw/pci.c  |   75 -
 hw/qdev.c |   11 ++--
 hw/qdev.h |2 +-
 hw/scsi-bus.c |   10 
 hw/usb/bus.c  |   41 +++
 5 files changed, 66 insertions(+), 73 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index bff303b..291181e 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@
 #endif
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
@@ -49,7 +48,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
 BusClass *k = BUS_CLASS(klass);
 
 k->print_dev = pcibus_dev_print;
-k->get_dev_path = pcibus_get_dev_path;
 k->get_fw_dev_path = pcibus_get_fw_dev_path;
 k->reset = pcibus_reset;
 }
@@ -1899,7 +1897,42 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev)
 return strdup(path);
 }
 
-static char *pcibus_get_dev_path(DeviceState *dev)
+static int pci_qdev_find_recursive(PCIBus *bus,
+   const char *id, PCIDevice **pdev)
+{
+DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
+if (!qdev) {
+return -ENODEV;
+}
+
+/* roughly check if given qdev is pci device */
+if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
+*pdev = PCI_DEVICE(qdev);
+return 0;
+}
+return -EINVAL;
+}
+
+int pci_qdev_find_device(const char *id, PCIDevice **pdev)
+{
+struct PCIHostBus *host;
+int rc = -ENODEV;
+
+QLIST_FOREACH(host, &host_buses, next) {
+int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
+if (!tmp) {
+rc = 0;
+break;
+}
+if (tmp != -ENODEV) {
+rc = tmp;
+}
+}
+
+return rc;
+}
+
+static char *pci_qdev_get_dev_path(DeviceState *dev)
 {
 PCIDevice *d = container_of(dev, PCIDevice, qdev);
 PCIDevice *t;
@@ -1948,41 +1981,6 @@ static char *pcibus_get_dev_path(DeviceState *dev)
 return path;
 }
 
-static int pci_qdev_find_recursive(PCIBus *bus,
-   const char *id, PCIDevice **pdev)
-{
-DeviceState *qdev = qdev_find_recursive(&bus->qbus, id);
-if (!qdev) {
-return -ENODEV;
-}
-
-/* roughly check if given qdev is pci device */
-if (object_dynamic_cast(OBJECT(qdev), TYPE_PCI_DEVICE)) {
-*pdev = PCI_DEVICE(qdev);
-return 0;
-}
-return -EINVAL;
-}
-
-int pci_qdev_find_device(const char *id, PCIDevice **pdev)
-{
-struct PCIHostBus *host;
-int rc = -ENODEV;
-
-QLIST_FOREACH(host, &host_buses, next) {
-int tmp = pci_qdev_find_recursive(host->bus, id, pdev);
-if (!tmp) {
-rc = 0;
-break;
-}
-if (tmp != -ENODEV) {
-rc = tmp;
-}
-}
-
-return rc;
-}
-
 MemoryRegion *pci_address_space(PCIDevice *dev)
 {
 return dev->bus->address_space_mem;
@@ -2000,6 +1998,7 @@ static void pci_device_class_init(ObjectClass *klass, 
void *data)
 k->unplug = pci_unplug_device;
 k->exit = pci_unregister_device;
 k->bus_type = TYPE_PCI_BUS;
+k->get_dev_path = pci_qdev_get_dev_path;
 }
 
 static Property pci_bus_properties[] = {
diff --git a/hw/qdev.c b/hw/qdev.c
index 87ff1a5..eaa3e12 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -528,15 +528,10 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
 
 char *qdev_get_dev_path(DeviceState *dev)
 {
-BusClass *bc;
-
-if (!dev->parent_bus) {
-return NULL;
-}
+DeviceClass *dc = DEVICE_GET_CLASS(dev);
 
-bc = BUS_GET_CLASS(dev->parent_bus);
-if (bc->get_dev_path) {
-return bc->get_dev_path(dev);
+if (dc->get_dev_path) {
+return dc->get_dev_path(dev);
 }
 
 return NULL;
diff --git a/hw/qdev.h b/hw/qdev.h
index 8ac703e..30bfbef 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -47,6 +47,7 @@ typedef struct DeviceClass {
 
 /* callbacks */
 void (*reset)(DeviceState *dev);
+char *(*get_dev_path)(DeviceState *dev);
 
 /* device state */
 const VMStateDescription *vmsd;
@@ -95,7 +96,6 @@ struct BusClass {
 
 /* FIXME first arg should be BusState */
 void (*print_dev)(Monitor *mon, DeviceState *dev, int indent);
-char *(*get_dev_path)(DeviceState *dev);
 char *(*get_fw_dev_path)(DeviceState *dev);
 int (*reset)(BusState *bus);
 };
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 9949192..38189f3 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -16,7 +16,6 @@ static void scsi_bus_class_init(ObjectClass *klass, void 
*data)
 {
 BusClass *k = BUS_CLASS(klass);
 
-k->get_dev_path = scsibus_get_dev_path;
 k->get_fw_dev_path = scsibus_get_fw_dev_path;
 }
 
@@ -1428,15 +1427,15 @@ void scsi_device_purge_requests(SCSIDevice *sdev, 
SCSISense sens

Re: [Qemu-devel] [PATCH 03/14] qdev: add qdev_add_properties

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> This allows a base class to easily add properties.
> 
> Signed-off-by: Anthony Liguori 

Implementation looks okay but /me not so happy with it: This conflicts
with the move of the qdev static property infrastructure from
DeviceState to Object.

Consider rebasing this onto part of Paolo's series and call it
object_add_properties?

Andreas

> ---
>  hw/qdev.c |   25 -
>  hw/qdev.h |2 ++
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 6a8f6bd..e17a9ab 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
>  static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
>   Error **errp);
>  
> -void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
> +void qdev_add_properties(DeviceState *dev, Property *props)
>  {
>  Property *prop;
>  
> +for (prop = props; prop && prop->name; prop++) {
> +qdev_property_add_legacy(dev, prop, NULL);
> +qdev_property_add_static(dev, prop, NULL);
> +}
> +qdev_prop_set_defaults(dev, props);
> +}
> +
> +void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
> +{
>  if (qdev_hotplug) {
>  assert(bus->allow_hotplug);
>  }
>  
>  dev->parent_bus = bus;
>  QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
> -
> -for (prop = qdev_get_bus_info(dev)->props; prop && prop->name; prop++) {
> -qdev_property_add_legacy(dev, prop, NULL);
> -qdev_property_add_static(dev, prop, NULL);
> -}
> -qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
> +qdev_add_properties(dev, dev->parent_bus->info->props);
>  }
>  
>  /* Create a new device.  This only initializes the device state structure
> @@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
>  dev->instance_id_alias = -1;
>  dev->state = DEV_STATE_CREATED;
>  
> -for (prop = qdev_get_props(dev); prop && prop->name; prop++) {
> -qdev_property_add_legacy(dev, prop, NULL);
> -qdev_property_add_static(dev, prop, NULL);
> -}
> -
> +qdev_add_properties(dev, qdev_get_props(dev));
>  object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
> -qdev_prop_set_defaults(dev, qdev_get_props(dev));
>  }
>  
>  /* Unlink device from bus and free the structure.  */
> diff --git a/hw/qdev.h b/hw/qdev.h
> index 4e90119..ca8386a 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
> @@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
>  
>  extern int qdev_hotplug;
>  
> +void qdev_add_properties(DeviceState *dev, Property *props);
> +
>  #endif


-- 
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 03/14] qdev: add qdev_add_properties

[Anthony Liguori]

This allows a base class to easily add properties.

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |   25 -
 hw/qdev.h |2 ++
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..e17a9ab 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -76,22 +76,26 @@ bool qdev_exists(const char *name)
 static void qdev_property_add_legacy(DeviceState *dev, Property *prop,
  Error **errp);
 
-void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+void qdev_add_properties(DeviceState *dev, Property *props)
 {
 Property *prop;
 
+for (prop = props; prop && prop->name; prop++) {
+qdev_property_add_legacy(dev, prop, NULL);
+qdev_property_add_static(dev, prop, NULL);
+}
+qdev_prop_set_defaults(dev, props);
+}
+
+void qdev_set_parent_bus(DeviceState *dev, BusState *bus)
+{
 if (qdev_hotplug) {
 assert(bus->allow_hotplug);
 }
 
 dev->parent_bus = bus;
 QTAILQ_INSERT_HEAD(&bus->children, dev, sibling);
-
-for (prop = qdev_get_bus_info(dev)->props; prop && prop->name; prop++) {
-qdev_property_add_legacy(dev, prop, NULL);
-qdev_property_add_static(dev, prop, NULL);
-}
-qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
+qdev_add_properties(dev, dev->parent_bus->info->props);
 }
 
 /* Create a new device.  This only initializes the device state structure
@@ -633,13 +637,8 @@ static void device_initfn(Object *obj)
 dev->instance_id_alias = -1;
 dev->state = DEV_STATE_CREATED;
 
-for (prop = qdev_get_props(dev); prop && prop->name; prop++) {
-qdev_property_add_legacy(dev, prop, NULL);
-qdev_property_add_static(dev, prop, NULL);
-}
-
+qdev_add_properties(dev, qdev_get_props(dev));
 object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
-qdev_prop_set_defaults(dev, qdev_get_props(dev));
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/hw/qdev.h b/hw/qdev.h
index 4e90119..ca8386a 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -360,4 +360,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
 
 extern int qdev_hotplug;
 
+void qdev_add_properties(DeviceState *dev, Property *props);
+
 #endif
-- 
1.7.5.4

[Qemu-devel] [PATCH 09/14] qdev: connect some links and move type to object (v2)

[Anthony Liguori]

This makes sysbus part of the root hierarchy and all busses children of their
respective parent DeviceState.

Signed-off-by: Anthony Liguori 
---
v1 -> v2
 - move sysbus to /machine/unattached (Andreas)
---
 hw/qdev.c|   12 ++--
 qom/object.c |   12 
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 19d510e..87ff1a5 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -428,6 +428,7 @@ static void do_qbus_create_inplace(BusState *bus, const 
char *typename,
 if (parent) {
 QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
 parent->num_child_bus++;
+object_property_add_child(OBJECT(parent), bus->name, OBJECT(bus), 
NULL);
 } else if (bus != main_system_bus) {
 /* TODO: once all bus devices are qdevified,
only reset handler for main_system_bus should be registered here. */
@@ -457,6 +458,9 @@ static void main_system_bus_create(void)
 /* assign main_system_bus before qbus_create_inplace()
  * in order to make "if (bus != main_system_bus)" work */
 main_system_bus = qbus_create(TYPE_SYSTEM_BUS, NULL, "main-system-bus");
+object_property_add_child(container_get(qdev_get_machine(),
+"/unattached"),
+  "sysbus", OBJECT(main_system_bus), NULL);
 }
 
 void qbus_free(BusState *bus)
@@ -538,11 +542,6 @@ char *qdev_get_dev_path(DeviceState *dev)
 return NULL;
 }
 
-static char *qdev_get_type(Object *obj, Error **errp)
-{
-return g_strdup(object_get_typename(obj));
-}
-
 /**
  * Legacy property handling
  */
@@ -658,7 +657,8 @@ static void device_initfn(Object *obj)
 
 qdev_add_properties(dev, dc->props);
 
-object_property_add_str(OBJECT(dev), "type", qdev_get_type, NULL, NULL);
+object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
+ (Object **)&dev->parent_bus, NULL);
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/qom/object.c b/qom/object.c
index 94928c5..0268f2a 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -256,12 +256,24 @@ static void object_interface_init(Object *obj, 
InterfaceImpl *iface)
 obj->interfaces = g_slist_prepend(obj->interfaces, iface_obj);
 }
 
+static char *object_get_typename_dup(Object *obj, Error **errp)
+{
+return g_strdup(object_get_typename(obj));
+}
+
+static void object_base_init(Object *obj)
+{
+object_property_add_str(obj, "type", object_get_typename_dup, NULL, NULL);
+}
+
 static void object_init_with_type(Object *obj, TypeImpl *ti)
 {
 int i;
 
 if (type_has_parent(ti)) {
 object_init_with_type(obj, type_get_parent(ti));
+} else {
+object_base_init(obj);
 }
 
 for (i = 0; i < ti->num_interfaces; i++) {
-- 
1.7.5.4

[Qemu-devel] [PULL 0/7] s390 patch queue 2012-05-01

[Alexander Graf]

Hi Anthony,

This is my current patch queue for s390. Please pull.

Alex


The following changes since commit b754e4fc1e8e68af975c545c38ebc3b001ebc98f:
  malc (1):
Remove stray HOST_LONG_SIZE

are available in the git repository at:

  git://repo.or.cz/qemu/agraf.git s390-for-upstream

Christian Borntraeger (3):
  S390: fix kernel_commandline handling
  S390: fix error handling on kernel and initrd failures
  S390: dont call system_shutdown on disabled wait

Einar Lueck (1):
  S390: remove default cdrom, sd-card and floppy support

Jens Freimann (3):
  S390: reboot: reset device pages on reboot
  S390: support reboot for kvm on s390
  s390: reset avail and used index on reboot

 hw/s390-virtio-bus.c |   26 +-
 hw/s390-virtio-bus.h |4 
 hw/s390-virtio.c |   17 -
 target-s390x/kvm.c   |   15 +--
 4 files changed, 58 insertions(+), 4 deletions(-)

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Peter Maydell]

On 1 May 2012 19:57, Mark Cave-Ayland  wrote:
> On 01/05/12 16:20, Peter Maydell wrote:
>
>>> Correctness is more important to me than brevity.
>>>
>>> And really, we should focus on killing things like i8259_init().
>>
>>
>> Functions like i8259_init() exist precisely because
>> QOM/qdev don't provide brevity and people trying to
>> use these devices do in fact value brevity. That's why
>> I want the standard native "connect this thing to this
>> other thing" function to be short and simple.
>
>
> My understanding was that the *_init() functions were legacy and shouldn't
> be used any more - at least I've started removing them and replacing them
> with the slighty more long-winded QOM versions in my working tree.

Yes, that is my opinion and is why I'm arguing with Anthony here...

-- PMM

Re: [Qemu-devel] [PATCH 02/14] object: add object_property_foreach

[Andreas Färber]

Am 01.05.2012 20:18, schrieb Anthony Liguori:
> Provide a mechanism to walk through each property for an object.
> 
> Signed-off-by: Anthony Liguori 

Reviewed-by: Andreas Färber 

/-F

-- 
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 02/14] object: add object_property_foreach

[Anthony Liguori]

Provide a mechanism to walk through each property for an object.

Signed-off-by: Anthony Liguori 
---
 include/qemu/object.h |   26 ++
 qom/object.c  |   10 ++
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/include/qemu/object.h b/include/qemu/object.h
index ca1649c..4ad6758 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -915,5 +915,31 @@ void object_property_add_str(Object *obj, const char *name,
  */
 Object *container_get(Object *root, const char *path);
 
+/**
+ * ObjectPropertyEnumerator:
+ * @obj: the object the property belongs to
+ * @name: the name of the property
+ * @typename: the string typename the property was created with
+ * @read_only: if #true, the property is read-only
+ * @opaque: the opaque handle passed to @object_property_foreach
+ *
+ * Callback function type for @object_property_foreach
+ */
+typedef void (ObjectPropertyEnumerator)(Object *obj,
+const char *name,
+const char *typename,
+bool read_only,
+void *opaque);
+
+/**
+ * object_property_foreach:
+ * @path: object to enumerate properties in
+ * @fn: callback function that will be invoked for each property
+ * @opaque: opaque to pass to @fn
+ *
+ * Enumerate all properties in an existing object.
+ */
+void object_property_foreach(Object *obj, ObjectPropertyEnumerator *fn,
+ void *opaque);
 
 #endif
diff --git a/qom/object.c b/qom/object.c
index e721fc2..94928c5 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1194,3 +1194,13 @@ void object_property_add_str(Object *obj, const char 
*name,
 property_release_str,
 prop, errp);
 }
+
+void object_property_foreach(Object *obj, ObjectPropertyEnumerator *fn,
+ void *opaque)
+{
+ObjectProperty *prop;
+
+QTAILQ_FOREACH(prop, &obj->properties, node) {
+fn(obj, prop->name, prop->type, !prop->set, opaque);
+}
+}
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] tests: add gcov target

[Blue Swirl]

On Tue, May 1, 2012 at 18:49, Blue Swirl  wrote:
> Add support for compiling for GCOV test coverage, enabled
> with '--enable-gcov' during configure.
>
> After tests, test coverage can be reported with 'make gcov'.

This is how it looks like:
File '/src/qemu/qjson.c'
Lines executed:86.52% of 141

File '/usr/include/bits/stdio2.h'
Lines executed:100.00% of 1

File '/usr/include/bits/string3.h'
Lines executed:100.00% of 1

File '/src/qemu/qstring.c'
Lines executed:90.48% of 42

File '/src/qemu/qfloat.c'
Lines executed:93.33% of 15

File '/src/qemu/qdict.c'
Lines executed:86.51% of 126

File '/src/qemu/qobject.h'
Lines executed:100.00% of 11

File '/src/qemu/qint.c'
Lines executed:93.33% of 15

File '/src/qemu/qlist.c'
Lines executed:94.12% of 51

File '/src/qemu/coroutine-ucontext.c'
Lines executed:85.00% of 80

(cd i386-softmmu; gcov -n hw/mc146818rtc.c)
File '/src/qemu/hw/mc146818rtc.c'
Lines executed:62.58% of 318

File '/src/qemu/qemu-timer.h'
Lines executed:100.00% of 2

File '/src/qemu/qemu-common.h'
Lines executed:100.00% of 7

File '/src/qemu/hw/irq.h'
Lines executed:100.00% of 2

(cd libhw64; gcov -n hw/m48t59.c)
File '/src/qemu/hw/m48t59.c'
Lines executed:87.19% of 320

File '/src/qemu/qemu-timer.h'
Lines executed:100.00% of 1

File '/src/qemu/qemu-common.h'
Lines executed:100.00% of 2

(cd qapi; gcov -n qapi/qmp-output-visitor.c qapi/qmp-input-visitor.c
qapi/qmp-dispatch.c qapi/string-input-visitor.c
qapi/string-output-visitor.c)
File '/src/qemu/qapi/string-input-visitor.c'
Lines executed:80.33% of 61

File '/src/qemu/qlist.h'
Lines executed:100.00% of 3

File '/src/qemu/qapi/qmp-input-visitor.c'
Lines executed:92.36% of 144

File '/src/qemu/qapi/qmp-output-visitor.c'
Lines executed:100.00% of 100

File '/src/qemu/qobject.h'
Lines executed:100.00% of 10

File '/src/qemu/qapi/qmp-dispatch.c'
Lines executed:74.14% of 58

File '/src/qemu/qapi/string-output-visitor.c'
Lines executed:100.00% of 36

With lcov it's possible to get HTML output, however generating it
takes ages for some reason.

>
> Add LDFLAGS (which may include GCOV flags) to libcacard
> Makefile to avoid a build breakage.
>
> Signed-off-by: Blue Swirl 
> ---
>  configure          |   18 ++
>  libcacard/Makefile |    2 +-
>  tests/Makefile     |   33 +
>  3 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 3c72fa0..dfea46a 100755
> --- a/configure
> +++ b/configure
> @@ -152,6 +152,8 @@ strip_opt="yes"
>  tcg_interpreter="no"
>  bigendian="no"
>  mingw32="no"
> +gcov="no"
> +gcov_tool="gcov"
>  EXESUF=""
>  prefix="/usr/local"
>  mandir="\${prefix}/share/man"
> @@ -556,6 +558,8 @@ for opt do
>   ;;
>   --python=*) python="$optarg"
>   ;;
> +  --gcov=*) gcov_tool="$optarg"
> +  ;;
>   --smbd=*) smbd="$optarg"
>   ;;
>   --extra-cflags=*)
> @@ -576,6 +580,8 @@ for opt do
>   ;;
>   --enable-gprof) gprof="yes"
>   ;;
> +  --enable-gcov) gcov="yes"
> +  ;;
>   --static)
>     static="yes"
>     LDFLAGS="-static $LDFLAGS"
> @@ -1110,6 +1116,8 @@ echo "  --disable-guest-agent    disable
> building of the QEMU Guest Agent"
>  echo "  --enable-guest-agent     enable building of the QEMU Guest Agent"
>  echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
>  echo "                           gthread, ucontext, sigaltstack, windows"
> +echo "  --enable-gcov            enable test coverage analysis with gcov"
> +echo "  --gcov=GCOV              use specified gcov [$gcov_tool]"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure
> is launched"
>  exit 1
> @@ -2883,6 +2891,11 @@ if test "$mingw32" = "yes" ; then
>     done
>  fi
>
> +if test "$gcov" = "yes" ; then
> +  QEMU_CFLAGS="-fprofile-arcs -ftest-coverage $QEMU_CFLAGS"
> +  LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
> +fi
> +
>  qemu_confdir=$sysconfdir$confsuffix
>  qemu_datadir=$datadir$confsuffix
>
> @@ -3008,6 +3021,8 @@ echo "OpenGL support    $opengl"
>  echo "libiscsi support  $libiscsi"
>  echo "build guest agent $guest_agent"
>  echo "coroutine backend $coroutine_backend"
> +echo "gcov              $gcov_tool"
> +echo "gcov enabled      $gcov"
>
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -3412,6 +3427,9 @@ echo "LIBS_TOOLS+=$libs_tools" >> $config_host_mak
>  echo "EXESUF=$EXESUF" >> $config_host_mak
>  echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
>  echo "POD2MAN=$POD2MAN" >> $config_host_mak
> +if test "$gcov" = "yes" ; then
> +  echo "GCOV=$gcov_tool" >> $config_host_mak
> +fi
>
>  # generate list of library paths for linker script
>
> diff --git a/libcacard/Makefile b/libcacard/Makefile
> index c6a896a..47a5eb1 100644
> --- a/libcacard/Makefile
> +++ b/libcacard/Makefile
> @@ -18,7 +18,7 @@ QEMU_CFLAGS+=-I../
>  libcacard.lib-y=$(addsuffix .lo,$(basename $(libcacard-y)))
>
>  vscclient: $(libcacard-y) $(QEMU_OBJS) vscclient.o
> -       $(call quiet-command

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 16:20, Peter Maydell wrote:


Correctness is more important to me than brevity.

And really, we should focus on killing things like i8259_init().


Functions like i8259_init() exist precisely because
QOM/qdev don't provide brevity and people trying to
use these devices do in fact value brevity. That's why
I want the standard native "connect this thing to this
other thing" function to be short and simple.


My understanding was that the *_init() functions were legacy and 
shouldn't be used any more - at least I've started removing them and 
replacing them with the slighty more long-winded QOM versions in my 
working tree.


Or should I just leave them as they are copy the bits I need to a 
separate initialisation function?



ATB,

Mark.

Re: [Qemu-devel] [PATCH 3/8] linux-user: Fix undefined HOST_LONG_SIZE on PPC hosts

[Alexander Graf]

On 01.05.2012, at 20:32, Andreas Färber wrote:

> Am 01.05.2012 10:58, schrieb Alexander Graf:
>> On my PPC host, HOST_LONG_SIZE is not defined even after
>> running configure. Use the normal C way of determining the
>> long size instead.
>> 
>> Signed-off-by: Alexander Graf 
>> ---
>> thunk.h |2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/thunk.h b/thunk.h
>> index 5be8f91..87025c3 100644
>> --- a/thunk.h
>> +++ b/thunk.h
>> @@ -113,7 +113,7 @@ static inline int thunk_type_size(const argtype 
>> *type_ptr, int is_host)
>>   defined(HOST_PARISC) || defined(HOST_SPARC64)
>> return 4;
>> #elif defined(HOST_PPC)
>> -return HOST_LONG_SIZE;
>> +return sizeof(void *);
> 
> malc has committed a different fix (TARGET_ABI_BITS / 8) - thanks for
> fixing the build - but this PULL will now conflict.

:(.

Removed from my queue. I hope I don't have to send out another pull request for 
that ;).


Alex

[Qemu-devel] [PATCH 08/14] qdev: convert busses to QEMU Object Model

[Anthony Liguori]

This is far less interesting than it sounds.  We simply add an Object to each
BusInfo and then register the types appropriately.  Most of the interesting
refactoring will follow in the next patches.

Since we're changing fundamental type names (BusInfo -> BusClass), it all needs
to convert at once.  Fortunately, not a lot of code is affected.

Signed-off-by: Anthony Liguori 
---
 hw/i2c.c  |   14 ++---
 hw/ide/qdev.c |   23 
 hw/intel-hda.c|   14 ++---
 hw/isa-bus.c  |   25 -
 hw/pci-hotplug.c  |6 +---
 hw/pci.c  |   27 --
 hw/pci_bridge.c   |2 +-
 hw/pci_internals.h|2 +-
 hw/qdev-monitor.c |   33 ++
 hw/qdev.c |   59 +
 hw/qdev.h |   38 +-
 hw/s390-virtio-bus.c  |   14 ++---
 hw/scsi-bus.c |   23 +++-
 hw/scsi.h |4 +++
 hw/spapr_vio.c|   14 ++---
 hw/ssi.c  |   14 ++---
 hw/sysbus.c   |   21 ++
 hw/usb/bus.c  |   28 ++-
 hw/usb/dev-smartcard-reader.c |   14 ++---
 hw/virtio-serial-bus.c|   22 +++
 20 files changed, 263 insertions(+), 134 deletions(-)

diff --git a/hw/i2c.c b/hw/i2c.c
index 6d9a7be..1cf1c17 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -17,9 +17,12 @@ struct i2c_bus
 uint8_t saved_address;
 };
 
-static struct BusInfo i2c_bus_info = {
-.name = "I2C",
-.size = sizeof(i2c_bus),
+#define TYPE_I2C_BUS "i2c-bus"
+
+static TypeInfo i2c_bus_info = {
+.name = TYPE_I2C_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(i2c_bus),
 };
 
 static void i2c_bus_pre_save(void *opaque)
@@ -57,7 +60,7 @@ i2c_bus *i2c_init_bus(DeviceState *parent, const char *name)
 {
 i2c_bus *bus;
 
-bus = FROM_QBUS(i2c_bus, qbus_create(&i2c_bus_info, parent, name));
+bus = FROM_QBUS(i2c_bus, qbus_create(TYPE_I2C_BUS, parent, name));
 vmstate_register(NULL, -1, &vmstate_i2c_bus, bus);
 return bus;
 }
@@ -214,7 +217,7 @@ static void i2c_slave_class_init(ObjectClass *klass, void 
*data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->init = i2c_slave_qdev_init;
-k->bus_info = &i2c_bus_info;
+k->bus_type = TYPE_I2C_BUS;
 }
 
 static Property i2c_bus_properties[] = {
@@ -239,6 +242,7 @@ static TypeInfo i2c_slave_type_info = {
 
 static void i2c_slave_register_types(void)
 {
+type_register_static(&i2c_bus_info);
 type_register_static(&i2c_slave_type_info);
 }
 
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 020207a..4a468f8 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -27,15 +27,25 @@
 
 static char *idebus_get_fw_dev_path(DeviceState *dev);
 
-static struct BusInfo ide_bus_info = {
-.name  = "IDE",
-.size  = sizeof(IDEBus),
-.get_fw_dev_path = idebus_get_fw_dev_path,
+#define TYPE_IDE_BUS "IDE"
+
+static void ide_bus_class_init(ObjectClass *klass, void *data)
+{
+BusClass *k = BUS_CLASS(klass);
+
+k->get_fw_dev_path = idebus_get_fw_dev_path;
+}
+
+static TypeInfo ide_bus_info = {
+.name = TYPE_IDE_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(IDEBus),
+.class_init = ide_bus_class_init,
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
 {
-qbus_create_inplace(&idebus->qbus, &ide_bus_info, dev, NULL);
+qbus_create_inplace(&idebus->qbus, TYPE_IDE_BUS, dev, NULL);
 idebus->bus_id = bus_id;
 }
 
@@ -244,7 +254,7 @@ static void ide_device_class_init(ObjectClass *klass, void 
*data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->init = ide_qdev_init;
-k->bus_info = &ide_bus_info;
+k->bus_type = TYPE_IDE_BUS;
 }
 
 static Property ide_bus_properties[] = {
@@ -269,6 +279,7 @@ static TypeInfo ide_device_type_info = {
 
 static void ide_register_types(void)
 {
+type_register_static(&ide_bus_info);
 type_register_static(&ide_hd_info);
 type_register_static(&ide_cd_info);
 type_register_static(&ide_drive_info);
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 9405f54..58497e5 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -29,16 +29,19 @@
 /* - */
 /* hda bus   */
 
-static struct BusInfo hda_codec_bus_info = {
-.name  = "HDA",
-.size  = sizeof(HDACodecBus),
+#define TYPE_HDA_BUS "HDA"
+
+static TypeInfo hda_codec_bus_info = {
+.name = TYPE_HDA_BUS,
+.parent = TYPE_BUS,
+.instance_size = sizeof(HDACodecBus),
 };
 
 void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
 hda_codec_response_func response,
 hda_codec_xfer_func xfer)
 {
-qbus_create_

[Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori 
---
 hw/ide/qdev.c |   33 +
 hw/isa-bus.c  |   31 +++
 hw/pci.c  |   31 +++
 hw/qdev.c |   10 +-
 hw/qdev.h |2 +-
 hw/scsi-bus.c |   33 -
 hw/sysbus.c   |   39 +++
 hw/usb/bus.c  |   55 +++
 8 files changed, 107 insertions(+), 127 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 4a468f8..5044018 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -25,22 +25,13 @@
 
 /* - */
 
-static char *idebus_get_fw_dev_path(DeviceState *dev);
-
 #define TYPE_IDE_BUS "IDE"
-
-static void ide_bus_class_init(ObjectClass *klass, void *data)
-{
-BusClass *k = BUS_CLASS(klass);
-
-k->get_fw_dev_path = idebus_get_fw_dev_path;
-}
+#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
 
 static TypeInfo ide_bus_info = {
 .name = TYPE_IDE_BUS,
 .parent = TYPE_BUS,
 .instance_size = sizeof(IDEBus),
-.class_init = ide_bus_class_init,
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -49,16 +40,6 @@ void ide_bus_new(IDEBus *idebus, DeviceState *dev, int 
bus_id)
 idebus->bus_id = bus_id;
 }
 
-static char *idebus_get_fw_dev_path(DeviceState *dev)
-{
-char path[30];
-
-snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
- ((IDEBus*)dev->parent_bus)->bus_id);
-
-return strdup(path);
-}
-
 static int ide_qdev_init(DeviceState *qdev)
 {
 IDEDevice *dev = IDE_DEVICE(qdev);
@@ -250,11 +231,23 @@ static TypeInfo ide_drive_info = {
 .class_init= ide_drive_class_init,
 };
 
+static char *ide_device_get_fw_dev_path(DeviceState *dev)
+{
+IDEBus *parent_bus = IDE_BUS(dev->parent_bus);
+char path[30];
+
+snprintf(path, sizeof(path), "%s@%d", qdev_fw_name(dev),
+ parent_bus->bus_id);
+
+return g_strdup(path);
+}
+
 static void ide_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->init = ide_qdev_init;
 k->bus_type = TYPE_IDE_BUS;
+k->get_fw_dev_path = ide_device_get_fw_dev_path;
 }
 
 static Property ide_bus_properties[] = {
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 2616f22..6141515 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -26,7 +26,6 @@ static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
 
 static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *isabus_get_fw_dev_path(DeviceState *dev);
 
 #define TYPE_ISA_BUS "ISA"
 
@@ -35,7 +34,6 @@ static void isa_bus_class_init(ObjectClass *klass, void *data)
 BusClass *k = BUS_CLASS(klass);
 
 k->print_dev = isabus_dev_print;
-k->get_fw_dev_path = isabus_get_fw_dev_path;
 }
 
 static TypeInfo isa_bus_info = {
@@ -204,11 +202,26 @@ static TypeInfo isabus_bridge_info = {
 .class_init= isabus_bridge_class_init,
 };
 
+static char *isa_device_get_fw_dev_path(DeviceState *dev)
+{
+ISADevice *d = (ISADevice*)dev;
+char path[40];
+int off;
+
+off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
+if (d->ioport_id) {
+snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
+}
+
+return strdup(path);
+}
+
 static void isa_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->init = isa_qdev_init;
 k->bus_type = TYPE_ISA_BUS;
+k->get_fw_dev_path = isa_device_get_fw_dev_path;
 }
 
 static TypeInfo isa_device_type_info = {
@@ -227,20 +240,6 @@ static void isabus_register_types(void)
 type_register_static(&isa_device_type_info);
 }
 
-static char *isabus_get_fw_dev_path(DeviceState *dev)
-{
-ISADevice *d = (ISADevice*)dev;
-char path[40];
-int off;
-
-off = snprintf(path, sizeof(path), "%s", qdev_fw_name(dev));
-if (d->ioport_id) {
-snprintf(path + off, sizeof(path) - off, "@%04x", d->ioport_id);
-}
-
-return strdup(path);
-}
-
 MemoryRegion *isa_address_space(ISADevice *dev)
 {
 return get_system_memory();
diff --git a/hw/pci.c b/hw/pci.c
index 291181e..425ceaa 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -40,7 +40,6 @@
 #endif
 
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-static char *pcibus_get_fw_dev_path(DeviceState *dev);
 static int pcibus_reset(BusState *qbus);
 
 static void pci_bus_class_init(ObjectClass *klass, void *data)
@@ -48,7 +47,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
 BusClass *k = BUS_CLASS(klass);
 
 k->print_dev = pcibus_dev_print;
-k->get_fw_dev_path = pcibus_get_fw_dev_path;
 k->reset = pcibus_reset;
 }
 
@@ -1883,20 +1881,6 @@ static char *pci_dev_fw_name(DeviceState *dev, char 
*buf, int len)
 return buf;
 }
 
-static char *pcibus_get_fw_dev_path(DeviceState *dev)
-

[Qemu-devel] [PATCH 06/14] qdev: move properties from businfo to base class instance init

[Anthony Liguori]

This removes knowledge of specific DeviceState subclasses from BusInfos.

Signed-off-by: Anthony Liguori 
---
 hw/i2c.c  |   15 +++
 hw/ide/qdev.c |   15 +++
 hw/intel-hda.c|   15 +++
 hw/pci.c  |   27 +--
 hw/scsi-bus.c |   19 +--
 hw/spapr_vio.c|   15 +++
 hw/usb/bus.c  |   19 +--
 hw/usb/dev-smartcard-reader.c |   15 +++
 hw/virtio-serial-bus.c|   17 -
 9 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/hw/i2c.c b/hw/i2c.c
index 23dfccb..6d9a7be 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -20,10 +20,6 @@ struct i2c_bus
 static struct BusInfo i2c_bus_info = {
 .name = "I2C",
 .size = sizeof(i2c_bus),
-.props = (Property[]) {
-DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
-DEFINE_PROP_END_OF_LIST(),
-}
 };
 
 static void i2c_bus_pre_save(void *opaque)
@@ -221,10 +217,21 @@ static void i2c_slave_class_init(ObjectClass *klass, void 
*data)
 k->bus_info = &i2c_bus_info;
 }
 
+static Property i2c_bus_properties[] = {
+DEFINE_PROP_UINT8("address", struct I2CSlave, address, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void i2c_slave_initfn(Object *obj)
+{
+qdev_add_properties(DEVICE(obj), i2c_bus_properties);
+}
+
 static TypeInfo i2c_slave_type_info = {
 .name = TYPE_I2C_SLAVE,
 .parent = TYPE_DEVICE,
 .instance_size = sizeof(I2CSlave),
+.instance_init = i2c_slave_initfn,
 .abstract = true,
 .class_size = sizeof(I2CSlaveClass),
 .class_init = i2c_slave_class_init,
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a46578d..020207a 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -31,10 +31,6 @@ static struct BusInfo ide_bus_info = {
 .name  = "IDE",
 .size  = sizeof(IDEBus),
 .get_fw_dev_path = idebus_get_fw_dev_path,
-.props = (Property[]) {
-DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
-DEFINE_PROP_END_OF_LIST(),
-},
 };
 
 void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -251,10 +247,21 @@ static void ide_device_class_init(ObjectClass *klass, 
void *data)
 k->bus_info = &ide_bus_info;
 }
 
+static Property ide_bus_properties[] = {
+DEFINE_PROP_UINT32("unit", IDEDevice, unit, -1),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ide_device_initfn(Object *obj)
+{
+qdev_add_properties(DEVICE(obj), ide_bus_properties);
+}
+
 static TypeInfo ide_device_type_info = {
 .name = TYPE_IDE_DEVICE,
 .parent = TYPE_DEVICE,
 .instance_size = sizeof(IDEDevice),
+.instance_init = ide_device_initfn,
 .abstract = true,
 .class_size = sizeof(IDEDeviceClass),
 .class_init = ide_device_class_init,
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..9405f54 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -32,10 +32,6 @@
 static struct BusInfo hda_codec_bus_info = {
 .name  = "HDA",
 .size  = sizeof(HDACodecBus),
-.props = (Property[]) {
-DEFINE_PROP_UINT32("cad", HDACodecDevice, cad, -1),
-DEFINE_PROP_END_OF_LIST()
-}
 };
 
 void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
@@ -1278,10 +1274,21 @@ static void hda_codec_device_class_init(ObjectClass 
*klass, void *data)
 k->bus_info = &hda_codec_bus_info;
 }
 
+static Property hda_codec_bus_properties[] = {
+DEFINE_PROP_UINT32("cad", HDACodecDevice, cad, -1),
+DEFINE_PROP_END_OF_LIST()
+};
+
+static void hda_codec_device_initfn(Object *obj)
+{
+qdev_add_properties(DEVICE(obj), hda_codec_bus_properties);
+}
+
 static TypeInfo hda_codec_device_type_info = {
 .name = TYPE_HDA_CODEC_DEVICE,
 .parent = TYPE_DEVICE,
 .instance_size = sizeof(HDACodecDevice),
+.instance_init = hda_codec_device_initfn,
 .abstract = true,
 .class_size = sizeof(HDACodecDeviceClass),
 .class_init = hda_codec_device_class_init,
diff --git a/hw/pci.c b/hw/pci.c
index b706e69..0a77025 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -51,16 +51,6 @@ struct BusInfo pci_bus_info = {
 .get_dev_path = pcibus_get_dev_path,
 .get_fw_dev_path = pcibus_get_fw_dev_path,
 .reset  = pcibus_reset,
-.props  = (Property[]) {
-DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1),
-DEFINE_PROP_STRING("romfile", PCIDevice, romfile),
-DEFINE_PROP_UINT32("rombar",  PCIDevice, rom_bar, 1),
-DEFINE_PROP_BIT("multifunction", PCIDevice, cap_present,
-QEMU_PCI_CAP_MULTIFUNCTION_BITNR, false),
-DEFINE_PROP_BIT("command_serr_enable", PCIDevice, cap_present,
-QEMU_PCI_CAP_SERR_BITNR, true),
-DEFINE_PROP_END_OF_LIST()
-}
 };
 
 static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -2004,10 +1994,27 @@ static void pci_device_class_init(ObjectClass *klass, 
void *d

[Qemu-devel] [PATCH] tests: add gcov target

[Blue Swirl]

Add support for compiling for GCOV test coverage, enabled
with '--enable-gcov' during configure.

After tests, test coverage can be reported with 'make gcov'.

Add LDFLAGS (which may include GCOV flags) to libcacard
Makefile to avoid a build breakage.

Signed-off-by: Blue Swirl 
---
 configure  |   18 ++
 libcacard/Makefile |2 +-
 tests/Makefile |   33 +
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 3c72fa0..dfea46a 100755
--- a/configure
+++ b/configure
@@ -152,6 +152,8 @@ strip_opt="yes"
 tcg_interpreter="no"
 bigendian="no"
 mingw32="no"
+gcov="no"
+gcov_tool="gcov"
 EXESUF=""
 prefix="/usr/local"
 mandir="\${prefix}/share/man"
@@ -556,6 +558,8 @@ for opt do
   ;;
   --python=*) python="$optarg"
   ;;
+  --gcov=*) gcov_tool="$optarg"
+  ;;
   --smbd=*) smbd="$optarg"
   ;;
   --extra-cflags=*)
@@ -576,6 +580,8 @@ for opt do
   ;;
   --enable-gprof) gprof="yes"
   ;;
+  --enable-gcov) gcov="yes"
+  ;;
   --static)
 static="yes"
 LDFLAGS="-static $LDFLAGS"
@@ -1110,6 +1116,8 @@ echo "  --disable-guest-agentdisable
building of the QEMU Guest Agent"
 echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
 echo "  --with-coroutine=BACKEND coroutine backend. Supported options:"
 echo "   gthread, ucontext, sigaltstack, windows"
+echo "  --enable-gcovenable test coverage analysis with gcov"
+echo "  --gcov=GCOV  use specified gcov [$gcov_tool]"
 echo ""
 echo "NOTE: The object files are built at the place where configure
is launched"
 exit 1
@@ -2883,6 +2891,11 @@ if test "$mingw32" = "yes" ; then
 done
 fi

+if test "$gcov" = "yes" ; then
+  QEMU_CFLAGS="-fprofile-arcs -ftest-coverage $QEMU_CFLAGS"
+  LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
+fi
+
 qemu_confdir=$sysconfdir$confsuffix
 qemu_datadir=$datadir$confsuffix

@@ -3008,6 +3021,8 @@ echo "OpenGL support$opengl"
 echo "libiscsi support  $libiscsi"
 echo "build guest agent $guest_agent"
 echo "coroutine backend $coroutine_backend"
+echo "gcov  $gcov_tool"
+echo "gcov enabled  $gcov"

 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3412,6 +3427,9 @@ echo "LIBS_TOOLS+=$libs_tools" >> $config_host_mak
 echo "EXESUF=$EXESUF" >> $config_host_mak
 echo "LIBS_QGA+=$libs_qga" >> $config_host_mak
 echo "POD2MAN=$POD2MAN" >> $config_host_mak
+if test "$gcov" = "yes" ; then
+  echo "GCOV=$gcov_tool" >> $config_host_mak
+fi

 # generate list of library paths for linker script

diff --git a/libcacard/Makefile b/libcacard/Makefile
index c6a896a..47a5eb1 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -18,7 +18,7 @@ QEMU_CFLAGS+=-I../
 libcacard.lib-y=$(addsuffix .lo,$(basename $(libcacard-y)))

 vscclient: $(libcacard-y) $(QEMU_OBJS) vscclient.o
-   $(call quiet-command,$(CC) -o $@ $^ $(libcacard_libs) $(LIBS),"  LINK  
$@")
+   $(call quiet-command,$(CC) $(LDFLAGS) -o $@ $^ $(libcacard_libs)
$(LIBS),"  LINK  $@")

 clean:
rm -f *.o */*.o *.d */*.d *.a */*.a *~ */*~ vscclient *.lo .libs/* *.la 
*.pc
diff --git a/tests/Makefile b/tests/Makefile
index 9988681..a316180 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,18 +1,42 @@
 export SRC_PATH

 check-unit-y = tests/check-qdict$(EXESUF)
+GCOV_FILES = qdict.c
 check-unit-y += tests/check-qfloat$(EXESUF)
+GCOV_FILES += qfloat.c
 check-unit-y += tests/check-qint$(EXESUF)
+GCOV_FILES += qint.c
 check-unit-y += tests/check-qstring$(EXESUF)
+GCOV_FILES += qstring.c
 check-unit-y += tests/check-qlist$(EXESUF)
+GCOV_FILES += qlist.c
 check-unit-y += tests/check-qjson$(EXESUF)
+GCOV_FILES += qjson.c
 check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
+GCOV_QAPI_FILES += qapi/qmp-output-visitor.c
 check-unit-y += tests/test-qmp-input-visitor$(EXESUF)
+GCOV_QAPI_FILES += qapi/qmp-input-visitor.c
 check-unit-y += tests/test-qmp-input-strict$(EXESUF)
 check-unit-y += tests/test-qmp-commands$(EXESUF)
+GCOV_QAPI_FILES += qapi/qmp-dispatch.c
 check-unit-y += tests/test-string-input-visitor$(EXESUF)
+GCOV_QAPI_FILES += qapi/string-input-visitor.c
 check-unit-y += tests/test-string-output-visitor$(EXESUF)
+GCOV_QAPI_FILES += qapi/string-output-visitor.c
 check-unit-y += tests/test-coroutine$(EXESUF)
+ifeq ($(CONFIG_WIN32),y)
+GCOV_FILES += coroutine-win32.c
+else
+ifeq ($(CONFIG_UCONTEXT_COROUTINE),y)
+GCOV_FILES += coroutine-ucontext.c
+else
+ifeq ($(CONFIG_SIGALTSTACK_COROUTINE),y)
+GCOV_FILES += coroutine-sigaltstack.c
+else
+GCOV_FILES += coroutine-gthread.c
+endif
+endif
+endif

 check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh

@@ -20,8 +44,10 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 # really in libqtest, not in the testcases themselves.
 check-qtest-i386-y = tests/rtc-test
 check-qtest-x86_64-y = $(check-qtest-i386-y)
+GCOV_I386_SOFTMMU_FILES += hw/mc146818rtc.c
 check-qtest

[Qemu-devel] [PATCH 01/14] qdev: fix adding of ptr properties

[Anthony Liguori]

ptr properties have neither a get/set or a print/parse which means that when
they're added they aren't treated as static or legacy properties.

Just assume properties like this are legacy properties and treat them as such.

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 0bcde20..6a8f6bd 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -576,9 +576,12 @@ void qdev_property_add_legacy(DeviceState *dev, Property 
*prop,
 {
 gchar *name, *type;
 
-if (!prop->info->print && !prop->info->parse) {
+/* Register pointer properties as legacy properties */
+if (!prop->info->print && !prop->info->parse &&
+(prop->info->set || prop->info->get)) {
 return;
 }
+
 name = g_strdup_printf("legacy-%s", prop->name);
 type = g_strdup_printf("legacy<%s>",
prop->info->legacy_name ?: prop->info->name);
-- 
1.7.5.4

[Qemu-devel] [PATCH 0/14] qom: convert busses to QOM (v2)

[Anthony Liguori]

This is the last of the core QOM series.  This series converts busses to QOM
using a model where busses are proper objects that inherit from Object directly.
Devices have a has-a relationship with any bus they implement.

This series also creates link associated with the device/bus relationships.  All
devices have a parent_bus link that can be (optionally) set to their parent_bus
property.  This link is typed as BusState.  Over time, I'd like to refactor
this to a subclass added property with a stronger type.  For instance, PCIDevice
would have a parent:link property.

Busses also have links to their children.  These are anonymous/unstable names.
Long term, I'd like to move to having stable names based on bus specific
information.  For instance, PCI busses ought to use a 'slot[0.0]' naming
convention.

I've tested this series pretty extensively.  It should be clean except for the
one patch that temporarily breaks and then fixes info qdm/qtree.
---
v1 -> v2
 - Move sysbus to /machine/unattached/sysbus (Andreas)
 - Rebase

 exec.c|4 
 hw/acpi_piix4.c   |   10 +
 hw/i2c.c  |   34 +++--
 hw/ide/qdev.c |   55 +
 hw/intel-hda.c|   44 ---
 hw/isa-bus.c  |   75 ++--
 hw/lsi53c895a.c   |5 
 hw/pci-hotplug.c  |6 -
 hw/pci.c  |  221 +++--
 hw/pci_bridge.c   |2 
 hw/pci_internals.h|2 
 hw/qdev-monitor.c |  177 +-
 hw/qdev-properties.c  |   33 +
 hw/qdev.c |  247 --
 hw/qdev.h |   53 +
 hw/s390-virtio-bus.c  |   39 +++---
 hw/scsi-bus.c |   80 +++--
 hw/scsi.h |4 
 hw/spapr_pci.c|7 -
 hw/spapr_vio.c|   56 +
 hw/spapr_vty.c|6 -
 hw/ssi.c  |   28 ++--
 hw/sysbus.c   |   81 ++---
 hw/usb/bus.c  |  158 ++
 hw/usb/dev-smartcard-reader.c |   29 +++-
 hw/virtio-scsi.c  |6 -
 hw/virtio-serial-bus.c|   55 +
 include/qemu/object.h |   26 
 qom/object.c  |   33 +
 savevm.c  |   12 +-
 30 files changed, 933 insertions(+), 655 deletions(-)

Re: [Qemu-devel] Poking a sun4v machine

[Alexander Graf]

On 30.04.2012, at 19:15, Andreas Färber wrote:

> Am 30.04.2012 18:39, schrieb Artyom Tarasenko:
>> Tried to boot QEMU Niagara machine with the firmware from the
>> OpenSPARC T1 emulator ( www.opensparc.net/opensparc-t1/download.html )
>> , and it dies very early.
>> The reason: in translate.c
>> 
>> #define hypervisor(dc) (dc->mem_idx == MMU_HYPV_IDX)
>> #define supervisor(dc) (dc->mem_idx >= MMU_KERNEL_IDX)
>> 
>> and the dc->mem_idx is initialized like this:
>> 
>>if (env1->tl > 0) {
>>return MMU_NUCLEUS_IDX;
>>} else if (cpu_hypervisor_mode(env1)) {
>>return MMU_HYPV_IDX;
>>} else if (cpu_supervisor_mode(env1)) {
>>return MMU_KERNEL_IDX;
>>} else {
>>return MMU_USER_IDX;
>>}
>> 
>> Which seems to be conceptually incorrect. After reset tl == MAXTL, but
>> still super- and hyper-visor bits are set, so both supervisor(dc) and
>> hypervisor(dc) must return 1 which is impossible in the current
>> implementation.
>> 
>> What would be the proper way to fix it? Make mem_idx bitmap, add two
>> more variables to DisasContext, or ...?
>> 
>> Some other findings/questions:
>> 
>>/* Sun4v generic Niagara machine */
>>{
>>.default_cpu_model = "Sun UltraSparc T1",
>>.console_serial_base = 0xfff0c2c000ULL,
>> 
>> Where is this address coming from? The OpenSPARC Niagara machine has a
>> "dumb serial" at 0x1f1000ULL.
>> 
>> And the biggest issue: UA2005 (as well as UA2007) describe a totally
>> different format for a MMU TTE entry than the one sun4u CPU are using.
>> I think the best way to handle it would be splitting off Niagara
>> machine, and #defining MMU bits differently for sun4u and sun4v
>> machines.
>> 
>> Do we the cases in qemu where more than two (qemu-system-xxx and
>> qemu-system-xxx64) binaries are produced?
>> Would the name qemu-system-sun4v fit the naming convention?
> 
> We have such a case for ppc (ppcemb) and it is kind of a maintenance
> nightmare - I'm working towards getting rid of it with my QOM CPU work.
> Better avoid it for sparc in the first place.

The big differentiator of ppcemb is that TARGET_PAGE_SIZE is 1k. That's a 
setting I really don't want to enable for anyone not running 440 code.


Alex

Re: [Qemu-devel] Memory API: handling unassigned physical memory

[Mark Cave-Ayland]

On 01/05/12 07:57, Blue Swirl wrote:


Therefore I can't change it to my (modified) sbus_mmio_map() function
because it would break other non-SPARC platforms, and AIUI there is nothing
in the memory API that allows me to move a subregion to a different
MemoryRegion parent, even if I can get a reference to it with
sysbus_mmio_get_region() after the sysbus_mmio_map() call - or have I
misunderstood something?


Sysbus is used as a generic class for motherboard devices, there is an
assumption that there is no higher level bus. What we need here is a
full blown bus. The translations and mappigs between bus addresses and
motherboard addresses should be done in a Sysbus to SBus bridge
device, just like PCI host bridges do.


Since SBus is mapped directly to physical addresses, is this mapping not 
just 1:1? Or does it make sense to re-work all the offsets of the 
various peripherals to be from the base address of the first slot?



ATB,

Mark.

[Qemu-devel] [PATCH 13/14] qbus: make child devices links

[Anthony Liguori]

Make qbus children show up as link<> properties.  There is no stable addressing
for qbus children so we use an unstable naming convention.

This is okay in QOM though because the composition name is expected to be what's
stable.

Signed-off-by: Anthony Liguori 
---
 hw/acpi_piix4.c  |   10 +---
 hw/i2c.c |5 ++-
 hw/intel-hda.c   |   15 +++-
 hw/lsi53c895a.c  |5 ++-
 hw/qdev-monitor.c|   26 +
 hw/qdev.c|   62 +-
 hw/qdev.h|   11 +++-
 hw/s390-virtio-bus.c |   25 +--
 hw/scsi-bus.c|   13 ++
 hw/spapr_pci.c   |7 +++--
 hw/spapr_vio.c   |   27 +++--
 hw/spapr_vty.c   |6 +++-
 hw/ssi.c |   14 ++-
 hw/virtio-scsi.c |6 ++--
 qom/object.c |   11 +++-
 15 files changed, 159 insertions(+), 84 deletions(-)

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 585da4e..03daf9a 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -284,7 +284,7 @@ static const VMStateDescription vmstate_acpi = {
 
 static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
 {
-DeviceState *qdev, *next;
+BusChild *kid, *next;
 BusState *bus = qdev_get_parent_bus(&s->dev.qdev);
 int slot = ffs(slots) - 1;
 bool slot_free = true;
@@ -292,7 +292,8 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned 
slots)
 /* Mark request as complete */
 s->pci0_status.down &= ~(1U << slot);
 
-QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+DeviceState *qdev = kid->child;
 PCIDevice *dev = PCI_DEVICE(qdev);
 PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
 if (PCI_SLOT(dev->devfn) == slot) {
@@ -312,7 +313,7 @@ static void piix4_update_hotplug(PIIX4PMState *s)
 {
 PCIDevice *dev = &s->dev;
 BusState *bus = qdev_get_parent_bus(&dev->qdev);
-DeviceState *qdev, *next;
+BusChild *kid, *next;
 
 /* Execute any pending removes during reset */
 while (s->pci0_status.down) {
@@ -322,7 +323,8 @@ static void piix4_update_hotplug(PIIX4PMState *s)
 s->pci0_hotplug_enable = ~0;
 s->pci0_slot_device_present = 0;
 
-QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
+DeviceState *qdev = kid->child;
 PCIDevice *pdev = PCI_DEVICE(qdev);
 PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
 int slot = PCI_SLOT(pdev->devfn);
diff --git a/hw/i2c.c b/hw/i2c.c
index 1cf1c17..50d5a84 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -80,11 +80,12 @@ int i2c_bus_busy(i2c_bus *bus)
 /* TODO: Make this handle multiple masters.  */
 int i2c_start_transfer(i2c_bus *bus, uint8_t address, int recv)
 {
-DeviceState *qdev;
+BusChild *kid;
 I2CSlave *slave = NULL;
 I2CSlaveClass *sc;
 
-QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
+QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+DeviceState *qdev = kid->child;
 I2CSlave *candidate = I2C_SLAVE_FROM_QDEV(qdev);
 if (candidate->address == address) {
 slave = candidate;
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 58497e5..00237c0 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -75,10 +75,11 @@ static int hda_codec_dev_exit(DeviceState *qdev)
 
 HDACodecDevice *hda_codec_find(HDACodecBus *bus, uint32_t cad)
 {
-DeviceState *qdev;
+BusChild *kid;
 HDACodecDevice *cdev;
 
-QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
+QTAILQ_FOREACH(kid, &bus->qbus.children, sibling) {
+DeviceState *qdev = kid->child;
 cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
 if (cdev->cad == cad) {
 return cdev;
@@ -480,10 +481,11 @@ static void intel_hda_parse_bdl(IntelHDAState *d, 
IntelHDAStream *st)
 
 static void intel_hda_notify_codecs(IntelHDAState *d, uint32_t stream, bool 
running, bool output)
 {
-DeviceState *qdev;
+BusChild *kid;
 HDACodecDevice *cdev;
 
-QTAILQ_FOREACH(qdev, &d->codecs.qbus.children, sibling) {
+QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
+DeviceState *qdev = kid->child;
 HDACodecDeviceClass *cdc;
 
 cdev = DO_UPCAST(HDACodecDevice, qdev, qdev);
@@ -1102,15 +1104,16 @@ static const MemoryRegionOps intel_hda_mmio_ops = {
 
 static void intel_hda_reset(DeviceState *dev)
 {
+BusChild *kid;
 IntelHDAState *d = DO_UPCAST(IntelHDAState, pci.qdev, dev);
-DeviceState *qdev;
 HDACodecDevice *cdev;
 
 intel_hda_regs_reset(d);
 d->wall_base_ns = qemu_get_clock_ns(vm_clock);
 
 /* reset codecs */
-QTAILQ_FOREACH(qdev, &d->codecs.qbus.children, sibling) {
+QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
+DeviceState *qdev = kid->child;
 cdev = DO_UPCAST(HDACodecDevice,

[Qemu-devel] [PATCH 12/14] qbus: move print_dev to DeviceClass

[Anthony Liguori]

It should have never been a bus method.

Signed-off-by: Anthony Liguori 
---
 hw/isa-bus.c   |   37 --
 hw/pci.c   |   79 +++
 hw/qdev-monitor.c  |   10 +++---
 hw/qdev.h  |3 +-
 hw/sysbus.c|   39 +--
 hw/usb/bus.c   |   35 -
 hw/virtio-serial-bus.c |   32 +++
 7 files changed, 99 insertions(+), 136 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 6141515..07a9ae4 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -25,22 +25,12 @@
 static ISABus *isabus;
 target_phys_addr_t isa_mem_base = 0;
 
-static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent);
-
 #define TYPE_ISA_BUS "ISA"
 
-static void isa_bus_class_init(ObjectClass *klass, void *data)
-{
-BusClass *k = BUS_CLASS(klass);
-
-k->print_dev = isabus_dev_print;
-}
-
 static TypeInfo isa_bus_info = {
 .name = TYPE_ISA_BUS,
 .parent = TYPE_BUS,
 .instance_size = sizeof(ISABus),
-.class_init = isa_bus_class_init,
 };
 
 ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space_io)
@@ -166,19 +156,6 @@ ISADevice *isa_create_simple(ISABus *bus, const char *name)
 return dev;
 }
 
-static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
-ISADevice *d = ISA_DEVICE(dev);
-
-if (d->isairq[1] != -1) {
-monitor_printf(mon, "%*sisa irqs %d,%d\n", indent, "",
-   d->isairq[0], d->isairq[1]);
-} else if (d->isairq[0] != -1) {
-monitor_printf(mon, "%*sisa irq %d\n", indent, "",
-   d->isairq[0]);
-}
-}
-
 static int isabus_bridge_init(SysBusDevice *dev)
 {
 /* nothing */
@@ -216,12 +193,26 @@ static char *isa_device_get_fw_dev_path(DeviceState *dev)
 return strdup(path);
 }
 
+static void isa_qdev_dev_print(DeviceState *dev, Monitor *mon, int indent)
+{
+ISADevice *d = ISA_DEVICE(dev);
+
+if (d->isairq[1] != -1) {
+monitor_printf(mon, "%*sisa irqs %d,%d\n", indent, "",
+   d->isairq[0], d->isairq[1]);
+} else if (d->isairq[0] != -1) {
+monitor_printf(mon, "%*sisa irq %d\n", indent, "",
+   d->isairq[0]);
+}
+}
+
 static void isa_device_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *k = DEVICE_CLASS(klass);
 k->init = isa_qdev_init;
 k->bus_type = TYPE_ISA_BUS;
 k->get_fw_dev_path = isa_device_get_fw_dev_path;
+k->print_dev = isa_qdev_dev_print;
 }
 
 static TypeInfo isa_device_type_info = {
diff --git a/hw/pci.c b/hw/pci.c
index 425ceaa..39c44b2 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -39,14 +39,12 @@
 # define PCI_DPRINTF(format, ...)   do { } while (0)
 #endif
 
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static int pcibus_reset(BusState *qbus);
 
 static void pci_bus_class_init(ObjectClass *klass, void *data)
 {
 BusClass *k = BUS_CLASS(klass);
 
-k->print_dev = pcibus_dev_print;
 k->reset = pcibus_reset;
 }
 
@@ -1815,44 +1813,6 @@ uint8_t pci_find_capability(PCIDevice *pdev, uint8_t 
cap_id)
 return pci_find_capability_list(pdev, cap_id, NULL);
 }
 
-static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent)
-{
-PCIDevice *d = (PCIDevice *)dev;
-const pci_class_desc *desc;
-char ctxt[64];
-PCIIORegion *r;
-int i, class;
-
-class = pci_get_word(d->config + PCI_CLASS_DEVICE);
-desc = pci_class_descriptions;
-while (desc->desc && class != desc->class)
-desc++;
-if (desc->desc) {
-snprintf(ctxt, sizeof(ctxt), "%s", desc->desc);
-} else {
-snprintf(ctxt, sizeof(ctxt), "Class %04x", class);
-}
-
-monitor_printf(mon, "%*sclass %s, addr %02x:%02x.%x, "
-   "pci id %04x:%04x (sub %04x:%04x)\n",
-   indent, "", ctxt, pci_bus_num(d->bus),
-   PCI_SLOT(d->devfn), PCI_FUNC(d->devfn),
-   pci_get_word(d->config + PCI_VENDOR_ID),
-   pci_get_word(d->config + PCI_DEVICE_ID),
-   pci_get_word(d->config + PCI_SUBSYSTEM_VENDOR_ID),
-   pci_get_word(d->config + PCI_SUBSYSTEM_ID));
-for (i = 0; i < PCI_NUM_REGIONS; i++) {
-r = &d->io_regions[i];
-if (!r->size)
-continue;
-monitor_printf(mon, "%*sbar %d: %s at 0x%"FMT_PCIBUS
-   " [0x%"FMT_PCIBUS"]\n",
-   indent, "",
-   i, r->type & PCI_BASE_ADDRESS_SPACE_IO ? "i/o" : "mem",
-   r->addr, r->addr + r->size - 1);
-}
-}
-
 static char *pci_dev_fw_name(DeviceState *dev, char *buf, int len)
 {
 PCIDevice *d = (PCIDevice *)dev;
@@ -1989,6 +1949,44 @@ static char *pci_qdev_get_fw_dev_path(DeviceState *dev)
 return strdup(path);
 }
 
+static void pci_qdev_dev_print(DeviceState *dev, Monitor *mon, int indent)
+{
+PCIDevice 

[Qemu-devel] [PATCH 07/14] qdev: fix info qtree/qdm

[Anthony Liguori]

Don't rely on bus_info.  I took a little liberty in the last commit as it would
cause info qtree/info qdm to not show any useful information.  But since this
is not considered a supported interface, breaking it across a single commit
seems okay.

This commit makes info qtree/qdm work again.

Signed-off-by: Anthony Liguori 
---
 hw/qdev-monitor.c|  120 --
 hw/qdev-properties.c |   30 +++-
 hw/qdev.c|   26 +++
 hw/qdev.h|1 -
 4 files changed, 83 insertions(+), 94 deletions(-)

diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index dc4e4e1..f5ee166 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -118,17 +118,44 @@ static const char *find_typename_by_alias(const char 
*alias)
 return NULL;
 }
 
+static void display_property(Object *obj, const char *name,
+ const char *typename, bool read_only,
+ void *opaque)
+{
+char *legacy_typename;
+
+if (!strstart(typename, "legacy<", NULL)) {
+return;
+}
+
+legacy_typename = g_strdup(&typename[7]);
+legacy_typename[strlen(legacy_typename) - 1] = 0;
+
+/*
+ * TODO Properties without a parser are just for dirty hacks.
+ * qdev_prop_ptr is the only such PropertyInfo.  It's marked
+ * for removal.  This conditional should be removed along with
+ * it.
+ */
+if (!read_only) {
+error_printf("%s.%s=%s\n", object_get_typename(obj),
+ &name[7], legacy_typename);
+}
+
+g_free(legacy_typename);
+}
+
 int qdev_device_help(QemuOpts *opts)
 {
 const char *driver;
-Property *prop;
 ObjectClass *klass;
-DeviceClass *info;
+Object *obj;
 
 driver = qemu_opt_get(opts, "driver");
 if (driver && !strcmp(driver, "?")) {
 bool show_no_user = false;
-object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, 
&show_no_user);
+object_class_foreach(qdev_print_devinfo, TYPE_DEVICE,
+ false, &show_no_user);
 return 1;
 }
 
@@ -149,30 +176,11 @@ int qdev_device_help(QemuOpts *opts)
 if (!klass) {
 return 0;
 }
-info = DEVICE_CLASS(klass);
-
-for (prop = info->props; prop && prop->name; prop++) {
-/*
- * TODO Properties without a parser are just for dirty hacks.
- * qdev_prop_ptr is the only such PropertyInfo.  It's marked
- * for removal.  This conditional should be removed along with
- * it.
- */
-if (!prop->info->parse) {
-continue;   /* no way to set it, don't show */
-}
-error_printf("%s.%s=%s\n", driver, prop->name,
- prop->info->legacy_name ?: prop->info->name);
-}
-if (info->bus_info) {
-for (prop = info->bus_info->props; prop && prop->name; prop++) {
-if (!prop->info->parse) {
-continue;   /* no way to set it, don't show */
-}
-error_printf("%s.%s=%s\n", driver, prop->name,
- prop->info->legacy_name ?: prop->info->name);
-}
-}
+
+obj = object_new(driver);
+object_property_foreach(obj, display_property, NULL);
+object_delete(obj);
+
 return 1;
 }
 
@@ -481,50 +489,56 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## 
__VA_ARGS__)
 static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
-static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
- const char *prefix, int indent)
+typedef struct QTreePrinter
 {
-if (!props)
-return;
-for (; props->name; props++) {
-Error *err = NULL;
-char *value;
-char *legacy_name = g_strdup_printf("legacy-%s", props->name);
-if (object_property_get_type(OBJECT(dev), legacy_name, NULL)) {
-value = object_property_get_str(OBJECT(dev), legacy_name, &err);
-} else {
-value = object_property_get_str(OBJECT(dev), props->name, &err);
-}
-g_free(legacy_name);
+Visitor visitor;
+Monitor *mon;
+int indent;
+const char *prefix;
+} QTreePrinter;
+
+static void qtree_printer_visit_str(Visitor *v, char **obj,
+const char *name, Error **errp)
+{
+QTreePrinter *p = (QTreePrinter *)v;
 
-if (err) {
-error_free(err);
-continue;
-}
-qdev_printf("%s-prop: %s = %s\n", prefix, props->name,
-value && *value ? value : "");
-g_free(value);
+monitor_printf(p->mon, "%*sdev-prop: %s = %s\n",
+   p->indent, "", &name[7], *obj);
+}
+
+static void qdev_print_prop(Object *obj, const char *name,
+ const char *typename, bool read_only,
+ void *opaque)
+{
+

[Qemu-devel] [PATCH 14/14] qbus: initialize in standard way

[Anthony Liguori]

Signed-off-by: Anthony Liguori 
---
 hw/qdev.c |   84 +
 1 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index 166c599..f637351 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -434,40 +434,35 @@ DeviceState *qdev_find_recursive(BusState *bus, const 
char *id)
 return NULL;
 }
 
-/* FIXME move this logic into instance_init */
-static void do_qbus_create_inplace(BusState *bus, const char *typename,
-   DeviceState *parent, const char *name)
+static void qbus_realize(BusState *bus)
 {
+const char *typename = object_get_typename(OBJECT(bus));
 char *buf;
 int i,len;
 
-bus->parent = parent;
-
-if (name) {
+if (bus->name) {
 /* use supplied name */
-bus->name = g_strdup(name);
-} else if (parent && parent->id) {
+} else if (bus->parent && bus->parent->id) {
 /* parent device has id -> use it for bus name */
-len = strlen(parent->id) + 16;
+len = strlen(bus->parent->id) + 16;
 buf = g_malloc(len);
-snprintf(buf, len, "%s.%d", parent->id, parent->num_child_bus);
+snprintf(buf, len, "%s.%d", bus->parent->id, 
bus->parent->num_child_bus);
 bus->name = buf;
 } else {
 /* no id -> use lowercase bus type for bus name */
 len = strlen(typename) + 16;
 buf = g_malloc(len);
 len = snprintf(buf, len, "%s.%d", typename,
-   parent ? parent->num_child_bus : 0);
+   bus->parent ? bus->parent->num_child_bus : 0);
 for (i = 0; i < len; i++)
 buf[i] = qemu_tolower(buf[i]);
 bus->name = buf;
 }
 
-QTAILQ_INIT(&bus->children);
-if (parent) {
-QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
-parent->num_child_bus++;
-object_property_add_child(OBJECT(parent), bus->name, OBJECT(bus), 
NULL);
+if (bus->parent) {
+QLIST_INSERT_HEAD(&bus->parent->child_bus, bus, sibling);
+bus->parent->num_child_bus++;
+object_property_add_child(OBJECT(bus->parent), bus->name, OBJECT(bus), 
NULL);
 } else if (bus != main_system_bus) {
 /* TODO: once all bus devices are qdevified,
only reset handler for main_system_bus should be registered here. */
@@ -479,7 +474,10 @@ void qbus_create_inplace(BusState *bus, const char 
*typename,
  DeviceState *parent, const char *name)
 {
 object_initialize(bus, typename);
-do_qbus_create_inplace(bus, typename, parent, name);
+
+bus->parent = parent;
+bus->name = name ? g_strdup(name) : NULL;
+qbus_realize(bus);
 }
 
 BusState *qbus_create(const char *typename, DeviceState *parent, const char 
*name)
@@ -488,7 +486,11 @@ BusState *qbus_create(const char *typename, DeviceState 
*parent, const char *nam
 
 bus = BUS(object_new(typename));
 bus->qdev_allocated = 1;
-do_qbus_create_inplace(bus, typename, parent, name);
+
+bus->parent = parent;
+bus->name = name ? g_strdup(name) : NULL;
+qbus_realize(bus);
+
 return bus;
 }
 
@@ -504,22 +506,10 @@ static void main_system_bus_create(void)
 
 void qbus_free(BusState *bus)
 {
-BusChild *kid;
-
-while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
-DeviceState *dev = kid->child;
-qdev_free(dev);
-}
-if (bus->parent) {
-QLIST_REMOVE(bus, sibling);
-bus->parent->num_child_bus--;
-} else {
-assert(bus != main_system_bus); /* main_system_bus is never freed */
-qemu_unregister_reset(qbus_reset_all_fn, bus);
-}
-g_free((void*)bus->name);
 if (bus->qdev_allocated) {
-g_free(bus);
+object_delete(OBJECT(bus));
+} else {
+object_finalize(OBJECT(bus));
 }
 }
 
@@ -753,12 +743,40 @@ static TypeInfo device_type_info = {
 .class_size = sizeof(DeviceClass),
 };
 
+static void qbus_initfn(Object *obj)
+{
+BusState *bus = BUS(obj);
+
+QTAILQ_INIT(&bus->children);
+}
+
+static void qbus_finalize(Object *obj)
+{
+BusState *bus = BUS(obj);
+BusChild *kid;
+
+while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
+DeviceState *dev = kid->child;
+qdev_free(dev);
+}
+if (bus->parent) {
+QLIST_REMOVE(bus, sibling);
+bus->parent->num_child_bus--;
+} else {
+assert(bus != main_system_bus); /* main_system_bus is never freed */
+qemu_unregister_reset(qbus_reset_all_fn, bus);
+}
+g_free((void*)bus->name);
+}
+
 static TypeInfo bus_info = {
 .name = TYPE_BUS,
 .parent = TYPE_OBJECT,
 .instance_size = sizeof(BusState),
 .abstract = true,
 .class_size = sizeof(BusClass),
+.instance_init = qbus_initfn,
+.instance_finalize = qbus_finalize,
 };
 
 static void qdev_register_types(void)
-- 
1.7.5.4

[Qemu-devel] [PATCH 05/14] qdev: use wrapper for qdev_get_path

[Anthony Liguori]

This makes it easier to remove it from BusInfo.

Signed-off-by: Anthony Liguori 
---
 exec.c|4 ++--
 hw/qdev.c |   16 
 hw/qdev.h |2 ++
 hw/usb/desc.c |7 +--
 savevm.c  |   12 ++--
 5 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 0607c9b..e3523d2 100644
--- a/exec.c
+++ b/exec.c
@@ -2583,8 +2583,8 @@ void qemu_ram_set_idstr(ram_addr_t addr, const char 
*name, DeviceState *dev)
 assert(new_block);
 assert(!new_block->idstr[0]);
 
-if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-char *id = dev->parent_bus->info->get_dev_path(dev);
+if (dev) {
+char *id = qdev_get_dev_path(dev);
 if (id) {
 snprintf(new_block->idstr, sizeof(new_block->idstr), "%s/", id);
 g_free(id);
diff --git a/hw/qdev.c b/hw/qdev.c
index e17a9ab..e835650 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -519,6 +519,22 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
 return strdup(path);
 }
 
+char *qdev_get_dev_path(DeviceState *dev)
+{
+BusClass *bc;
+
+if (!dev->parent_bus) {
+return NULL;
+}
+
+bc = BUS_GET_CLASS(dev->parent_bus);
+if (bc->get_dev_path) {
+return bc->get_dev_path(dev);
+}
+
+return NULL;
+}
+
 static char *qdev_get_type(Object *obj, Error **errp)
 {
 return g_strdup(object_get_typename(obj));
diff --git a/hw/qdev.h b/hw/qdev.h
index ca8386a..fc3b50f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -362,4 +362,6 @@ extern int qdev_hotplug;
 
 void qdev_add_properties(DeviceState *dev, Property *props);
 
+char *qdev_get_dev_path(DeviceState *dev);
+
 #endif
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index e8a3c6a..64352c9 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -433,11 +433,14 @@ void usb_desc_create_serial(USBDevice *dev)
 int index = desc->id.iSerialNumber;
 char serial[64];
 int dst;
+char *path = NULL;
 
 assert(index != 0 && desc->str[index] != NULL);
 dst = snprintf(serial, sizeof(serial), "%s", desc->str[index]);
-if (hcd && hcd->parent_bus && hcd->parent_bus->info->get_dev_path) {
-char *path = hcd->parent_bus->info->get_dev_path(hcd);
+if (hcd->parent_bus && hcd->parent_bus->parent) {
+path = qdev_get_dev_path(hcd->parent_bus->parent);
+}
+if (path) {
 dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", path);
 }
 dst += snprintf(serial+dst, sizeof(serial)-dst, "-%s", dev->port->path);
diff --git a/savevm.c b/savevm.c
index 2d18bab..818ddfc 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1248,8 +1248,8 @@ int register_savevm_live(DeviceState *dev,
 se->is_ram = 1;
 }
 
-if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-char *id = dev->parent_bus->info->get_dev_path(dev);
+if (dev) {
+char *id = qdev_get_dev_path(dev);
 if (id) {
 pstrcpy(se->idstr, sizeof(se->idstr), id);
 pstrcat(se->idstr, sizeof(se->idstr), "/");
@@ -1292,8 +1292,8 @@ void unregister_savevm(DeviceState *dev, const char 
*idstr, void *opaque)
 SaveStateEntry *se, *new_se;
 char id[256] = "";
 
-if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-char *path = dev->parent_bus->info->get_dev_path(dev);
+if (dev) {
+char *path = qdev_get_dev_path(dev);
 if (path) {
 pstrcpy(id, sizeof(id), path);
 pstrcat(id, sizeof(id), "/");
@@ -1334,8 +1334,8 @@ int vmstate_register_with_alias_id(DeviceState *dev, int 
instance_id,
 se->alias_id = alias_id;
 se->no_migrate = vmsd->unmigratable;
 
-if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-char *id = dev->parent_bus->info->get_dev_path(dev);
+if (dev) {
+char *id = qdev_get_dev_path(dev);
 if (id) {
 pstrcpy(se->idstr, sizeof(se->idstr), id);
 pstrcat(se->idstr, sizeof(se->idstr), "/");
-- 
1.7.5.4

[Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name

[Anthony Liguori]

This is technically a compatibility breaker.  However:

1) libvirt does not rely on this (it always uses the driver name)

2) This behavior isn't actually documented anywhere (the docs just say driver).

3) I suspect there are less than three people on earth that even know this is
   possible (minus the people reading this message).

So I think we can safely break it :-)

Signed-off-by: Anthony Liguori 
---
 hw/qdev-properties.c |3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 98dd06a..894fa79 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1172,8 +1172,7 @@ void qdev_prop_set_globals(DeviceState *dev)
 GlobalProperty *prop;
 
 QTAILQ_FOREACH(prop, &global_props, next) {
-if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0 &&
-strcmp(qdev_get_bus_info(dev)->name, prop->driver) != 0) {
+if (strcmp(object_get_typename(OBJECT(dev)), prop->driver) != 0) {
 continue;
 }
 if (qdev_prop_parse(dev, prop->property, prop->value) != 0) {
-- 
1.7.5.4