Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
Anthony Liguori anth...@codemonkey.ws wrote on 05/01/2012 06:26:05 PM:
Anthony Liguori anth...@codemonkey.ws
05/01/2012 06:26 PM
To
Paul Moore pmo...@redhat.com
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 Moorepmo...@redhat.com
---
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
+#includesyslog.h
#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
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
Anthony Liguori anth...@codemonkey.ws wrote on 05/01/2012 06:45:47 PM:
Anthony Liguori anth...@codemonkey.ws
05/01/2012 06:45 PM
To
George Wilson/Austin/IBM@IBMUS
cc
Paul Moore pmo...@redhat.com, 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 Liguorianth...@codemonkey.ws wrote on 05/01/2012 06:26:05
PM:
Anthony Liguorianth...@codemonkey.ws
05/01/2012 06:26 PM
To
Paul Moorepmo...@redhat.com
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 Moorepmo...@redhat.com
---
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
+#includesyslog.h
#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
Re: [Qemu-devel] [PATCH 04/14] qdev: don't allow globals to be set by bus name
Il 02/05/2012 00:18, Anthony Liguori ha scritto:
anthony@titi:~/build/qemu$ x86_64-softmmu/qemu-system-x86_64 -device
rtl8139,?
there is no output
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 2/3] runstate: introduce suspended state
diff --git a/input.c b/input.c
index 6b5c2c3..47e6900 100644
--- a/input.c
+++ b/input.c
@@ -130,7 +130,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
void kbd_put_keycode(int keycode)
{
-if (!runstate_is_running()) {
+if (!runstate_is_running() !runstate_check(RUN_STATE_SUSPENDED)) {
return;
}
if (qemu_put_kbd_event) {
IIRC there is a simliar check for the mouse ...
Does it make sense to add a runstate_is_running_or_suspended() function?
Overall the series looks good to me.
cheers,
Gerd
Re: [Qemu-devel] [PATCH 07/14] qdev: fix info qtree/qdm
Il 01/05/2012 20:18, Anthony Liguori ha scritto: 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. It omits all non-legacy properties, so I think it doesn't. Paolo
Re: [Qemu-devel] [PATCH 10/14] qbus: move get_dev_path to DeviceState
Il 01/05/2012 20:18, Anthony Liguori ha scritto:
It should have never been a bus method.
If in the long term you want slots to be child properties in the bus,
the method _will_ actually belong to buses.
It is clear cut for print_dev, but not so much for the others.
Paolo
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
---
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
Re: [Qemu-devel] [PATCH 11/14] qbus: move get_fw_dev_path to DeviceClass
Il 02/05/2012 00:36, Andreas Färber ha scritto: 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. A lot of these were missing, and furthermore they should be in a header file. Paolo
Re: [Qemu-devel] [V6 PATCH 1/4] net: announce self after vm start
On 03/28/2012 07:40 AM, Jason Wang wrote:
qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. A global variable
need_announce were introduced to record the pending announcement, and
vm_start() would send gratuitous packet depends on this value.
Signed-off-by: Jason Wang jasow...@redhat.com
---
migration.c |2 +-
migration.h |2 ++
vl.c|5 +
3 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/migration.c b/migration.c
index 00fa1e3..861cce9 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f)
fprintf(stderr, load of migration failed\n);
exit(0);
}
-qemu_announce_self();
+need_announce = true;
DPRINTF(successfully loaded vm state\n);
/* Make sure all file formats flush their mutable metadata */
diff --git a/migration.h b/migration.h
index 372b066..0a31463 100644
--- a/migration.h
+++ b/migration.h
@@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
+extern bool need_announce;
+
Hi Jason,
I don't like this external flag.
As this is only related to migration I think we can add a new state
RUN_STATE_MIG_PRELAUNCH.
In vm_start call qemu_announce_self only if the state was
RUN_STATE_MIG_PRELAUNCH.
This will we useful if we will need to do something else when resuming a
migrated guest.
Regards,
Orit
#endif
diff --git a/vl.c b/vl.c
index 65f11f2..05ebf57 100644
--- a/vl.c
+++ b/vl.c
@@ -231,6 +231,7 @@ int boot_menu;
uint8_t *boot_splash_filedata;
int boot_splash_filedata_size;
uint8_t qemu_extra_params_fw[2];
+bool need_announce = false;
typedef struct FWBootEntry FWBootEntry;
@@ -1266,6 +1267,10 @@ void vm_start(void)
vm_state_notify(1, RUN_STATE_RUNNING);
resume_all_vcpus();
monitor_protocol_event(QEVENT_RESUME, NULL);
+if (need_announce) {
+need_announce = false;
+qemu_announce_self();
+}
}
}
Re: [Qemu-devel] [V6 PATCH 1/4] net: announce self after vm start
On 05/02/2012 03:49 PM, Orit Wasserman wrote:
On 03/28/2012 07:40 AM, Jason Wang wrote:
qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. A global variable
need_announce were introduced to record the pending announcement, and
vm_start() would send gratuitous packet depends on this value.
Signed-off-by: Jason Wangjasow...@redhat.com
---
migration.c |2 +-
migration.h |2 ++
vl.c|5 +
3 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/migration.c b/migration.c
index 00fa1e3..861cce9 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,7 @@ void process_incoming_migration(QEMUFile *f)
fprintf(stderr, load of migration failed\n);
exit(0);
}
-qemu_announce_self();
+need_announce = true;
DPRINTF(successfully loaded vm state\n);
/* Make sure all file formats flush their mutable metadata */
diff --git a/migration.h b/migration.h
index 372b066..0a31463 100644
--- a/migration.h
+++ b/migration.h
@@ -95,4 +95,6 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
+extern bool need_announce;
+
Hi Jason,
I don't like this external flag.
As this is only related to migration I think we can add a new state
RUN_STATE_MIG_PRELAUNCH.
In vm_start call qemu_announce_self only if the state was
RUN_STATE_MIG_PRELAUNCH.
This will we useful if we will need to do something else when resuming a
migrated guest.
Regards,
Orit
Hi Orit:
No problem, the only reason of using global variable is simplicity, we
thought to add one new run state in the past. I would add one like what
you suggested.
Thanks
#endif
diff --git a/vl.c b/vl.c
index 65f11f2..05ebf57 100644
--- a/vl.c
+++ b/vl.c
@@ -231,6 +231,7 @@ int boot_menu;
uint8_t *boot_splash_filedata;
int boot_splash_filedata_size;
uint8_t qemu_extra_params_fw[2];
+bool need_announce = false;
typedef struct FWBootEntry FWBootEntry;
@@ -1266,6 +1267,10 @@ void vm_start(void)
vm_state_notify(1, RUN_STATE_RUNNING);
resume_all_vcpus();
monitor_protocol_event(QEVENT_RESUME, NULL);
+if (need_announce) {
+need_announce = false;
+qemu_announce_self();
+}
}
}
Re: [Qemu-devel] [PATCH 1/3] raw-posix: Do not use CONFIG_COCOA macro
Am 01.05.2012 00:15, schrieb Andreas Färber: From: Pavel Borzenkov pavel.borzen...@gmail.com Use __APPLE__ and __MACH__ macros instead of CONFIG_COCOA to detect Mac OS X host. The patch is based on Ben Leslie's patch: http://patchwork.ozlabs.org/patch/97859/ Signed-off-by: Ben Leslie be...@benno.id.au Signed-off-by: Pavel Borzenkov pavel.borzen...@gmail.com Signed-off-by: Andreas Färber andreas.faer...@web.de --- block/raw-posix.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 2d1bc13..03fcfcc 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -29,7 +29,7 @@ #include module.h #include block/raw-posix-aio.h -#ifdef CONFIG_COCOA +#if defined(__APPLE__) (__MACH__) Is there a 'defined' missing? Kevin
Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd
Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? This doesn't look like something that I'd like a lot. qemu should be able to continue to run no matter what the management tool does, whether it responds to RPCs properly or whether it has crashed. You need a really good use case for the RPC that cannot be covered otherwise in order to justify this. Kevin
Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd
On Wed, May 2, 2012 at 9:20 AM, Kevin Wolf kw...@redhat.com wrote: Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? It solves the problem of snapshot_blkdev and other operations that re-open files. Using -blockdev and hotplug for image files as file descriptors only solves the static configuration problem, not the runtime problem we get with snapshot_blkdev. That's why this approach is more powerful than -blockdev fd=N. Stefan
Re: [Qemu-devel] [V6 PATCH 1/4] net: announce self after vm start
Il 02/05/2012 09:59, Jason Wang ha scritto: I don't like this external flag. As this is only related to migration I think we can add a new state RUN_STATE_MIG_PRELAUNCH. In vm_start call qemu_announce_self only if the state was RUN_STATE_MIG_PRELAUNCH. This will we useful if we will need to do something else when resuming a migrated guest. I don't think this global is replacing a runstate. It is simply saying that some work has been delayed to the next time the CPU runs. Perhaps we can instead use a runstate-change notifier and keep the global hidden to vl.c. Paolo
[Qemu-devel] API for single stepping an emulated CPU
Dear developers, I am designing a virtual peripheral for Qemu, for which I need to single step through the program on the emulated CPU (my peripheral will be some kind of a debugger). My question is: is there an API to execute exactly one instruction in Qemu? I've already found the *_pause and *_resume functions, but they are - as far as I know - not applicable for my task. Regards, Gabor Wacha EE student at Budapest University of Technology an Economics
Re: [Qemu-devel] [PATCH 14/14] qbus: initialize in standard way
Il 01/05/2012 20:18, Anthony Liguori ha scritto:
if (bus-qdev_allocated) {
-g_free(bus);
+object_delete(OBJECT(bus));
+} else {
+object_finalize(OBJECT(bus));
}
Time is ripe to add a more versatile freeing mechanism along the lines
you've set in the past (either a callback or a notifier that object_new
would set to a g_free wrapper). Anthony, are you going to do this?
Paolo
Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd
On Wed, May 02, 2012 at 10:20:17AM +0200, Kevin Wolf wrote: Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? This doesn't look like something that I'd like a lot. qemu should be able to continue to run no matter what the management tool does, whether it responds to RPCs properly or whether it has crashed. You need a really good use case for the RPC that cannot be covered otherwise in order to justify this. Indeed, this solution breaks if you stop or restart libvirtd while QEMU is running. Restarting libvirt while QEMU is running is something we must support, since installing RPM updates will restart libvirtd and we cannot let guests die in this case. I would much prefer to see us be able to pass FDs in directly alongside the disk config as we do for netdev TAP/etc, and for QEMU / kernel to be fixed so that you do not need to re-open FDs on the fly. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC 2/5] block: add new command line parameter that and protocol description
On Tue, May 01, 2012 at 04:31:44PM +0100, Stefan Hajnoczi wrote:
From: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
qemu-options.hx | 42 ++
1 file changed, 42 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index a169792..ccf4d1d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2724,6 +2724,48 @@ DEF(qtest-log, HAS_ARG, QEMU_OPTION_qtest_log,
-qtest-log LOG specify tracing options\n,
QEMU_ARCH_ALL)
+DEF(open-hook-fd, HAS_ARG, QEMU_OPTION_open_hook_fd,
+-open-hook-fd fd\n
+delegate opens to external process using fd\n,
QEMU_ARCH_ALL)
+STEXI
+@item -open-hook-fd @var{fd}
+@findex -open-hook-fd
+Delegates open()s to an external process using @varfd to communicate
commands.
+@varfd should be an open Unix Domain socket pipe that file descriptors can
be
+received from. The protocol the socket uses is a simple request/response
initiated
+by the client. All integers are in host byte order. It is assumed that
this protocol
+is only ever used on the same physical machine. It is currently defined as:
+
+u32 message_size
+u32 command
+u8 payload[message_size - 8]
+
+The contents of payload depend on command. Currently the following commands
are
+defined:
+
+1. QEMU_OPEN (1)
+
+The full message will be:
+
+u32 message_size
+u32 command = 1
+u32 flags (O_ flags defined by libc)
+u32 mode (mode_t flags as defined by libc)
+u16 filename_len;
+u8 filename[filename_len]
I think this should include the ID of the backend of the drive wanting
this file opened, so you can reliably match up to the -drive that mgmt
configured at startup. ie not have to search through every single
device in the guest all their backing files to find if there is a
match.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd
On Tue, May 01, 2012 at 04:31:42PM +0100, 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. While it would work, I really am not a fan of this approach overall, since I think it adds significant complexity great potential for problems. In particular the potential for deadlock between the mgmt app QEMU (sure we should design to avoid deadlocks, but mistakes happen). It is also not resilient against the mgmt app going away (whether due to crash, or intentional restart for RPM upgrades). I strongly prefer to see FDs being passed in at time of device configuration/creation as with netdevs, and then fixing the places which re-open files to not require that. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Tue, May 01, 2012 at 06:26:05PM -0500, Anthony Liguori wrote: 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? All the SSL libraries for a start (NSS, OpenSSL GNUTLS). If we were using one of those for the VNC DES code we would be disabled. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Tue, May 01, 2012 at 05:20:40PM -0400, 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.
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 pmo...@redhat.com
---
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 syslog.h
#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)
s/int/bool/ and use true/false as values
+{
+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;
+}
As already pointed out,wWe should probably make this depend on
__linux__, and 'return false' fo other platforms.
+
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);
+}
I think this syslog message is better placed in the next chunk of the
patch where you actually test the vs-fips value.
+
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;
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Tue, May 01, 2012 at 06:45:47PM -0500, Anthony Liguori wrote: On 05/01/2012 06:43 PM, George Wilson wrote: Anthony Liguorianth...@codemonkey.ws wrote on 05/01/2012 06:26:05 PM: Anthony Liguorianth...@codemonkey.ws 05/01/2012 06:26 PM To Paul Moorepmo...@redhat.com 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. In the wonderful world of security certifications, sensible is rarely a relevant factor. To answer your point though, FIPS itself is just about restricting use of insecure algorithms. Whether an application uses authentication or not, is outside the scope of that. Other security standards are responsible for dictating whether authentication is mandatory or not. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [RFC 0/5] block: File descriptor passing using -open-hook-fd
Am 02.05.2012 10:27, schrieb Stefan Hajnoczi: On Wed, May 2, 2012 at 9:20 AM, Kevin Wolf kw...@redhat.com wrote: Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? It solves the problem of snapshot_blkdev and other operations that re-open files. Using -blockdev and hotplug for image files as file descriptors only solves the static configuration problem, not the runtime problem we get with snapshot_blkdev. That's why this approach is more powerful than -blockdev fd=N. The important thing that blockdev-add must define is a QAPI type that describes a block device. This is the same type that can be used for all other operations as well. In fact, live snapshots don't even require this. You just pass fd:42 (or actually whatever the -blockdev syntax for it will be) as the new image and use 'existing' mode. You don't even need blockdev-add for this, you can do it with what we have today. (Warning, hack: You can even use live snapshots with existing files to describe the full backing file structure today and use only FDs. The only new thing you need there is the fd protocol. But... don't use this.) Kevin
Re: [Qemu-devel] [PATCH 1/3] raw-posix: Do not use CONFIG_COCOA macro
Am 02.05.2012 10:02, schrieb Kevin Wolf: Am 01.05.2012 00:15, schrieb Andreas Färber: From: Pavel Borzenkov pavel.borzen...@gmail.com Use __APPLE__ and __MACH__ macros instead of CONFIG_COCOA to detect Mac OS X host. The patch is based on Ben Leslie's patch: http://patchwork.ozlabs.org/patch/97859/ Signed-off-by: Ben Leslie be...@benno.id.au Signed-off-by: Pavel Borzenkov pavel.borzen...@gmail.com Signed-off-by: Andreas Färber andreas.faer...@web.de --- block/raw-posix.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 2d1bc13..03fcfcc 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -29,7 +29,7 @@ #include module.h #include block/raw-posix-aio.h -#ifdef CONFIG_COCOA +#if defined(__APPLE__) (__MACH__) Is there a 'defined' missing? Seems like it, I'll send you a fixup patch. Thanks for noticing, Andreas
Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd
Am 02.05.2012 10:53, schrieb Daniel P. Berrange: On Wed, May 02, 2012 at 10:20:17AM +0200, Kevin Wolf wrote: Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? This doesn't look like something that I'd like a lot. qemu should be able to continue to run no matter what the management tool does, whether it responds to RPCs properly or whether it has crashed. You need a really good use case for the RPC that cannot be covered otherwise in order to justify this. Indeed, this solution breaks if you stop or restart libvirtd while QEMU is running. Restarting libvirt while QEMU is running is something we must support, since installing RPM updates will restart libvirtd and we cannot let guests die in this case. I would much prefer to see us be able to pass FDs in directly alongside the disk config as we do for netdev TAP/etc, and for QEMU / kernel to be fixed so that you do not need to re-open FDs on the fly. I agree, and this is what -blockdev would give us. Part of why I don't like the RFC (apart from RPCing the management tool being just wrong) is that once again it's trying to take shortcuts and only provide a hack for the urgent need instead of doing it properly and implementing -blockdev. I suspect that if we take something half-baked like this, we will keep being unhappy with the situation in the block layer, but it won't hurt enough any more to actually spend effort on it, so that we'll go another five years with it. Kevin
Re: [Qemu-devel] [RFC 2/5] block: add new command line parameter that and protocol description
On Tue, May 01, 2012 at 04:31:44PM +0100, Stefan Hajnoczi wrote:
From: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com
---
qemu-options.hx | 42 ++
1 file changed, 42 insertions(+)
diff --git a/qemu-options.hx b/qemu-options.hx
index a169792..ccf4d1d 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2724,6 +2724,48 @@ DEF(qtest-log, HAS_ARG, QEMU_OPTION_qtest_log,
-qtest-log LOG specify tracing options\n,
QEMU_ARCH_ALL)
+DEF(open-hook-fd, HAS_ARG, QEMU_OPTION_open_hook_fd,
+-open-hook-fd fd\n
+delegate opens to external process using fd\n,
QEMU_ARCH_ALL)
+STEXI
+@item -open-hook-fd @var{fd}
+@findex -open-hook-fd
+Delegates open()s to an external process using @varfd to communicate
commands.
+@varfd should be an open Unix Domain socket pipe that file descriptors can
be
+received from. The protocol the socket uses is a simple request/response
initiated
+by the client. All integers are in host byte order. It is assumed that
this protocol
+is only ever used on the same physical machine. It is currently defined as:
+
+u32 message_size
+u32 command
+u8 payload[message_size - 8]
+
+The contents of payload depend on command. Currently the following commands
are
+defined:
+
+1. QEMU_OPEN (1)
+
+The full message will be:
+
+u32 message_size
+u32 command = 1
+u32 flags (O_ flags defined by libc)
+u32 mode (mode_t flags as defined by libc)
+u16 filename_len;
+u8 filename[filename_len]
+
+The server will then respond with:
+
+u32 message_size
+u32 command = 1
+s32 result
If we're going for a binary protocol, then I'd like to see it defined
based on the XDR specification, so we can auto-generate our data
marshallers/demarshallers using existing tools / libraries and not
have to write something custom by hand. Your spec here is close enough
that it would not be significant work. The changes would be
- Everything is always big-endian
- Each field has 4-byte alignment
- Strings would have a u32 length, and the payload padded with NUL
to the 4 byte boundary
Daniel.
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [libvirt] [RFC 0/5] block: File descriptor passing using -open-hook-fd
On Wed, May 02, 2012 at 11:45:26AM +0200, Kevin Wolf wrote: Am 02.05.2012 10:53, schrieb Daniel P. Berrange: On Wed, May 02, 2012 at 10:20:17AM +0200, Kevin Wolf wrote: Am 01.05.2012 22:25, schrieb 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 Liguorialigu...@us.ibm.com 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. What does it provide on top? This doesn't look like something that I'd like a lot. qemu should be able to continue to run no matter what the management tool does, whether it responds to RPCs properly or whether it has crashed. You need a really good use case for the RPC that cannot be covered otherwise in order to justify this. Indeed, this solution breaks if you stop or restart libvirtd while QEMU is running. Restarting libvirt while QEMU is running is something we must support, since installing RPM updates will restart libvirtd and we cannot let guests die in this case. I would much prefer to see us be able to pass FDs in directly alongside the disk config as we do for netdev TAP/etc, and for QEMU / kernel to be fixed so that you do not need to re-open FDs on the fly. I agree, and this is what -blockdev would give us. Part of why I don't like the RFC (apart from RPCing the management tool being just wrong) is that once again it's trying to take shortcuts and only provide a hack for the urgent need instead of doing it properly and implementing -blockdev. I suspect that if we take something half-baked like this, we will keep being unhappy with the situation in the block layer, but it won't hurt enough any more to actually spend effort on it, so that we'll go another five years with it. I tend to agree - we have been talking about -blockdev for faar to long without (AFAICT) making any real progress towards getting it done. I'd love to see someone bite the bullet have a go at implementing it Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
Am 30.04.2012 17:52, schrieb Michael Tokarev:
This value is used currently for virtio-blk only. It was defined
as uint16_t before, which is the same as in kernel=user interface
(in virtio_blk.h, struct virtio_blk_config). But the problem is
that in kernel=user interface the units are sectors (which is
usually 512 bytes or more), while in qemu it is in bytes. However,
for, say, md raid5 arrays, it is typical to have actual min_io_size
of a host device to be large. For example, for raid5 device of
3 drives with 64Kb chunk size, that value will be 128Kb, which does
not fit in a uint16_t anymore.
Increase the value size from 16bits to 32bits for now.
But apparently, the kernel=user interface needs to be fixed too
(while it is much more difficult due to compatibility issues),
because even with 512byte units, the 16bit value there will overflow
too with quite normal MD RAID configuration.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
block.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.h b/block.h
index f163e54..cd5ae79 100644
--- a/block.h
+++ b/block.h
@@ -425,7 +425,7 @@ typedef struct BlockConf {
BlockDriverState *bs;
uint16_t physical_block_size;
uint16_t logical_block_size;
-uint16_t min_io_size;
+uint32_t min_io_size;
uint32_t opt_io_size;
int32_t bootindex;
uint32_t discard_granularity;
@@ -450,7 +450,7 @@ static inline unsigned int
get_physical_block_exp(BlockConf *conf)
_conf.logical_block_size, 512), \
DEFINE_PROP_BLOCKSIZE(physical_block_size, _state,\
_conf.physical_block_size, 512), \
-DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT32(min_io_size, _state, _conf.min_io_size, 0), \
DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\
DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\
DEFINE_PROP_UINT32(discard_granularity, _state, \
Don't you need an additional check in virtio-blk now so that you don't
overflow the 16 bit field in the virtio protocol? And I guess the same
for SCSI, where INQUIRY reports only a 16 bits sector count as well.
Kevin
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
02.05.2012 13:57, Kevin Wolf пишет:
Am 30.04.2012 17:52, schrieb Michael Tokarev:
This value is used currently for virtio-blk only. It was defined
as uint16_t before, which is the same as in kernel=user interface
(in virtio_blk.h, struct virtio_blk_config). But the problem is
that in kernel=user interface the units are sectors (which is
usually 512 bytes or more), while in qemu it is in bytes. However,
for, say, md raid5 arrays, it is typical to have actual min_io_size
of a host device to be large. For example, for raid5 device of
3 drives with 64Kb chunk size, that value will be 128Kb, which does
not fit in a uint16_t anymore.
Increase the value size from 16bits to 32bits for now.
But apparently, the kernel=user interface needs to be fixed too
(while it is much more difficult due to compatibility issues),
because even with 512byte units, the 16bit value there will overflow
too with quite normal MD RAID configuration.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
block.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.h b/block.h
index f163e54..cd5ae79 100644
--- a/block.h
+++ b/block.h
@@ -425,7 +425,7 @@ typedef struct BlockConf {
BlockDriverState *bs;
uint16_t physical_block_size;
uint16_t logical_block_size;
-uint16_t min_io_size;
+uint32_t min_io_size;
uint32_t opt_io_size;
int32_t bootindex;
uint32_t discard_granularity;
@@ -450,7 +450,7 @@ static inline unsigned int
get_physical_block_exp(BlockConf *conf)
_conf.logical_block_size, 512), \
DEFINE_PROP_BLOCKSIZE(physical_block_size, _state,\
_conf.physical_block_size, 512), \
-DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT32(min_io_size, _state, _conf.min_io_size, 0), \
DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\
DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\
DEFINE_PROP_UINT32(discard_granularity, _state, \
Don't you need an additional check in virtio-blk now so that you don't
overflow the 16 bit field in the virtio protocol? And I guess the same
for SCSI, where INQUIRY reports only a 16 bits sector count as well.
That probably should be added too, but I'm not sure I agree these should be
added into virtio-blk.
As I already mentioned, the virtio protocol has the same defect (but there
it is less serious due to usage of larger units). And that's where the
additional overflow needs to be ELIMINATED, not just checked. Ie, the
protocol should be changed somehow - the only issue is that I don't know
how to do that now, once it has been in use for quite some time.
Note that now, we don't have ANY checks of this theme whatsoever, at all:
I tried using 128K as min_io_size, and the guest sees it as 0 currently, --
the limits (at least the fact that the value fits in the defined number
of bits) should be checked in the common function which implements these
DEFINE_PROP_UINT* defines.
I'm not sure I can fix all of this in one go, so I went on and fixed the
most specific bug first, without any additional bad side effects.
Besides, as I also already mentioned, this whole structure is used in
virtio-blk *only*, so only virtio driver passes this information into
guest, scsi does not use it. And again, in case of scsi, the units are
*sectors*, not bytes.
So maybe the solution is to keep this as 16bits but switch to sectors.
But for this, DEFINE_PROP_UINT can't be used anymore, unless we agree
to change user interface TOO and require the user to specify these
values in sectors to start with.
And yes, if SCSI uses 16bit value here, it will have the same issue
as virtio currently have -- pretty normal MD RAID array can't be
expressed using 16bit number of sectors already... Oh well, but we
at least have a (small) chance to fix virtio.
Thanks,
/mjt
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
+blkcfg.sectors = secs ~(blk_size / pblk_size - 1); I'm not sure here what you mean. Usually blk_size = pblk_size on non-s390 systems, so this is completely different from the previous code, which is effectively I was trying to prevent the masking of the sector number. the first version of the patch simply did blkcfg.sectors = secs; but this broke setups that really need that masking. blkcfg.sectors = secs ~(blk_size / 512 - 1); I wonder if s390 gives a different meaning to logical vs. physical sector sizes, compared to what virtio expects (which is what SCSI says, basically). Physical block sizes on non-s390 systems are really just useful as an alignment hint, they do not affect correctness. Maybe that really points to the problem that we are trying to solve here. For a dasd device, there is usually a 4096 byte block size and on the host these 4096 arereported via getss and getpbsz. The geometry reported by the device driver is usually 15 head and 12 sectors per track, but actually means 12 sectors of 4096 bytes size (a track ~ 48k). What I want to achieve is that the guest view is identical to the host view for cyls, heads, secs, and all block sizes. Christian
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
Il 02/05/2012 12:18, Christian Borntraeger ha scritto: Maybe that really points to the problem that we are trying to solve here. For a dasd device, there is usually a 4096 byte block size and on the host these 4096 arereported via getss and getpbsz. The geometry reported by the device driver is usually 15 head and 12 sectors per track, but actually means 12 sectors of 4096 bytes size (a track ~ 48k). What I want to achieve is that the guest view is identical to the host view for cyls, heads, secs, and all block sizes. I think what you want is _not_ to have the same view as the host. What you want is simply to have a default that is consistent with what is common on actual s390 disks. Perhaps what we want here is to move the guessing of cyls/head/sector count from -drive to -device, so that virtio-blk-s390 can apply a different default (cyls=auto, head=15, sector=12, and also lbsz=pbsz=4096). Markus, have you ever thought about that? I'm a bit torn because these are media parameters rather than device parameters, on the other hand the same applies to lbsz/pbsz at least. Paolo
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 27/04/12 18:12, Paolo Bonzini wrote:
Il 26/04/2012 15:49, Christian Borntraeger ha scritto:
+#ifdef __linux__
+} else if (bdrv_ioctl(bs, HDIO_GETGEO, geo) == 0) {
+*pcyls = geo.cylinders;
+*pheads = geo.heads;
+*psecs = geo.sectors;
+bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
+#endif
Perhaps you could instead move guess_disk_lchs to target-specific code,
adding add this code to the s390-specific implementation and under
#ifdef __s390__. For x86 it doesn't make much sense, because a disk's
geometry most likely will be a wrong guess for the geometry that a guest
(for guests that care at all about geometries).
Fine with me. We care about the geometry only for dasd devices, Even for
FCP-based
SCSI devices on s390 the geometry is not relevant. So moving that part to
s390 specific code might make sense if nobody else needs that.
Is that the case?
Alex, would that be ok for you?
Christian
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Wed, May 02, 2012 at 12:54:21AM +0200, Andreas F??rber wrote: +fds = fopen(/proc/sys/crypto/fips_enabled, r); How standardized is this? Should we limit this to __linux__ or something? It's completelt non-standard and doesn't even exist in mainline Linux. All the FIPS bullshit is a RHEL-private feature, which is where this patch should stay as well.
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
On 02/05/12 12:25, Paolo Bonzini wrote: Il 02/05/2012 12:18, Christian Borntraeger ha scritto: Maybe that really points to the problem that we are trying to solve here. For a dasd device, there is usually a 4096 byte block size and on the host these 4096 arereported via getss and getpbsz. The geometry reported by the device driver is usually 15 head and 12 sectors per track, but actually means 12 sectors of 4096 bytes size (a track ~ 48k). What I want to achieve is that the guest view is identical to the host view for cyls, heads, secs, and all block sizes. I think what you want is _not_ to have the same view as the host. What you want is simply to have a default that is consistent with what is common on actual s390 disks. Let me put it in another way: I want to have these values to match the _device_ that we are passing to the guest because several tools and the partition detection code for a compatible disk format (those that can be accessed by z/OS) needs those values to work properly. That of course means that the guest view is identical to the host view because both views describe a real property of the hardware. IOW the geometry for dasd devices is not an artifical number, it has some real meaning that has a influence on the data structures on the disk. Thing is, the easiest way of getting the hardware property is to query the host. Does that make the situation a bit clearer? Christian
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Wed, May 02, 2012 at 12:28:02PM +0200, Christoph Hellwig wrote:
On Wed, May 02, 2012 at 12:54:21AM +0200, Andreas F??rber wrote:
+fds = fopen(/proc/sys/crypto/fips_enabled, r);
How standardized is this? Should we limit this to __linux__ or something?
It's completelt non-standard and doesn't even exist in mainline Linux.
All the FIPS bullshit is a RHEL-private feature, which is where this patch
should stay as well.
You really should check your facts before making such grand assertions
that are trivially disprovable
It *is* in the mainline kernel:
$ wget https://www.kernel.org/pub/linux/kernel/v3.0/linux-3.3.4.tar.bz2
$ tar jxvf linux-3.3.4.tar.bz2
$ cd linux-3.3.4
$ find | grep -i fips
./crypto/fips.c
./include/linux/fips.h
$ find -type f | xargs grep fips_enabled
./drivers/char/random.c: if (fips_enabled) {
./crypto/fips.c:int fips_enabled;
./crypto/fips.c:EXPORT_SYMBOL_GPL(fips_enabled);
./crypto/fips.c: fips_enabled = !!simple_strtol(str, NULL, 0);
./crypto/fips.c: fips_enabled ? enabled : disabled);
./crypto/tcrypt.c:if (fips_enabled ret == -EINVAL)
./crypto/tcrypt.c:if (!fips_enabled)
./crypto/testmgr.c: if (fips_enabled
!alg_test_descs[i].fips_allowed)
./crypto/testmgr.c: if (fips_enabled ((i = 0
!alg_test_descs[i].fips_allowed) ||
./crypto/testmgr.c: if (fips_enabled rc)
./crypto/testmgr.c: if (fips_enabled !rc)
./crypto/proc.c: .procname = fips_enabled,
./crypto/proc.c: .data = fips_enabled,
./include/linux/fips.h:extern int fips_enabled;
./include/linux/fips.h:#define fips_enabled 0
It is *not* solely RHEL bullshit:
$ cat /etc/fedora-release
Fedora release 17 (Beefy Miracle)
$ rpm -qf /lib64/libfipscheck.so.1
fipscheck-lib-1.3.0-3.fc17.x86_64
It *is* in all upstream crypto libraries:
# wget ftp://ftp.gnupg.org/gcrypt/libgcrypt/libgcrypt-1.5.0.tar.bz2
# tar jxvf libgcrypt-1.5.0.tar.bz2
# cd libgcrypt-1.5.0
# find | grep fips
./random/random-fips.c
./src/fips.c
./tests/fips186-dsa.c
./tests/fipsdrv.c
./doc/fips-fsm.pdf
./doc/fips-fsm.fig
./doc/fips-fsm.png
./doc/fips-fsm.eps
# wget http://www.openssl.org/source/openssl-1.0.1b.tar.gz
# tar zxvf openssl-1.0.1b.tar.gz
# cd openssl-1.0.1b
# find | grep fips
./test/testfipsssl
./crypto/dsa/fips186a.txt
./crypto/o_fips.c
./crypto/evp/evp_fips.c
./crypto/fips_err.h
./crypto/fips_ers.c
# wget
ftp://ftp.mozilla.org/pub/mozilla.org/security/nss/releases/NSS_3_13_4_RTM/src/nss-3.13.4.tar.gz
# tar zxvf nss-3.13.4.tar.gz
# cd nss-3.13.4
# find | grep fips
./mozilla/security/nss/lib/softoken/fipsaudt.c
./mozilla/security/nss/lib/softoken/fipstest.c
./mozilla/security/nss/lib/softoken/fipstokn.c
./mozilla/security/nss/tests/fips
./mozilla/security/nss/tests/fips/fips.sh
./mozilla/security/nss/cmd/fipstest
./mozilla/security/nss/cmd/fipstest/dsa.sh
./mozilla/security/nss/cmd/fipstest/rsa.sh
./mozilla/security/nss/cmd/fipstest/manifest.mn
./mozilla/security/nss/cmd/fipstest/rng.sh
./mozilla/security/nss/cmd/fipstest/fipstest.c
./mozilla/security/nss/cmd/fipstest/sha.sh
./mozilla/security/nss/cmd/fipstest/hmac.sh
./mozilla/security/nss/cmd/fipstest/tdea.sh
./mozilla/security/nss/cmd/fipstest/Makefile
./mozilla/security/nss/cmd/fipstest/ecdsa.sh
./mozilla/security/nss/cmd/fipstest/aes.sh
And fully documented by upstreams too
http://www.gnupg.org/documentation/manuals/gcrypt/Enabling-FIPS-mode.html
https://www.mozilla.org/projects/security/pki/nss/fips/
http://www.openssl.org/docs/fips/fipsnotes.html
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
Il 02/05/2012 12:50, Christian Borntraeger ha scritto: On 02/05/12 12:25, Paolo Bonzini wrote: Il 02/05/2012 12:18, Christian Borntraeger ha scritto: Maybe that really points to the problem that we are trying to solve here. For a dasd device, there is usually a 4096 byte block size and on the host these 4096 arereported via getss and getpbsz. The geometry reported by the device driver is usually 15 head and 12 sectors per track, but actually means 12 sectors of 4096 bytes size (a track ~ 48k). What I want to achieve is that the guest view is identical to the host view for cyls, heads, secs, and all block sizes. I think what you want is _not_ to have the same view as the host. What you want is simply to have a default that is consistent with what is common on actual s390 disks. Let me put it in another way: I want to have these values to match the _device_ that we are passing to the guest because several tools and the partition detection code for a compatible disk format (those that can be accessed by z/OS) needs those values to work properly. Ah, you never pass part of a disk to a guest and part of the same disk to another? IOW the geometry for dasd devices is not an artifical number, it has some real meaning that has a influence on the data structures on the disk. Yes, I understood this. Paolo
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
On 05/02/2012 12:50 PM, Christian Borntraeger wrote: On 02/05/12 12:25, Paolo Bonzini wrote: Il 02/05/2012 12:18, Christian Borntraeger ha scritto: Maybe that really points to the problem that we are trying to solve here. For a dasd device, there is usually a 4096 byte block size and on the host these 4096 arereported via getss and getpbsz. The geometry reported by the device driver is usually 15 head and 12 sectors per track, but actually means 12 sectors of 4096 bytes size (a track ~ 48k). What I want to achieve is that the guest view is identical to the host view for cyls, heads, secs, and all block sizes. I think what you want is _not_ to have the same view as the host. What you want is simply to have a default that is consistent with what is common on actual s390 disks. Let me put it in another way: I want to have these values to match the _device_ that we are passing to the guest because several tools and the partition detection code for a compatible disk format (those that can be accessed by z/OS) needs those values to work properly. That of course means that the guest view is identical to the host view because both views describe a real property of the hardware. IOW the geometry for dasd devices is not an artifical number, it has some real meaning that has a influence on the data structures on the disk. Thing is, the easiest way of getting the hardware property is to query the host. Does that make the situation a bit clearer? Another thing to consider is that there are 2 generic types of disks: * SCSI disks * DASD disks Both can be accessed using virtio-blk-s390. The former have the same semantics on geometry as what we're used to. They use normal MBRs and essentially the geometry doesn't mean too much these days anymore ;). However, if possible, I would like to diverge these as little as possible from x86 virtio disks. For DASD disks, the geometry is important, as its disk label is usually not MBR, but something s390 specific. That one is different depending on the geometry. So here the geometry is very important. The geometry on the same disk can also be different, depending on how it's formatted. So it's not quite as easy to reconstruct it from the imformation we already have. IIUC if we have the logical block size and the information that it's a DASD disk, we should be able to guess the geometry though. Alex
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
Il 02/05/2012 13:07, Alexander Graf ha scritto: Both can be accessed using virtio-blk-s390. The former have the same semantics on geometry as what we're used to. They use normal MBRs and essentially the geometry doesn't mean too much these days anymore ;). However, if possible, I would like to diverge these as little as possible from x86 virtio disks. If anything, the geometry should be guessed like we do on x86. For DASD disks, the geometry is important, as its disk label is usually not MBR, but something s390 specific. Can we use this to guess the geometry like we do on x86? Paolo
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 05/02/2012 12:27 PM, Christian Borntraeger wrote:
On 27/04/12 18:12, Paolo Bonzini wrote:
Il 26/04/2012 15:49, Christian Borntraeger ha scritto:
+#ifdef __linux__
+} else if (bdrv_ioctl(bs, HDIO_GETGEO,geo) == 0) {
+*pcyls = geo.cylinders;
+*pheads = geo.heads;
+*psecs = geo.sectors;
+bdrv_set_geometry_hint(bs, *pcyls, *pheads, *psecs);
+#endif
Perhaps you could instead move guess_disk_lchs to target-specific code,
adding add this code to the s390-specific implementation and under
#ifdef __s390__. For x86 it doesn't make much sense, because a disk's
geometry most likely will be a wrong guess for the geometry that a guest
(for guests that care at all about geometries).
Fine with me. We care about the geometry only for dasd devices, Even for
FCP-based
SCSI devices on s390 the geometry is not relevant. So moving that part to
s390 specific code might make sense if nobody else needs that.
Is that the case?
Alex, would that be ok for you?
As hinted in my other mail, I think the way to go would be to give a
hint to the geometry code that we're running on a DASD disk. Then we can
* Ask the host device if it can give us its geometry, if so, use it
* Guess depending on the logical block size
and everyone should be happy :). I would really like to have as little
#ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as
it means we won't be able to execise that code path on other architectures.
Alex
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
On 05/02/2012 01:09 PM, Paolo Bonzini wrote: Il 02/05/2012 13:07, Alexander Graf ha scritto: Both can be accessed using virtio-blk-s390. The former have the same semantics on geometry as what we're used to. They use normal MBRs and essentially the geometry doesn't mean too much these days anymore ;). However, if possible, I would like to diverge these as little as possible from x86 virtio disks. If anything, the geometry should be guessed like we do on x86. For DASD disks, the geometry is important, as its disk label is usually not MBR, but something s390 specific. Can we use this to guess the geometry like we do on x86? Yes, but what do you do with a blank disk? :) Alex
Re: [Qemu-devel] [PATCH 1/3] Fix geometry sector calculation
Il 02/05/2012 13:10, Alexander Graf ha scritto: For DASD disks, the geometry is important, as its disk label is usually not MBR, but something s390 specific. Can we use this to guess the geometry like we do on x86? Yes, but what do you do with a blank disk? :) Right. :) Paolo
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? Paolo
[Qemu-devel] [PATCH 01/21] qom: documentation addition
Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- include/qemu/object.h |6 ++ 1 file changed, 6 insertions(+) diff --git a/include/qemu/object.h b/include/qemu/object.h index ca1649c..d315dfa 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -555,6 +555,12 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass, */ const char *object_class_get_name(ObjectClass *klass); +/** + * object_class_by_name: + * @typename: The QOM typename to obtain the class for. + * + * Returns: The class for @typename. + */ ObjectClass *object_class_by_name(const char *typename); void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), -- 1.7.9.3
[Qemu-devel] [PATCH 00/21] qbus QOM conversion, rebased on top of my patches
Here is Anthony's qbus conversion, with the qdev_add_properties changes replaced by my bus property changes. I left out the removal of bus methods; I'm not sure I agree with all of them (though I may just give up :)) and anyway they can be left to 1.2. Following Andreas's remark, I moved all TYPE_* definitions for buses to the same file that defines the struct, and added typecasting macros. I'm still a bit weary of putting this in this late, but I'll defer to others on this too. Anthony Liguori (6): qdev: fix adding of ptr properties qdev: use wrapper for qdev_get_path qdev: convert busses to QEMU Object Model qdev: connect busses with their parent devices qbus: make child devices links qbus: initialize in standard way Paolo Bonzini (15): qom: documentation addition qom: add object_class_get_parent qom: add class_base_init qom: make Object a type qom: assert that public types have a non-NULL parent field qdev: push type property up to Object qdev: fix -device foo,? qdev: use object_property_print in info qtree qdev: move bus properties to a separate global qdev: do not propagate properties to subclasses qdev: move bus properties to abstract superclasses pc: add back PCI.rombar compat property qdev: clean up global properties qdev: remove qdev_prop_set_defaults qdev: move sysbus initialization to sysbus.c exec.c|4 +- hw/acpi_piix4.c | 10 +- hw/i2c.c | 30 +++-- hw/ide/internal.h |3 + hw/ide/qdev.c | 31 +++-- hw/intel-hda.c| 37 ++--- hw/intel-hda.h|3 + hw/isa-bus.c | 23 +++- hw/isa.h |3 + hw/lsi53c895a.c |5 +- hw/pc_piix.c |9 +- hw/pci-hotplug.c |6 +- hw/pci.c | 49 --- hw/pci_bridge.c |2 +- hw/pci_internals.h|3 +- hw/qdev-monitor.c | 105 --- hw/qdev-properties.c | 62 +++-- hw/qdev.c | 298 +++-- hw/qdev.h | 56 hw/s390-virtio-bus.c | 37 ++--- hw/s390-virtio-bus.h |3 + hw/scsi-bus.c | 54 +--- hw/scsi.h |3 + hw/spapr_pci.c|7 +- hw/spapr_vio.c| 49 --- hw/spapr_vio.h|3 + hw/spapr_vty.c|6 +- hw/ssi.c | 29 ++-- hw/sysbus.c | 45 ++- hw/sysbus.h |3 + hw/usb.h |3 + hw/usb/bus.c | 45 --- hw/usb/desc.c |5 +- hw/usb/dev-smartcard-reader.c | 25 ++-- hw/virtio-scsi.c |6 +- hw/virtio-serial-bus.c| 35 +++-- include/qemu/object.h | 27 +++- qom/object.c | 105 +++ savevm.c | 12 +- 39 files changed, 783 insertions(+), 458 deletions(-) -- 1.7.9.3
[Qemu-devel] [PATCH 19/21] qdev: connect busses with their parent devices
From: Anthony Liguori aligu...@us.ibm.com
This makes sysbus part of the root hierarchy and all busses children of their
respective parent DeviceState.
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c |4
hw/sysbus.c |3 +++
2 files changed, 7 insertions(+)
diff --git a/hw/qdev.c b/hw/qdev.c
index b97ba00..decbcb1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -404,6 +404,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 != sysbus_get_default()) {
/* TODO: once all bus devices are qdevified,
only reset handler for main_system_bus should be registered here. */
@@ -651,6 +652,9 @@ static void device_initfn(Object *obj)
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
qdev_prop_set_globals(dev);
+
+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/hw/sysbus.c b/hw/sysbus.c
index f10a7d1..417fbd4 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -275,6 +275,9 @@ static void main_system_bus_create(void)
qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL,
main-system-bus);
main_system_bus-qdev_allocated = 1;
+object_property_add_child(container_get(qdev_get_machine(),
+/unattached),
+ sysbus, OBJECT(main_system_bus), NULL);
}
BusState *sysbus_get_default(void)
--
1.7.9.3
Re: [Qemu-devel] Heavy memory_region_get_dirty() -- Re: [PATCH 0/1 v2] KVM: Alleviate mmu_lock contention during dirty logging
On 05/02/2012 02:24 PM, Takuya Yoshikawa wrote:
During checking mmu_lock contention, I noticed that QEMU's
memory_region_get_dirty() was using unexpectedly much CPU time.
Thanks,
Takuya
=
perf top -t ${QEMU_TID}
=
51.52% qemu-system-x86_64 [.] memory_region_get_dirty
16.73% qemu-system-x86_64 [.] ram_save_remaining
memory_region_get_dirty() is called from ram_save_remaining(). Looks
like quadratic behaviour here: we send a few pages in
ram_save_remaining(), then walk the entire dirty bitmap to calculate
expected_time().
We should probably calculate expected_time once per iteration.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 05/02/2012 01:26 PM, Paolo Bonzini wrote: and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? If we make things a flag for the guessing code, it should work just as well with image files, right? Alex
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 05/02/2012 01:26 PM, Paolo Bonzini wrote: and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? If we make things a flag for the guessing code, it should work just as well with image files, right? Only when they're not blank. :) I was only thinking of #ifdef __s390__ for the call to HDIO_GETGEO. Paolo
Re: [Qemu-devel] [PATCH 12/21] pc: add back PCI.rombar compat property
On Wed, May 02, 2012 at 01:31:04PM +0200, Paolo Bonzini wrote:
This was erroneously dropped in d6c730086cbf24382eb8cff25551798769edfd84.
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Yes I think it's a mistake, however we had:
-},{
-.driver = PCI,
-.property = rombar,
-.value= stringify(0),
-},{
and not TYPE_PCI_DEVICE.
So let's put it back the way it was?
---
hw/pc_piix.c |4
1 file changed, 4 insertions(+)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index ef6afb1..15a62ef 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -523,6 +523,10 @@ static QEMUMachine pc_machine_v0_12 = {
.driver = virtio-blk-pci,\
.property = vectors,\
.value= stringify(0),\
+},{\
+.driver = TYPE_PCI_DEVICE,\
+.property = rombar,\
+.value= stringify(0),\
}
static QEMUMachine pc_machine_v0_11 = {
--
1.7.9.3
Re: [Qemu-devel] [PATCH 12/21] pc: add back PCI.rombar compat property
Il 02/05/2012 13:38, Michael S. Tsirkin ha scritto:
On Wed, May 02, 2012 at 01:31:04PM +0200, Paolo Bonzini wrote:
This was erroneously dropped in d6c730086cbf24382eb8cff25551798769edfd84.
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Yes I think it's a mistake, however we had:
-},{
-.driver = PCI,
-.property = rombar,
-.value= stringify(0),
-},{
and not TYPE_PCI_DEVICE.
PCI-TYPE_PCI_DEVICE is required by earlier patches in the series.
Paolo
Re: [Qemu-devel] [PATCH 12/21] pc: add back PCI.rombar compat property
On Wed, May 02, 2012 at 01:41:30PM +0200, Paolo Bonzini wrote:
Il 02/05/2012 13:38, Michael S. Tsirkin ha scritto:
On Wed, May 02, 2012 at 01:31:04PM +0200, Paolo Bonzini wrote:
This was erroneously dropped in d6c730086cbf24382eb8cff25551798769edfd84.
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Yes I think it's a mistake, however we had:
-},{
-.driver = PCI,
-.property = rombar,
-.value= stringify(0),
-},{
and not TYPE_PCI_DEVICE.
PCI-TYPE_PCI_DEVICE is required by earlier patches in the series.
Paolo
Haven't received them yet.
Ideally bugfixes come first in the series.
[Qemu-devel] [PATCH 09/21] qdev: move bus properties to a separate global
Simple code movement in order to simplify future refactoring.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c.c | 10 ++
hw/ide/qdev.c | 10 ++
hw/intel-hda.c| 10 ++
hw/pci.c | 22 --
hw/scsi-bus.c | 14 --
hw/spapr_vio.c| 10 ++
hw/usb/bus.c | 15 +--
hw/usb/dev-smartcard-reader.c | 10 ++
hw/virtio-serial-bus.c| 12 +++-
9 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/hw/i2c.c b/hw/i2c.c
index 23dfccb..cb10b1d 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -17,13 +17,15 @@ struct i2c_bus
uint8_t saved_address;
};
+static Property i2c_props[] = {
+DEFINE_PROP_UINT8(address, struct I2CSlave, address, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
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(),
-}
+.props = i2c_props,
};
static void i2c_bus_pre_save(void *opaque)
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a46578d..b67df3d 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -27,14 +27,16 @@
static char *idebus_get_fw_dev_path(DeviceState *dev);
+static Property ide_props[] = {
+DEFINE_PROP_UINT32(unit, IDEDevice, unit, -1),
+DEFINE_PROP_END_OF_LIST(),
+};
+
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(),
-},
+.props = ide_props,
};
void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..0994f6b 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -29,13 +29,15 @@
/* - */
/* hda bus */
+static Property hda_props[] = {
+DEFINE_PROP_UINT32(cad, HDACodecDevice, cad, -1),
+DEFINE_PROP_END_OF_LIST()
+};
+
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()
-}
+.props = hda_props,
};
void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
diff --git a/hw/pci.c b/hw/pci.c
index b706e69..403651f 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -44,6 +44,17 @@ static char *pcibus_get_dev_path(DeviceState *dev);
static char *pcibus_get_fw_dev_path(DeviceState *dev);
static int pcibus_reset(BusState *qbus);
+static Property pci_props[] = {
+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()
+};
+
struct BusInfo pci_bus_info = {
.name = PCI,
.size = sizeof(PCIBus),
@@ -51,16 +62,7 @@ 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()
-}
+.props = pci_props,
};
static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index dbdb99c..7f9eb53 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -12,17 +12,19 @@ static char *scsibus_get_fw_dev_path(DeviceState *dev);
static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
static void scsi_req_dequeue(SCSIRequest *req);
+static Property scsi_props[] = {
+DEFINE_PROP_UINT32(channel, SCSIDevice, channel, 0),
+DEFINE_PROP_UINT32(scsi-id, SCSIDevice, id, -1),
+DEFINE_PROP_UINT32(lun, SCSIDevice, lun, -1),
+DEFINE_PROP_END_OF_LIST(),
+};
+
static struct BusInfo scsi_bus_info = {
.name = SCSI,
.size = sizeof(SCSIBus),
.get_dev_path = scsibus_get_dev_path,
.get_fw_dev_path =
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
As hinted in my other mail, I think the way to go would be to give a hint to the geometry code that we're running on a DASD disk.. Just as an idea if we are going that path, we might use the BIODASDINFO2 or DASDAPIVER ioctls in qemu to detect if that is a dasd. Christian
[Qemu-devel] [PATCH 14/21] qdev: remove qdev_prop_set_defaults
Instead, qdev_property_add_static can set the default.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev-properties.c | 22 --
hw/qdev.c| 26 +++---
hw/qdev.h|1 -
3 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 572b83c..244646c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1130,28 +1130,6 @@ void qdev_prop_set_ptr(DeviceState *dev, const char
*name, void *value)
*ptr = value;
}
-void qdev_prop_set_defaults(DeviceState *dev, Property *props)
-{
-Object *obj = OBJECT(dev);
-if (!props)
-return;
-for (; props-name; props++) {
-Error *errp = NULL;
-if (props-qtype == QTYPE_NONE) {
-continue;
-}
-if (props-qtype == QTYPE_QBOOL) {
-object_property_set_bool(obj, props-defval, props-name, errp);
-} else if (props-info-enum_table) {
-object_property_set_str(obj,
props-info-enum_table[props-defval],
-props-name, errp);
-} else if (props-qtype == QTYPE_QINT) {
-object_property_set_int(obj, props-defval, props-name, errp);
-}
-assert_no_error(errp);
-}
-}
-
static QTAILQ_HEAD(, GlobalProperty) global_props =
QTAILQ_HEAD_INITIALIZER(global_props);
static void qdev_prop_register_global(GlobalProperty *prop)
diff --git a/hw/qdev.c b/hw/qdev.c
index 7288b8e..f953c51 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -576,6 +576,9 @@ void qdev_property_add_legacy(DeviceState *dev, Property
*prop,
void qdev_property_add_static(DeviceState *dev, Property *prop,
Error **errp)
{
+Error *local_err = NULL;
+Object *obj = OBJECT(dev);
+
/*
* TODO qdev_prop_ptr does not have getters or setters. It must
* go now that it can be replaced with links. The test should be
@@ -585,10 +588,28 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop,
return;
}
-object_property_add(OBJECT(dev), prop-name, prop-info-name,
+object_property_add(obj, prop-name, prop-info-name,
prop-info-get, prop-info-set,
prop-info-release,
-prop, errp);
+prop, local_err);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+if (prop-qtype == QTYPE_NONE) {
+return;
+}
+
+if (prop-qtype == QTYPE_QBOOL) {
+object_property_set_bool(obj, prop-defval, prop-name, local_err);
+} else if (prop-info-enum_table) {
+object_property_set_str(obj, prop-info-enum_table[prop-defval],
+prop-name, local_err);
+} else if (prop-qtype == QTYPE_QINT) {
+object_property_set_int(obj, prop-defval, prop-name, local_err);
+}
+assert_no_error(local_err);
}
static void device_initfn(Object *obj)
@@ -611,7 +632,6 @@ static void device_initfn(Object *obj)
qdev_property_add_legacy(dev, prop, NULL);
qdev_property_add_static(dev, prop, NULL);
}
-qdev_prop_set_defaults(dev, DEVICE_CLASS(class)-props);
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
qdev_prop_set_globals(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index 4d6458f..23147df 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -311,7 +311,6 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name, uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
/* FIXME: Remove opaque pointer properties. */
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
-void qdev_prop_set_defaults(DeviceState *dev, Property *props);
void qdev_prop_register_global_list(GlobalProperty *props);
void qdev_prop_set_globals(DeviceState *dev);
--
1.7.9.3
Re: [Qemu-devel] [PATCH 01/21] qom: documentation addition
Am 02.05.2012 13:30, schrieb Paolo Bonzini: Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- include/qemu/object.h |6 ++ 1 file changed, 6 insertions(+) diff --git a/include/qemu/object.h b/include/qemu/object.h index ca1649c..d315dfa 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -555,6 +555,12 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass, */ const char *object_class_get_name(ObjectClass *klass); +/** + * object_class_by_name: + * @typename: The QOM typename to obtain the class for. + * + * Returns: The class for @typename. ...or NULL if not found. + */ ObjectClass *object_class_by_name(const char *typename); void object_class_foreach(void (*fn)(ObjectClass *klass, void *opaque), This is certainly a good fit for 1.1. :) 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 11/21] qdev: move bus properties to abstract superclasses
In qdev, each bus in practice identified an abstract superclass, but
this was mostly hidden. In QOM, instead, these abstract classes are
explicit so we can move bus properties there.
All bus property walks are removed, and all device property walks
are changed to look along the class hierarchy instead.
This breaks global bus properties, an obscure feature when used
with the command-line which is actually useful and used when used by
backwards-compatible machine types. So this patch also adjust the
global bus properties in hw/pc_piix.c to refer to the abstract class.
Globals and other properties must be modified in the same patch to
avoid complications related to initialization ordering.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c.c |2 +-
hw/ide/qdev.c |2 +-
hw/intel-hda.c|2 +-
hw/pc_piix.c |5 +++--
hw/pci.c |2 +-
hw/qdev-monitor.c | 41 ++---
hw/qdev-properties.c | 40 ++--
hw/qdev.c | 36 ++--
hw/qdev.h |5 -
hw/scsi-bus.c |2 +-
hw/spapr_vio.c|2 +-
hw/usb/bus.c |2 +-
hw/usb/dev-smartcard-reader.c |2 +-
hw/virtio-serial-bus.c|2 +-
14 files changed, 62 insertions(+), 83 deletions(-)
diff --git a/hw/i2c.c b/hw/i2c.c
index cb10b1d..af5979e 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -25,7 +25,6 @@ static Property i2c_props[] = {
static struct BusInfo i2c_bus_info = {
.name = I2C,
.size = sizeof(i2c_bus),
-.props = i2c_props,
};
static void i2c_bus_pre_save(void *opaque)
@@ -221,6 +220,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-props = i2c_props;
}
static TypeInfo i2c_slave_type_info = {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b67df3d..a91e878 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -36,7 +36,6 @@ static struct BusInfo ide_bus_info = {
.name = IDE,
.size = sizeof(IDEBus),
.get_fw_dev_path = idebus_get_fw_dev_path,
-.props = ide_props,
};
void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -251,6 +250,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-props = ide_props;
}
static TypeInfo ide_device_type_info = {
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 0994f6b..e2bd41e 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -37,7 +37,6 @@ static Property hda_props[] = {
static struct BusInfo hda_codec_bus_info = {
.name = HDA,
.size = sizeof(HDACodecBus),
-.props = hda_props,
};
void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
@@ -1278,6 +1277,7 @@ static void hda_codec_device_class_init(ObjectClass
*klass, void *data)
k-init = hda_codec_dev_init;
k-exit = hda_codec_dev_exit;
k-bus_info = hda_codec_bus_info;
+k-props = hda_props;
}
static TypeInfo hda_codec_device_type_info = {
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..ef6afb1 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -29,6 +29,7 @@
#include apic.h
#include pci.h
#include pci_ids.h
+#include usb.h
#include net.h
#include boards.h
#include ide.h
@@ -378,7 +379,7 @@ static QEMUMachine pc_machine_v1_1 = {
.property = vapic,\
.value= off,\
},{\
-.driver = USB,\
+.driver = TYPE_USB_DEVICE,\
.property = full-path,\
.value= no,\
}
@@ -451,7 +452,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/pci.c b/hw/pci.c
index 403651f..6319f4d 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -62,7 +62,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 = pci_props,
};
static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -2004,6 +2003,7 @@ static void pci_device_class_init(ObjectClass *klass,
void *data)
k-unplug = pci_unplug_device;
k-exit = pci_unregister_device;
k-bus_info = pci_bus_info;
+k-props = pci_props;
}
static TypeInfo pci_device_type_info = {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index eed781d..d9c6adc 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -123,7 +123,6 @@ int qdev_device_help(QemuOpts *opts)
[Qemu-devel] [PATCH 15/21] qdev: fix adding of ptr properties
From: Anthony Liguori aligu...@us.ibm.com
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 aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index f953c51..7f18590 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -550,9 +550,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.9.3
[Qemu-devel] [PATCH 16/21] qdev: use wrapper for qdev_get_path
From: Anthony Liguori aligu...@us.ibm.com
This makes it easier to remove it from BusInfo.
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
Anthony's patch used BUS_CLASS so it broke bisectability; fixed here.
exec.c|4 ++--
hw/qdev.c | 16
hw/qdev.h |2 ++
hw/scsi-bus.c |4 +---
hw/usb/bus.c |5 ++---
hw/usb/desc.c |5 +++--
savevm.c | 12 ++--
7 files changed, 32 insertions(+), 16 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 7f18590..7b2802d 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -494,6 +494,22 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
+char *qdev_get_dev_path(DeviceState *dev)
+{
+BusInfo *businfo;
+
+if (!dev || !dev-parent_bus) {
+return NULL;
+}
+
+businfo = dev-parent_bus-info;
+if (businfo-get_dev_path) {
+return businfo-get_dev_path(dev);
+}
+
+return NULL;
+}
+
/**
* Legacy property handling
*/
diff --git a/hw/qdev.h b/hw/qdev.h
index 23147df..2e82a2f 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -354,4 +354,6 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
extern int qdev_hotplug;
+char *qdev_get_dev_path(DeviceState *dev);
+
#endif
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index d7054da..0dba3aa 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1434,9 +1434,7 @@ static char *scsibus_get_dev_path(DeviceState *dev)
char *id = NULL;
char *path;
-if (hba hba-parent_bus hba-parent_bus-info-get_dev_path) {
-id = hba-parent_bus-info-get_dev_path(hba);
-}
+id = qdev_get_dev_path(hba);
if (id) {
path = g_strdup_printf(%s/%d:%d:%d, id, d-channel, d-id, d-lun);
} else {
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 64887d5..8b08f93 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -467,9 +467,8 @@ static char *usb_get_dev_path(DeviceState *qdev)
DeviceState *hcd = qdev-parent_bus-parent;
char *id = NULL;
-if ((dev-flags (1 USB_DEV_FLAG_FULL_PATH))
-hcd hcd-parent_bus hcd-parent_bus-info-get_dev_path) {
-id = hcd-parent_bus-info-get_dev_path(hcd);
+if (dev-flags (1 USB_DEV_FLAG_FULL_PATH)) {
+id = qdev_get_dev_path(hcd);
}
if (id) {
char *ret = g_strdup_printf(%s/%s, id, dev-port-path);
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index e8a3c6a..0a9d3c9 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -432,12 +432,13 @@ void usb_desc_create_serial(USBDevice *dev)
const USBDesc *desc = usb_device_get_usb_desc(dev);
int index = desc-id.iSerialNumber;
char serial[64];
+char *path;
int dst;
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);
+path = qdev_get_dev_path(hcd);
+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
Re: [Qemu-devel] [PATCH 02/21] qom: add object_class_get_parent
Am 02.05.2012 13:30, schrieb Paolo Bonzini: This simple bit of functionality was missing and we'll need it soon, so add it. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- include/qemu/object.h |8 qom/object.c | 13 + 2 files changed, 21 insertions(+) diff --git a/include/qemu/object.h b/include/qemu/object.h index d315dfa..063c817 100644 --- a/include/qemu/object.h +++ b/include/qemu/object.h @@ -548,6 +548,14 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass, const char *typename); /** + * object_class_get_parent: + * @klass: The class to obtain the parent for. + * + * Returns: The parent for @klass. Here too we should indicate that this may return NULL (rather than aborting). Other than that looks good. Andreas + */ +ObjectClass *object_class_get_parent(ObjectClass *klass); + +/** * object_class_get_name: * @klass: The class to obtain the QOM typename for. * [snip] -- 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/21] qdev: move bus properties to abstract superclasses
On 05/02/2012 06:31 AM, Paolo Bonzini wrote:
In qdev, each bus in practice identified an abstract superclass, but
this was mostly hidden. In QOM, instead, these abstract classes are
explicit so we can move bus properties there.
All bus property walks are removed, and all device property walks
are changed to look along the class hierarchy instead.
This breaks global bus properties, an obscure feature when used
with the command-line which is actually useful and used when used by
backwards-compatible machine types. So this patch also adjust the
global bus properties in hw/pc_piix.c to refer to the abstract class.
Globals and other properties must be modified in the same patch to
avoid complications related to initialization ordering.
Signed-off-by: Paolo Bonzinipbonz...@redhat.com
---
hw/i2c.c |2 +-
hw/ide/qdev.c |2 +-
hw/intel-hda.c|2 +-
hw/pc_piix.c |5 +++--
hw/pci.c |2 +-
hw/qdev-monitor.c | 41 ++---
hw/qdev-properties.c | 40 ++--
hw/qdev.c | 36 ++--
hw/qdev.h |5 -
hw/scsi-bus.c |2 +-
hw/spapr_vio.c|2 +-
hw/usb/bus.c |2 +-
hw/usb/dev-smartcard-reader.c |2 +-
hw/virtio-serial-bus.c|2 +-
14 files changed, 62 insertions(+), 83 deletions(-)
diff --git a/hw/i2c.c b/hw/i2c.c
index cb10b1d..af5979e 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -25,7 +25,6 @@ static Property i2c_props[] = {
static struct BusInfo i2c_bus_info = {
.name = I2C,
.size = sizeof(i2c_bus),
-.props = i2c_props,
};
static void i2c_bus_pre_save(void *opaque)
@@ -221,6 +220,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-props = i2c_props;
}
static TypeInfo i2c_slave_type_info = {
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index b67df3d..a91e878 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -36,7 +36,6 @@ static struct BusInfo ide_bus_info = {
.name = IDE,
.size = sizeof(IDEBus),
.get_fw_dev_path = idebus_get_fw_dev_path,
-.props = ide_props,
};
void ide_bus_new(IDEBus *idebus, DeviceState *dev, int bus_id)
@@ -251,6 +250,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-props = ide_props;
}
static TypeInfo ide_device_type_info = {
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index 0994f6b..e2bd41e 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -37,7 +37,6 @@ static Property hda_props[] = {
static struct BusInfo hda_codec_bus_info = {
.name = HDA,
.size = sizeof(HDACodecBus),
-.props = hda_props,
};
void hda_codec_bus_init(DeviceState *dev, HDACodecBus *bus,
@@ -1278,6 +1277,7 @@ static void hda_codec_device_class_init(ObjectClass
*klass, void *data)
k-init = hda_codec_dev_init;
k-exit = hda_codec_dev_exit;
k-bus_info =hda_codec_bus_info;
+k-props = hda_props;
}
static TypeInfo hda_codec_device_type_info = {
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..ef6afb1 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -29,6 +29,7 @@
#include apic.h
#include pci.h
#include pci_ids.h
+#include usb.h
#include net.h
#include boards.h
#include ide.h
@@ -378,7 +379,7 @@ static QEMUMachine pc_machine_v1_1 = {
.property = vapic,\
.value= off,\
},{\
-.driver = USB,\
+.driver = TYPE_USB_DEVICE,\
.property = full-path,\
.value= no,\
}
@@ -451,7 +452,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/pci.c b/hw/pci.c
index 403651f..6319f4d 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -62,7 +62,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 = pci_props,
};
static PCIBus *pci_find_bus_nr(PCIBus *bus, int bus_num);
@@ -2004,6 +2003,7 @@ static void pci_device_class_init(ObjectClass *klass,
void *data)
k-unplug = pci_unplug_device;
k-exit = pci_unregister_device;
k-bus_info =pci_bus_info;
+k-props = pci_props;
}
static TypeInfo pci_device_type_info = {
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index eed781d..d9c6adc 100644
---
Re: [Qemu-devel] [PATCH 14/21] qdev: remove qdev_prop_set_defaults
On 05/02/2012 06:31 AM, Paolo Bonzini wrote:
Instead, qdev_property_add_static can set the default.
Signed-off-by: Paolo Bonzinipbonz...@redhat.com
Great idea!
Reviewed-by: Anthony Liguori aligu...@us.ibm.com
Regards,
Anthony Liguori
---
hw/qdev-properties.c | 22 --
hw/qdev.c| 26 +++---
hw/qdev.h|1 -
3 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 572b83c..244646c 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -1130,28 +1130,6 @@ void qdev_prop_set_ptr(DeviceState *dev, const char
*name, void *value)
*ptr = value;
}
-void qdev_prop_set_defaults(DeviceState *dev, Property *props)
-{
-Object *obj = OBJECT(dev);
-if (!props)
-return;
-for (; props-name; props++) {
-Error *errp = NULL;
-if (props-qtype == QTYPE_NONE) {
-continue;
-}
-if (props-qtype == QTYPE_QBOOL) {
-object_property_set_bool(obj, props-defval, props-name,errp);
-} else if (props-info-enum_table) {
-object_property_set_str(obj,
props-info-enum_table[props-defval],
-props-name,errp);
-} else if (props-qtype == QTYPE_QINT) {
-object_property_set_int(obj, props-defval, props-name,errp);
-}
-assert_no_error(errp);
-}
-}
-
static QTAILQ_HEAD(, GlobalProperty) global_props =
QTAILQ_HEAD_INITIALIZER(global_props);
static void qdev_prop_register_global(GlobalProperty *prop)
diff --git a/hw/qdev.c b/hw/qdev.c
index 7288b8e..f953c51 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -576,6 +576,9 @@ void qdev_property_add_legacy(DeviceState *dev, Property
*prop,
void qdev_property_add_static(DeviceState *dev, Property *prop,
Error **errp)
{
+Error *local_err = NULL;
+Object *obj = OBJECT(dev);
+
/*
* TODO qdev_prop_ptr does not have getters or setters. It must
* go now that it can be replaced with links. The test should be
@@ -585,10 +588,28 @@ void qdev_property_add_static(DeviceState *dev, Property
*prop,
return;
}
-object_property_add(OBJECT(dev), prop-name, prop-info-name,
+object_property_add(obj, prop-name, prop-info-name,
prop-info-get, prop-info-set,
prop-info-release,
-prop, errp);
+prop,local_err);
+
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+if (prop-qtype == QTYPE_NONE) {
+return;
+}
+
+if (prop-qtype == QTYPE_QBOOL) {
+object_property_set_bool(obj, prop-defval, prop-name,local_err);
+} else if (prop-info-enum_table) {
+object_property_set_str(obj, prop-info-enum_table[prop-defval],
+prop-name,local_err);
+} else if (prop-qtype == QTYPE_QINT) {
+object_property_set_int(obj, prop-defval, prop-name,local_err);
+}
+assert_no_error(local_err);
}
static void device_initfn(Object *obj)
@@ -611,7 +632,6 @@ static void device_initfn(Object *obj)
qdev_property_add_legacy(dev, prop, NULL);
qdev_property_add_static(dev, prop, NULL);
}
-qdev_prop_set_defaults(dev, DEVICE_CLASS(class)-props);
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
qdev_prop_set_globals(dev);
diff --git a/hw/qdev.h b/hw/qdev.h
index 4d6458f..23147df 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -311,7 +311,6 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name, uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
/* FIXME: Remove opaque pointer properties. */
void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value);
-void qdev_prop_set_defaults(DeviceState *dev, Property *props);
void qdev_prop_register_global_list(GlobalProperty *props);
void qdev_prop_set_globals(DeviceState *dev);
[Qemu-devel] [PATCH 02/21] qom: add object_class_get_parent
This simple bit of functionality was missing and we'll need it soon,
so add it.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/qemu/object.h |8
qom/object.c | 13 +
2 files changed, 21 insertions(+)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index d315dfa..063c817 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -548,6 +548,14 @@ ObjectClass *object_class_dynamic_cast(ObjectClass *klass,
const char *typename);
/**
+ * object_class_get_parent:
+ * @klass: The class to obtain the parent for.
+ *
+ * Returns: The parent for @klass.
+ */
+ObjectClass *object_class_get_parent(ObjectClass *klass);
+
+/**
* object_class_get_name:
* @klass: The class to obtain the QOM typename for.
*
diff --git a/qom/object.c b/qom/object.c
index e721fc2..3e9fed7 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -545,6 +545,19 @@ ObjectClass *object_class_by_name(const char *typename)
return type-class;
}
+ObjectClass *object_class_get_parent(ObjectClass *class)
+{
+TypeImpl *type = type_get_parent(class-type);
+
+if (!type) {
+return NULL;
+}
+
+type_initialize(type);
+
+return type-class;
+}
+
typedef struct OCFData
{
void (*fn)(ObjectClass *klass, void *opaque);
--
1.7.9.3
[Qemu-devel] [PATCH 10/21] qdev: do not propagate properties to subclasses
As soon as we'll look up properties along the inheritance chain, we
will have duplicates if class A defines some properties and its
subclass B does not define any, because class_b-props will be
left equal to class_a-props.
The solution here is to reintroduce the class_base_init TypeInfo
callback, that was present in one of the early QOM versions but
removed (on my request...) before committing.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/hw/qdev.c b/hw/qdev.c
index 94fb32e..67d7770 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -658,6 +658,16 @@ static void device_finalize(Object *obj)
QTAILQ_REMOVE(dev-parent_bus-children, dev, sibling);
}
+static void device_class_base_init(ObjectClass *class, void *data)
+{
+DeviceClass *klass = DEVICE_CLASS(class);
+
+/* We explicitly look up properties in the superclasses,
+ * so do not propagate them to the subclasses.
+ */
+klass-props = NULL;
+}
+
void device_reset(DeviceState *dev)
{
DeviceClass *klass = DEVICE_GET_CLASS(dev);
@@ -684,6 +694,7 @@ static TypeInfo device_type_info = {
.instance_size = sizeof(DeviceState),
.instance_init = device_initfn,
.instance_finalize = device_finalize,
+.class_base_init = device_class_base_init,
.abstract = true,
.class_size = sizeof(DeviceClass),
};
--
1.7.9.3
Re: [Qemu-devel] [PATCH 05/21] qom: assert that public types have a non-NULL parent field
Am 02.05.2012 13:30, schrieb Paolo Bonzini:
This protects against unwanted effects of changing TYPE_OBJECT from
NULL to a string. Suggested by Andreas Faerber.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/qemu/object.h |1 -
qom/object.c | 14 ++
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 9c49d99..ec2b382 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -527,7 +527,6 @@ const char *object_get_typename(Object *obj);
*/
Type type_register_static(const TypeInfo *info);
-#define type_register_static_alias(info, name) do { } while (0)
Unrelated removal of unused code?
Other than that
Reviewed-by: Andreas Färber afaer...@suse.de
Thanks,
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 05/14] qdev: use wrapper for qdev_get_path
On 05/01/12 20:36, Anthony Liguori wrote:
Hi Gerd,
Could you carefully review the USB changes here? I'm not really sure
what our contract is with the guest in terms of ABI compatibility. I
think it's good but it could use a second set of eyes.
incremental fix attached.
cheers,
Gerd
From 8823f1f5491639119d267289610fcad6dd1a0872 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Wed, 2 May 2012 14:30:54 +0200
Subject: [PATCH] fixup
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/usb/desc.c |6 ++
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/usb/desc.c b/hw/usb/desc.c
index 64352c9..84ea9f2 100644
--- a/hw/usb/desc.c
+++ b/hw/usb/desc.c
@@ -433,13 +433,11 @@ void usb_desc_create_serial(USBDevice *dev)
int index = desc-id.iSerialNumber;
char serial[64];
int dst;
-char *path = NULL;
+char *path;
assert(index != 0 desc-str[index] != NULL);
dst = snprintf(serial, sizeof(serial), %s, desc-str[index]);
-if (hcd-parent_bus hcd-parent_bus-parent) {
-path = qdev_get_dev_path(hcd-parent_bus-parent);
-}
+path = qdev_get_dev_path(hcd);
if (path) {
dst += snprintf(serial+dst, sizeof(serial)-dst, -%s, path);
}
--
1.7.1
[Qemu-devel] [PATCH 04/21] qom: make Object a type
Right now the base Object class has a special NULL type. Change this so
that we will be able to add class_init and class_base_init callbacks.
To do this, remove some special casing of ObjectClass that is not really
necessary.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/qemu/object.h |2 +-
qom/object.c | 59 +
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 597a2f6..9c49d99 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -33,7 +33,7 @@ typedef struct TypeInfo TypeInfo;
typedef struct InterfaceClass InterfaceClass;
typedef struct InterfaceInfo InterfaceInfo;
-#define TYPE_OBJECT NULL
+#define TYPE_OBJECT object
/**
* SECTION:object.h
diff --git a/qom/object.c b/qom/object.c
index e66d3ab..88ec417 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -210,7 +210,7 @@ static void type_class_interface_init(TypeImpl *ti,
InterfaceImpl *iface)
static void type_initialize(TypeImpl *ti)
{
-size_t class_size = sizeof(ObjectClass);
+TypeImpl *parent;
int i;
if (ti-class) {
@@ -221,30 +221,24 @@ static void type_initialize(TypeImpl *ti)
ti-instance_size = type_object_get_size(ti);
ti-class = g_malloc0(ti-class_size);
-ti-class-type = ti;
-
-if (type_has_parent(ti)) {
-TypeImpl *parent = type_get_parent(ti);
+parent = type_get_parent(ti);
+if (parent) {
type_initialize(parent);
-class_size = parent-class_size;
g_assert(parent-class_size = ti-class_size);
+memcpy(ti-class, parent-class, parent-class_size);
+}
-memcpy((void *)ti-class + sizeof(ObjectClass),
- (void *)parent-class + sizeof(ObjectClass),
- parent-class_size - sizeof(ObjectClass));
+ti-class-type = ti;
-while (parent) {
-if (parent-class_base_init) {
-parent-class_base_init(ti-class, ti-class_data);
-}
-parent = type_get_parent(parent);
+while (parent) {
+if (parent-class_base_init) {
+parent-class_base_init(ti-class, ti-class_data);
}
+parent = type_get_parent(parent);
}
-memset((void *)ti-class + class_size, 0, ti-class_size - class_size);
-
for (i = 0; i ti-num_interfaces; i++) {
type_class_interface_init(ti, ti-interfaces[i]);
}
@@ -467,19 +461,6 @@ Object *object_dynamic_cast(Object *obj, const char
*typename)
}
-static void register_types(void)
-{
-static TypeInfo interface_info = {
-.name = TYPE_INTERFACE,
-.instance_size = sizeof(Interface),
-.abstract = true,
-};
-
-type_interface = type_register_static(interface_info);
-}
-
-type_init(register_types)
-
Object *object_dynamic_cast_assert(Object *obj, const char *typename)
{
Object *inst;
@@ -1216,3 +1197,23 @@ void object_property_add_str(Object *obj, const char
*name,
property_release_str,
prop, errp);
}
+
+static void register_types(void)
+{
+static TypeInfo interface_info = {
+.name = TYPE_INTERFACE,
+.instance_size = sizeof(Interface),
+.abstract = true,
+};
+
+static TypeInfo object_info = {
+.name = TYPE_OBJECT,
+.instance_size = sizeof(Object),
+.abstract = true,
+};
+
+type_interface = type_register_static(interface_info);
+type_register_static(object_info);
+}
+
+type_init(register_types)
--
1.7.9.3
[Qemu-devel] [PATCH 07/21] qdev: fix -device foo,?
Since most property types do not have a parse property now, this was
broken. Fix it by looking at the setter instead.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev-monitor.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index dc4e4e1..41b9e2c 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -158,7 +158,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,
@@ -166,7 +166,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,
--
1.7.9.3
[Qemu-devel] [PATCH 21/21] qbus: initialize in standard way
From: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c | 84 +
1 file changed, 51 insertions(+), 33 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 6965da0..9aaa1d3 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -410,40 +410,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 != sysbus_get_default()) {
/* TODO: once all bus devices are qdevified,
only reset handler for main_system_bus should be registered here. */
@@ -455,7 +450,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)
@@ -464,28 +462,20 @@ 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;
}
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 != sysbus_get_default()); /* 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));
}
}
@@ -765,12 +755,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 != sysbus_get_default()); /* main_system_bus is never freed
*/
+qemu_unregister_reset(qbus_reset_all_fn, bus);
+}
+g_free((char *)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.9.3
[Qemu-devel] [PATCH 20/21] qbus: make child devices links
From: Anthony Liguori aligu...@us.ibm.com
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 aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
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| 64 +-
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, 161 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 cd8fa31..47219f5 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -86,11 +86,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 63bdb27..fb585b5 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -78,10 +78,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;
@@ -483,10 +484,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);
@@ -1105,15 +1107,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) {
+
[Qemu-devel] [PATCH 18/21] qdev: convert busses to QEMU Object Model
From: Anthony Liguori aligu...@us.ibm.com
This is far less interesting than it sounds. We simply add an Object to each
BusState 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 aligu...@us.ibm.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i2c.c | 15 ++
hw/ide/internal.h |3 ++
hw/ide/qdev.c | 21 ++
hw/intel-hda.c| 12
hw/intel-hda.h|3 ++
hw/isa-bus.c | 23 ++-
hw/isa.h |3 ++
hw/pci-hotplug.c |6 +---
hw/pci.c | 27 --
hw/pci_bridge.c |2 +-
hw/pci_internals.h|3 +-
hw/qdev-monitor.c | 33 ++
hw/qdev.c | 62 ++---
hw/qdev.h | 37
hw/s390-virtio-bus.c | 12
hw/s390-virtio-bus.h |3 ++
hw/scsi-bus.c | 23 ++-
hw/scsi.h |3 ++
hw/spapr_vio.c| 12
hw/spapr_vio.h|3 ++
hw/ssi.c | 15 ++
hw/sysbus.c | 27 --
hw/sysbus.h |3 ++
hw/usb.h |3 ++
hw/usb/bus.c | 25 +++--
hw/usb/dev-smartcard-reader.c | 15 ++
hw/virtio-serial-bus.c| 23 +++
27 files changed, 280 insertions(+), 137 deletions(-)
diff --git a/hw/i2c.c b/hw/i2c.c
index af5979e..cd8fa31 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -22,9 +22,13 @@ static Property i2c_props[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static struct BusInfo i2c_bus_info = {
-.name = I2C,
-.size = sizeof(i2c_bus),
+#define TYPE_I2C_BUS i2c-bus
+#define I2C_BUS(obj) OBJECT_CHECK(i2c_bus, (obj), TYPE_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)
@@ -62,7 +66,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;
}
@@ -219,7 +223,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;
k-props = i2c_props;
}
@@ -234,6 +238,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/internal.h b/hw/ide/internal.h
index f8a027d..1a02f57 100644
--- a/hw/ide/internal.h
+++ b/hw/ide/internal.h
@@ -25,6 +25,9 @@ typedef struct IDEState IDEState;
typedef struct IDEDMA IDEDMA;
typedef struct IDEDMAOps IDEDMAOps;
+#define TYPE_IDE_BUS IDE
+#define IDE_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
+
/* Bits of HD_STATUS */
#define ERR_STAT 0x01
#define INDEX_STAT 0x02
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a91e878..24c3101 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -32,15 +32,23 @@ static Property ide_props[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static struct BusInfo ide_bus_info = {
-.name = IDE,
-.size = sizeof(IDEBus),
-.get_fw_dev_path = idebus_get_fw_dev_path,
+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;
}
@@ -249,7 +257,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;
k-props = ide_props;
}
@@ -264,6 +272,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);
[Qemu-devel] [PATCH 12/21] pc: add back PCI.rombar compat property
This was erroneously dropped in d6c730086cbf24382eb8cff25551798769edfd84.
Cc: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/pc_piix.c |4
1 file changed, 4 insertions(+)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index ef6afb1..15a62ef 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -523,6 +523,10 @@ static QEMUMachine pc_machine_v0_12 = {
.driver = virtio-blk-pci,\
.property = vectors,\
.value= stringify(0),\
+},{\
+.driver = TYPE_PCI_DEVICE,\
+.property = rombar,\
+.value= stringify(0),\
}
static QEMUMachine pc_machine_v0_11 = {
--
1.7.9.3
[Qemu-devel] [PATCH 08/21] qdev: use object_property_print in info qtree
Otherwise, non-string properties without a legacy counterpart are missed.
Also fix error propagation in object_property_print itself.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev-monitor.c |2 +-
qom/object.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index 41b9e2c..eed781d 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -493,7 +493,7 @@ static void qdev_print_props(Monitor *mon, DeviceState
*dev, Property *props,
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);
+value = object_property_print(OBJECT(dev), props-name, err);
}
g_free(legacy_name);
diff --git a/qom/object.c b/qom/object.c
index 464fc8f..b4f6c1d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -839,7 +839,7 @@ char *object_property_print(Object *obj, const char *name,
char *string;
mo = string_output_visitor_new();
-object_property_get(obj, string_output_get_visitor(mo), name, NULL);
+object_property_get(obj, string_output_get_visitor(mo), name, errp);
string = string_output_get_string(mo);
string_output_visitor_cleanup(mo);
return string;
--
1.7.9.3
[Qemu-devel] [PATCH 13/21] qdev: clean up global properties
Now that global properties do not depend on buses anymore, set
them directly in the device instance_init function.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev-monitor.c |1 -
hw/qdev.c |2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/qdev-monitor.c b/hw/qdev-monitor.c
index d9c6adc..6f2d83e 100644
--- a/hw/qdev-monitor.c
+++ b/hw/qdev-monitor.c
@@ -440,7 +440,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* create device, set properties */
qdev = DEVICE(object_new(driver));
qdev_set_parent_bus(qdev, bus);
-qdev_prop_set_globals(qdev);
id = qemu_opts_id(opts);
if (id) {
diff --git a/hw/qdev.c b/hw/qdev.c
index 98efc8b..7288b8e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -111,7 +111,6 @@ DeviceState *qdev_try_create(BusState *bus, const char
*type)
}
qdev_set_parent_bus(dev, bus);
-qdev_prop_set_globals(dev);
return dev;
}
@@ -615,6 +614,7 @@ static void device_initfn(Object *obj)
qdev_prop_set_defaults(dev, DEVICE_CLASS(class)-props);
class = object_class_get_parent(class);
} while (class != object_class_by_name(TYPE_DEVICE));
+qdev_prop_set_globals(dev);
}
/* Unlink device from bus and free the structure. */
--
1.7.9.3
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 05/02/2012 01:38 PM, Paolo Bonzini wrote: On 05/02/2012 01:26 PM, Paolo Bonzini wrote: and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? If we make things a flag for the guessing code, it should work just as well with image files, right? Only when they're not blank. :) I was only thinking of #ifdef __s390__ for the call to HDIO_GETGEO. Well, if guessing is a function guess_size(disk_size, block_size) then we would be able to do the same on an image file. Christian, would that work? Alex
[Qemu-devel] [PATCH 03/21] qom: add class_base_init
The class_base_init TypeInfo callback was present in one of the early
QOM versions but removed (on my request...) before committing. We
will need it soon, add it.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/qemu/object.h | 10 --
qom/object.c |9 +
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 063c817..597a2f6 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -291,10 +291,15 @@ struct Object
* has occurred to allow a class to set its default virtual method pointers.
* This is also the function to use to override virtual methods from a parent
* class.
+ * @class_base_init: This function is called for all base classes after all
+ * parent class initialization has occurred, but before the class itself
+ * is initialized. This is the function to use to undo the effects of
+ * memcpy from the parent class to the descendents.
* @class_finalize: This function is called during class destruction and is
* meant to release and dynamic parameters allocated by @class_init.
- * @class_data: Data to pass to the @class_init and @class_finalize functions.
- * This can be useful when building dynamic classes.
+ * @class_data: Data to pass to the @class_init, @class_base_init and
+ * @class_finalize functions. This can be useful when building dynamic
+ * classes.
* @interfaces: The list of interfaces associated with this type. This
* should point to a static array that's terminated with a zero filled
* element.
@@ -312,6 +317,7 @@ struct TypeInfo
size_t class_size;
void (*class_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, void *data);
void (*class_finalize)(ObjectClass *klass, void *data);
void *class_data;
diff --git a/qom/object.c b/qom/object.c
index 3e9fed7..e66d3ab 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -45,6 +45,7 @@ struct TypeImpl
size_t instance_size;
void (*class_init)(ObjectClass *klass, void *data);
+void (*class_base_init)(ObjectClass *klass, void *data);
void (*class_finalize)(ObjectClass *klass, void *data);
void *class_data;
@@ -112,6 +113,7 @@ TypeImpl *type_register(const TypeInfo *info)
ti-instance_size = info-instance_size;
ti-class_init = info-class_init;
+ti-class_base_init = info-class_base_init;
ti-class_finalize = info-class_finalize;
ti-class_data = info-class_data;
@@ -232,6 +234,13 @@ static void type_initialize(TypeImpl *ti)
memcpy((void *)ti-class + sizeof(ObjectClass),
(void *)parent-class + sizeof(ObjectClass),
parent-class_size - sizeof(ObjectClass));
+
+while (parent) {
+if (parent-class_base_init) {
+parent-class_base_init(ti-class, ti-class_data);
+}
+parent = type_get_parent(parent);
+}
}
memset((void *)ti-class + class_size, 0, ti-class_size - class_size);
--
1.7.9.3
[Qemu-devel] [PATCH 17/21] qdev: move sysbus initialization to sysbus.c
TYPE_SYSTEM_BUS will be local to hw/sysbus.c, so move existing references
to main_system_bus and system_bus_info there.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c | 26 ++
hw/sysbus.c | 21 +
2 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 7b2802d..7816a37 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -34,10 +34,6 @@ int qdev_hotplug = 0;
static bool qdev_hot_added = false;
static bool qdev_hot_removed = false;
-/* This is a nasty hack to allow passing a NULL bus to qdev_create. */
-static BusState *main_system_bus;
-static void main_system_bus_create(void);
-
/* Register a new device type. */
const VMStateDescription *qdev_get_vmsd(DeviceState *dev)
{
@@ -188,14 +184,6 @@ static int qdev_reset_one(DeviceState *dev, void *opaque)
return 0;
}
-BusState *sysbus_get_default(void)
-{
-if (!main_system_bus) {
-main_system_bus_create();
-}
-return main_system_bus;
-}
-
static int qbus_reset_one(BusState *bus, void *opaque)
{
if (bus-info-reset) {
@@ -415,7 +403,7 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
if (parent) {
QLIST_INSERT_HEAD(parent-child_bus, bus, sibling);
parent-num_child_bus++;
-} else if (bus != main_system_bus) {
+} else if (bus != sysbus_get_default()) {
/* TODO: once all bus devices are qdevified,
only reset handler for main_system_bus should be registered here. */
qemu_register_reset(qbus_reset_all_fn, bus);
@@ -432,16 +420,6 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent,
const char *name)
return bus;
}
-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 = g_malloc0(system_bus_info.size);
-main_system_bus-qdev_allocated = 1;
-qbus_create_inplace(main_system_bus, system_bus_info, NULL,
-main-system-bus);
-}
-
void qbus_free(BusState *bus)
{
DeviceState *dev;
@@ -453,7 +431,7 @@ void qbus_free(BusState *bus)
QLIST_REMOVE(bus, sibling);
bus-parent-num_child_bus--;
} else {
-assert(bus != main_system_bus); /* main_system_bus is never freed */
+assert(bus != sysbus_get_default()); /* main_system_bus is never freed
*/
qemu_unregister_reset(qbus_reset_all_fn, bus);
}
g_free((void*)bus-name);
diff --git a/hw/sysbus.c b/hw/sysbus.c
index db4efcc..fe5c421 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -256,6 +256,27 @@ static TypeInfo sysbus_device_type_info = {
.class_init = sysbus_device_class_init,
};
+/* This is a nasty hack to allow passing a NULL bus to qdev_create. */
+static BusState *main_system_bus;
+
+static void main_system_bus_create(void)
+{
+/* assign main_system_bus before qbus_create_inplace()
+ * in order to make if (bus != sysbus_get_default()) work */
+main_system_bus = g_malloc0(system_bus_info.size);
+main_system_bus-qdev_allocated = 1;
+qbus_create_inplace(main_system_bus, system_bus_info, NULL,
+main-system-bus);
+}
+
+BusState *sysbus_get_default(void)
+{
+if (!main_system_bus) {
+main_system_bus_create();
+}
+return main_system_bus;
+}
+
static void sysbus_register_types(void)
{
type_register_static(sysbus_device_type_info);
--
1.7.9.3
Re: [Qemu-devel] [Qemu-ppc] [PATCH 20/22] ppc: move load and store helpers, switch to AREG0 free mode
On 04/30/2012 05:34 PM, malc wrote:
On Mon, 30 Apr 2012, Alexander Graf wrote:
On 30.04.2012, at 12:45, Alexander Graf wrote:
On 22.04.2012, at 15:26, Blue Swirl wrote:
Add an explicit CPUPPCState parameter instead of relying on AREG0
and rename op_helper.c (which only contains load and store helpers)
to mem_helper.c. Remove AREG0 swapping in
tlb_fill().
Switch to AREG0 free mode. Use cpu_ld{l,uw}_code in translation
and interrupt handling, cpu_{ld,st}{l,uw}_data in loads and stores.
This patch breaks qemu-system-ppc64 on ppc32 host user space for me. I'm trying
to debug it down, but worst case I'll omit this patch set for 1.1.
Ok, so apparently nobody ever tested TCG_AREG0 mode with the ppc tcg
target. It looks as if the
64-bit-guest-registers-in-32-bit-host-registers code path is missing
completely.
This actually makes me less confident that this is a change we want for
1.1. I'll remove the patches from the queue.
Alex
TCG register swizzling code:
#ifdef CONFIG_TCG_PASS_AREG0
/* XXX/FIXME: suboptimal */
tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3],
tcg_target_call_iarg_regs[2]);
tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2],
tcg_target_call_iarg_regs[1]);
tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1],
tcg_target_call_iarg_regs[0]);
tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0],
TCG_AREG0);
#endif
tcg_out_call (s, (tcg_target_long) qemu_st_helpers[opc], 1);
The above snippet is incorrect for SysV ppc32 ABI, due to misalignment
of long long argument in register file.
Hmm - so what would be the correct version? :)
Alex
[Qemu-devel] [PATCH 06/21] qdev: push type property up to Object
Now that Object is a type, add an instance_init function and push
the type property from qdev to there.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/qdev.c|6 --
qom/object.c | 11 +++
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 0bcde20..94fb32e 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -515,11 +515,6 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
return strdup(path);
}
-static char *qdev_get_type(Object *obj, Error **errp)
-{
-return g_strdup(object_get_typename(obj));
-}
-
/**
* Legacy property handling
*/
@@ -635,7 +630,6 @@ static void device_initfn(Object *obj)
qdev_property_add_static(dev, prop, NULL);
}
-object_property_add_str(OBJECT(dev), type, qdev_get_type, NULL, NULL);
qdev_prop_set_defaults(dev, qdev_get_props(dev));
}
diff --git a/qom/object.c b/qom/object.c
index e536ece..464fc8f 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1204,6 +1204,16 @@ void object_property_add_str(Object *obj, const char
*name,
prop, errp);
}
+static char *qdev_get_type(Object *obj, Error **errp)
+{
+return g_strdup(object_get_typename(obj));
+}
+
+static void object_instance_init(Object *obj)
+{
+object_property_add_str(obj, type, qdev_get_type, NULL, NULL);
+}
+
static void register_types(void)
{
static TypeInfo interface_info = {
@@ -1215,6 +1225,7 @@ static void register_types(void)
static TypeInfo object_info = {
.name = TYPE_OBJECT,
.instance_size = sizeof(Object),
+.instance_init = object_instance_init,
.abstract = true,
};
--
1.7.9.3
[Qemu-devel] [PATCH 05/21] qom: assert that public types have a non-NULL parent field
This protects against unwanted effects of changing TYPE_OBJECT from
NULL to a string. Suggested by Andreas Faerber.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
include/qemu/object.h |1 -
qom/object.c | 14 ++
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 9c49d99..ec2b382 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -527,7 +527,6 @@ const char *object_get_typename(Object *obj);
*/
Type type_register_static(const TypeInfo *info);
-#define type_register_static_alias(info, name) do { } while (0)
/**
* type_register:
diff --git a/qom/object.c b/qom/object.c
index 88ec417..e536ece 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -95,7 +95,7 @@ static TypeImpl *type_table_lookup(const char *name)
return g_hash_table_lookup(type_table_get(), name);
}
-TypeImpl *type_register(const TypeInfo *info)
+static TypeImpl *type_register_internal(const TypeInfo *info)
{
TypeImpl *ti = g_malloc0(sizeof(*ti));
@@ -137,6 +137,12 @@ TypeImpl *type_register(const TypeInfo *info)
return ti;
}
+TypeImpl *type_register(const TypeInfo *info)
+{
+assert(info-parent);
+return type_register_internal(info);
+}
+
TypeImpl *type_register_static(const TypeInfo *info)
{
return type_register(info);
@@ -204,7 +210,7 @@ static void type_class_interface_init(TypeImpl *ti,
InterfaceImpl *iface)
char *name = g_strdup_printf(%s::%s, ti-name, iface-parent);
info.name = name;
-iface-type = type_register(info);
+iface-type = type_register_internal(info);
g_free(name);
}
@@ -1212,8 +1218,8 @@ static void register_types(void)
.abstract = true,
};
-type_interface = type_register_static(interface_info);
-type_register_static(object_info);
+type_interface = type_register_internal(interface_info);
+type_register_internal(object_info);
}
type_init(register_types)
--
1.7.9.3
Re: [Qemu-devel] [PATCH 11/21] qdev: move bus properties to abstract superclasses
This little bit of magic is a bit too magical for my taste. Polymorphism relies on the idea that a subclass overloads base class members/methods. From the base classes perspective, it's unaware if a subclass has overloaded something (that's allowed to be overloaded). This code doesn't get the current class *as* the base class but rather gets the non-overloaded form of the base class. This isn't really something that most OO systems allow and I think it could lead to major ugliness in the future. Not really, in fact this kind of class-side data is really bread and butter of all dynamic languages, and it's how most of them implement polymorphism. They have an associative array (method names - method bytecode for example) in each class on the hierarchy, and walk the hierarchy for each function call. Doing method lookups like that is not something we can do in C of course, but for data it's perfectly fine IMHO. I much prefer moving property installation to a function call that is invoked during base class init. That leaves you without the possibility to inspect static property info, unless you do the gross-ish hack of calling object_new and immediately freeing the object. Paolo
Re: [Qemu-devel] [PATCH] slirp: Untangle TCPOLEN_* from TCPOPT_*
But that would leave Illumos broken. I'd rather apply this one. Please do. The right solution for 1.2 is to understand the places in which slirp cannot use netinet/tcp.h constants (the MSS for example), and otherwise use that header. Paolo
Re: [Qemu-devel] [PATCH] slirp: Untangle TCPOLEN_* from TCPOPT_*
Am 02.05.2012 15:24, schrieb Paolo Bonzini: But that would leave Illumos broken. For the record, reverting would leave whatever Paolo was fixing broken but would restore Illumos. I'd rather apply this one. Please do. The right solution for 1.2 is to understand the places in which slirp cannot use netinet/tcp.h constants (the MSS for example), and otherwise use that header. Jan, should I prepare a mechanical prefix addition as suggested by Blue? Should we apply this ugly-but-least-intrusive build fix? Or do you have any other preference or idea? Andreas
Re: [Qemu-devel] [SeaBIOS] [PATCH 11/12] Migrate 64bit entries to 64bit pci regions
Hi, Is this just a matter of removing the if (pci_bdf_to_bus(pci-bdf) != 0) break from pci_bios_init_devices()? Seems to do the trick, at least the disks connected appear in the boot menu now and the seabios log file looks sane. The guest kernel has no virtio-scsi drivers though, need to update it for more testing. The code should probably handle the irq swizzling that pci bridges do though. i.e. add bridge handling to pci_slot_get_irq() ? cheers, Gerd
Re: [Qemu-devel] [PATCH] slirp: Untangle TCPOLEN_* from TCPOPT_*
On 2012-05-02 10:40, Andreas Färber wrote: Am 02.05.2012 15:24, schrieb Paolo Bonzini: But that would leave Illumos broken. For the record, reverting would leave whatever Paolo was fixing broken but would restore Illumos. I'd rather apply this one. Please do. The right solution for 1.2 is to understand the places in which slirp cannot use netinet/tcp.h constants (the MSS for example), and otherwise use that header. Jan, should I prepare a mechanical prefix addition as suggested by Blue? Should we apply this ugly-but-least-intrusive build fix? Or do you have any other preference or idea? I'm fine with a minimal fix like you suggested. The renaming should probably be applied to a broader scope. So it's an exercise to be done systematically and without any hurry. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH qemu v2 0/6] -no-user-config option, move CPU models to /usr/share
Anthony, isn't this going to get in for 1.1? I was expecting it to be applied before the freeze. On Tue, Apr 24, 2012 at 05:32:55PM -0300, Eduardo Habkost wrote: Changes v1 - v2: - Move qemu_read_default_config_files() prototype to qemu-config.h - Make defconfig and userconfig variable bool - Coding style change Patches 1 to 4 just move some code around, patch 5 just adds the new option without adding any new config file. Patch 6 finally creates a /usr/share/qemu /cpus-x86_64.conf file, with the CPU models we currently have on Qemu. Reference to previous discussion: - http://marc.info/?l=qemu-develm=133278877315665 Eduardo Habkost (6): move code to read default config files to a separate function (v2) eliminate arch_config_name variable move list of default config files to an array vl.c: change 'defconfig' variable to bool implement -no-user-config command-line option (v2) move CPU definitions to /usr/share/qemu/cpus-x86_64.conf (v2) Makefile | 12 +++- arch_init.c | 32 - arch_init.h |2 - qemu-config.h|4 + qemu-options.hx | 16 - sysconfigs/target/cpus-x86_64.conf | 128 ++ sysconfigs/target/target-x86_64.conf | 128 -- vl.c | 18 ++--- 8 files changed, 193 insertions(+), 147 deletions(-) create mode 100644 sysconfigs/target/cpus-x86_64.conf -- 1.7.3.2 -- Eduardo
Re: [Qemu-devel] [PATCH qemu v2 0/6] -no-user-config option, move CPU models to /usr/share
Am 02.05.2012 15:50, schrieb Eduardo Habkost: Anthony, isn't this going to get in for 1.1? I was expecting it to be applied before the freeze. You wrote you would respin it with s/int/bool/. :) 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 0/2] pci: 64bit bits
Hi, seabios (master branch) just got 64bit pci support. When running out of address space in the pci memory window below 4G (0xe000+) seabios will map 64bit pci bars above 4G to make room below 4G. This patch series carries two little patches for qemu to adapt it to the seabios changes. First patch creates a memory window for the 64bit pci bars. Second patch adds a 64bit option to the ivshmem driver, which allows to use huge shared memory chunks. please review apply, Gerd Gerd Hoffmann (2): pc: add pci64 memory hole ivshmem: add 64bit option hw/ivshmem.c | 13 ++--- hw/pc.c | 17 ++--- hw/pc.h |6 ++ hw/pc_piix.c | 22 ++ 4 files changed, 40 insertions(+), 18 deletions(-)
[Qemu-devel] [PATCH 1/2] pc: add pci64 memory hole
This patch adds a address space hole for 64bit PCI ressources.
It starts at 0x80 (512 GB) and ends at 0x100 (1 TB),
thus has 512 GB in size. This matches what the seabios is doing
(latest master branch).
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/pc.c | 17 ++---
hw/pc.h |6 ++
hw/pc_piix.c | 22 ++
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/hw/pc.c b/hw/pc.c
index 4d34a33..de1b297 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -981,12 +981,13 @@ void pc_memory_init(MemoryRegion *system_memory,
const char *initrd_filename,
ram_addr_t below_4g_mem_size,
ram_addr_t above_4g_mem_size,
+ram_addr_t above_1t_mem_size,
MemoryRegion *rom_memory,
MemoryRegion **ram_memory)
{
int linux_boot, i;
MemoryRegion *ram, *option_rom_mr;
-MemoryRegion *ram_below_4g, *ram_above_4g;
+MemoryRegion *ram_below_4g, *ram_above_4g, *ram_above_1t;
void *fw_cfg;
linux_boot = (kernel_filename != NULL);
@@ -997,7 +998,9 @@ void pc_memory_init(MemoryRegion *system_memory,
*/
ram = g_malloc(sizeof(*ram));
memory_region_init_ram(ram, pc.ram,
- below_4g_mem_size + above_4g_mem_size);
+ below_4g_mem_size +
+ above_4g_mem_size +
+ above_1t_mem_size);
vmstate_register_ram_global(ram);
*ram_memory = ram;
ram_below_4g = g_malloc(sizeof(*ram_below_4g));
@@ -1008,9 +1011,17 @@ void pc_memory_init(MemoryRegion *system_memory,
ram_above_4g = g_malloc(sizeof(*ram_above_4g));
memory_region_init_alias(ram_above_4g, ram-above-4g, ram,
below_4g_mem_size, above_4g_mem_size);
-memory_region_add_subregion(system_memory, 0x1ULL,
+memory_region_add_subregion(system_memory, PCI_HOLE_END,
ram_above_4g);
}
+if (above_1t_mem_size 0) {
+ram_above_1t = g_malloc(sizeof(*ram_above_1t));
+memory_region_init_alias(ram_above_1t, ram-above-1t, ram,
+ below_4g_mem_size + above_4g_mem_size,
+ above_1t_mem_size);
+memory_region_add_subregion(system_memory, PCI_HOLE64_END,
+ram_above_1t);
+}
/* Initialize PC system firmware */
diff --git a/hw/pc.h b/hw/pc.h
index 74d3369..0c5e14e 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -12,6 +12,11 @@
/* PC-style peripherals (also used by other machines). */
+#define PCI_HOLE_START 0xe000ULL
+#define PCI_HOLE_END 0x0001ULL
+#define PCI_HOLE64_START 0x0080ULL
+#define PCI_HOLE64_END 0x0100ULL
+
/* serial.c */
SerialState *serial_init(int base, qemu_irq irq, int baudbase,
@@ -112,6 +117,7 @@ void pc_memory_init(MemoryRegion *system_memory,
const char *initrd_filename,
ram_addr_t below_4g_mem_size,
ram_addr_t above_4g_mem_size,
+ram_addr_t above_1t_mem_size,
MemoryRegion *rom_memory,
MemoryRegion **ram_memory);
qemu_irq *pc_allocate_cpu_irq(void);
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6a75718..27f990f 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -133,7 +133,7 @@ static void pc_init1(MemoryRegion *system_memory,
int kvmclock_enabled)
{
int i;
-ram_addr_t below_4g_mem_size, above_4g_mem_size;
+ram_addr_t below_4g_mem_size, above_4g_mem_size, above_1t_mem_size;
PCIBus *pci_bus;
ISABus *isa_bus;
PCII440FXState *i440fx_state;
@@ -157,13 +157,10 @@ static void pc_init1(MemoryRegion *system_memory,
kvmclock_create();
}
-if (ram_size = 0xe000 ) {
-above_4g_mem_size = ram_size - 0xe000;
-below_4g_mem_size = 0xe000;
-} else {
-above_4g_mem_size = 0;
-below_4g_mem_size = ram_size;
-}
+below_4g_mem_size = MIN(ram_size, PCI_HOLE_START);
+above_4g_mem_size = MIN(ram_size - below_4g_mem_size,
+PCI_HOLE64_START - PCI_HOLE_END);
+above_1t_mem_size = ram_size - below_4g_mem_size - above_4g_mem_size;
if (pci_enabled) {
pci_memory = g_new(MemoryRegion, 1);
@@ -178,7 +175,7 @@ static void pc_init1(MemoryRegion *system_memory,
if (!xen_enabled()) {
pc_memory_init(system_memory,
kernel_filename, kernel_cmdline, initrd_filename,
- below_4g_mem_size, above_4g_mem_size,
+ below_4g_mem_size, above_4g_mem_size, above_1t_mem_size,
pci_enabled ? rom_memory : system_memory, ram_memory);
}
@@ -195,11 +192,12 @@ static void pc_init1(MemoryRegion *system_memory,
[Qemu-devel] [PATCH 2/2] ivshmem: add 64bit option
This patch adds a use64 property which will make the ivshmem driver
register a 64bit memory bar when set, so you have something to play with
when testing 64bit pci bits. It also allows to have quite big shared
memory regions, like this:
[root@fedora ~]# lspci -vs1:1
01:01.0 RAM memory: Red Hat, Inc Device 1110
Subsystem: Red Hat, Inc Device 1100
Physical Slot: 1-1
Flags: fast devsel
Memory at fd40 (32-bit, non-prefetchable) [disabled] [size=256]
Memory at 804000 (64-bit, prefetchable) [size=1G]
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
hw/ivshmem.c | 13 ++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index d48e5f9..9c16c7b 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -72,6 +72,8 @@ typedef struct IVShmemState {
MemoryRegion ivshmem;
MemoryRegion msix_bar;
uint64_t ivshmem_size; /* size of shared memory region */
+uint32_t ivshmem_attr;
+uint32_t ivshmem_64bit;
int shm_fd; /* shared memory file descriptor */
Peer *peers;
@@ -344,7 +346,7 @@ static void create_shared_memory_BAR(IVShmemState *s, int
fd) {
memory_region_add_subregion(s-bar, 0, s-ivshmem);
/* region for shared memory */
-pci_register_bar(s-dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY, s-bar);
+pci_register_bar(s-dev, 2, s-ivshmem_attr, s-bar);
}
static void close_guest_eventfds(IVShmemState *s, int posn)
@@ -694,6 +696,11 @@ static int pci_ivshmem_init(PCIDevice *dev)
s-ivshmem_mmio);
memory_region_init(s-bar, ivshmem-bar2-container, s-ivshmem_size);
+s-ivshmem_attr = PCI_BASE_ADDRESS_SPACE_MEMORY |
+PCI_BASE_ADDRESS_MEM_PREFETCH;
+if (s-ivshmem_64bit) {
+s-ivshmem_attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+}
if ((s-server_chr != NULL)
(strncmp(s-server_chr-filename, unix:, 5) == 0)) {
@@ -719,8 +726,7 @@ static int pci_ivshmem_init(PCIDevice *dev)
/* allocate/initialize space for interrupt handling */
s-peers = g_malloc0(s-nb_peers * sizeof(Peer));
-pci_register_bar(s-dev, 2,
- PCI_BASE_ADDRESS_SPACE_MEMORY, s-bar);
+pci_register_bar(s-dev, 2, s-ivshmem_attr, s-bar);
s-eventfd_chr = g_malloc0(s-vectors * sizeof(CharDriverState *));
@@ -792,6 +798,7 @@ static Property ivshmem_properties[] = {
DEFINE_PROP_BIT(msi, IVShmemState, features, IVSHMEM_MSI, true),
DEFINE_PROP_STRING(shm, IVShmemState, shmobj),
DEFINE_PROP_STRING(role, IVShmemState, role),
+DEFINE_PROP_UINT32(use64, IVShmemState, ivshmem_64bit, 0),
DEFINE_PROP_END_OF_LIST(),
};
--
1.7.1
Re: [Qemu-devel] [PATCH qemu v2 0/6] -no-user-config option, move CPU models to /usr/share
On Wed, May 02, 2012 at 04:01:55PM +0200, Andreas Färber wrote: Am 02.05.2012 15:50, schrieb Eduardo Habkost: Anthony, isn't this going to get in for 1.1? I was expecting it to be applied before the freeze. You wrote you would respin it with s/int/bool/. :) I sent an additional patch instead of respinning (see patch 7/6 sent as reply to this series). -- Eduardo
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 02/05/12 14:54, Alexander Graf wrote: On 05/02/2012 01:38 PM, Paolo Bonzini wrote: On 05/02/2012 01:26 PM, Paolo Bonzini wrote: and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? If we make things a flag for the guessing code, it should work just as well with image files, right? Only when they're not blank. :) I was only thinking of #ifdef __s390__ for the call to HDIO_GETGEO. Well, if guessing is a function guess_size(disk_size, block_size) then we would be able to do the same on an image file. Christian, would that work? I think that the geometry values can not always be guessed correctly based on block_size and disk_size. Stefan, can you clarify that? If we cannot reliably guess the geometry based on blocksize and size, I still think that we should use the host values, e.g. after checking that BIODASDINFO2 returns successfully. Christian
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
Am 02.05.2012 12:08, schrieb Michael Tokarev:
02.05.2012 13:57, Kevin Wolf пишет:
Am 30.04.2012 17:52, schrieb Michael Tokarev:
This value is used currently for virtio-blk only. It was defined
as uint16_t before, which is the same as in kernel=user interface
(in virtio_blk.h, struct virtio_blk_config). But the problem is
that in kernel=user interface the units are sectors (which is
usually 512 bytes or more), while in qemu it is in bytes. However,
for, say, md raid5 arrays, it is typical to have actual min_io_size
of a host device to be large. For example, for raid5 device of
3 drives with 64Kb chunk size, that value will be 128Kb, which does
not fit in a uint16_t anymore.
Increase the value size from 16bits to 32bits for now.
But apparently, the kernel=user interface needs to be fixed too
(while it is much more difficult due to compatibility issues),
because even with 512byte units, the 16bit value there will overflow
too with quite normal MD RAID configuration.
Signed-off-by: Michael Tokarev m...@tls.msk.ru
---
block.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block.h b/block.h
index f163e54..cd5ae79 100644
--- a/block.h
+++ b/block.h
@@ -425,7 +425,7 @@ typedef struct BlockConf {
BlockDriverState *bs;
uint16_t physical_block_size;
uint16_t logical_block_size;
-uint16_t min_io_size;
+uint32_t min_io_size;
uint32_t opt_io_size;
int32_t bootindex;
uint32_t discard_granularity;
@@ -450,7 +450,7 @@ static inline unsigned int
get_physical_block_exp(BlockConf *conf)
_conf.logical_block_size, 512), \
DEFINE_PROP_BLOCKSIZE(physical_block_size, _state,\
_conf.physical_block_size, 512), \
-DEFINE_PROP_UINT16(min_io_size, _state, _conf.min_io_size, 0), \
+DEFINE_PROP_UINT32(min_io_size, _state, _conf.min_io_size, 0), \
DEFINE_PROP_UINT32(opt_io_size, _state, _conf.opt_io_size, 0),\
DEFINE_PROP_INT32(bootindex, _state, _conf.bootindex, -1),\
DEFINE_PROP_UINT32(discard_granularity, _state, \
Don't you need an additional check in virtio-blk now so that you don't
overflow the 16 bit field in the virtio protocol? And I guess the same
for SCSI, where INQUIRY reports only a 16 bits sector count as well.
That probably should be added too, but I'm not sure I agree these should be
added into virtio-blk.
As I already mentioned, the virtio protocol has the same defect (but there
it is less serious due to usage of larger units). And that's where the
additional overflow needs to be ELIMINATED, not just checked. Ie, the
protocol should be changed somehow - the only issue is that I don't know
how to do that now, once it has been in use for quite some time.
Even if you create a new version of the protocol (introduce a new
feature flag or whatever), newer qemus will still have to deal with
older guests and vice versa.
Note that now, we don't have ANY checks of this theme whatsoever, at all:
I tried using 128K as min_io_size, and the guest sees it as 0 currently, --
the limits (at least the fact that the value fits in the defined number
of bits) should be checked in the common function which implements these
DEFINE_PROP_UINT* defines.
Looks like a bug in the qdev property parser to me. 128k is obviously an
invalid value for a 16 bit property.
But now that you're extending the property value to 32 bits, but only 25
bits can be really used, the property type doesn't even theoretically
suffice as a check.
I'm not sure I can fix all of this in one go, so I went on and fixed the
most specific bug first, without any additional bad side effects.
Besides, as I also already mentioned, this whole structure is used in
virtio-blk *only*, so only virtio driver passes this information into
guest, scsi does not use it. And again, in case of scsi, the units are
*sectors*, not bytes.
Yes, same as for virtio-blk.
But I can't see which structure is only used by virtio-blk, though. The
min_io_size property is contained in DEFINE_BLOCK_PROPERTIES(), which is
used by every qdevified block device. Most of them ignore min_io_size,
but virtio-blk and scsi-disk don't.
So maybe the solution is to keep this as 16bits but switch to sectors.
But for this, DEFINE_PROP_UINT can't be used anymore, unless we agree
to change user interface TOO and require the user to specify these
values in sectors to start with.
That wouldn't be a good interface. Let's just take a 32 bit number and
add the checks in the devices.
And yes, if SCSI uses 16bit value here, it will have the same issue
as virtio currently have -- pretty normal MD RAID array can't be
expressed using 16bit number of sectors already... Oh well, but we
at least have a (small) chance to fix virtio.
Just curious... What values do you want to use? The 32 MB minimum I/O
size that are possible with 16 bits
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 4:06 PM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, May 1, 2012 at 13:54, Artyom Tarasenko atar4q...@gmail.com wrote:
On Tue, May 1, 2012 at 11:25 AM, Blue Swirl blauwir...@gmail.com wrote:
On Mon, Apr 30, 2012 at 17:38, Artyom Tarasenko atar4q...@gmail.com wrote:
On Mon, Apr 30, 2012 at 7:15 PM, Andreas Färber afaer...@suse.de 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.
Instead, you should add a callback function pointer to SPARCCPUClass
that you initialize based on CPU model so that is behaves differently at
runtime rather than at compile time.
Or if it's just about the class_init then after the Hard Freeze I can
start polishing my subclasses for sparc so that you can add a special
class_init for Niagara.
But this would mean that the defines from
#define TTE_NFO_BIT (1ULL 60)
to
#define TTE_PGSIZE(tte) (((tte) 61) 3ULL)
inclusive would need to be replaced with functions and variables?
Sounds like a further performance regression for sun4u?
There could be parallel definitions for sun4u (actually UltraSparc-III
onwards the MMU is again different) and sun4v.
At tlb_fill(), different implementations can be selected based on MMU
model. For ASI accesses, we can add conditional code but for higher
performance, some checks can be moved to translation time.
Can be done, but what is the gain of having it runtime configurable?
I was thinking of code like this in:
switch (env-mmu_model) {
case MMU_US2:
return tlb_fill_us2(..);
case MMU_US3:
return tlb_fill_us3(..);
case MMU_US4:
return tlb_fill_us4(..);
case MMU_T1:
return tlb_fill_t1(..);
case MMU_T2:
return tlb_fill_t2(..);
}
The perfomance cost shouldn't be too high. Alternatively a function
pointer could be set up.
Actually I was more worried about get_physical_address_* than filling,
there we would have to use variables instead of constants and
functions instead of macros.
Yes, we can always provide the register bank, older models just access
some of those.
cpu_change_pstate should probably have another parameter (new_GL)
which is only valid for sun4v.
And, depending on a trap type, env-htba has to be taken instead of
env-tbr. To me it looks like at the end do_interrupt will have less
common parts between sun4u and sun4v than specific ones.
Same as tlb_fill(), switch() or function pointer. The functions are different.
This is unavoidable (unless maybe in the future the TLB handling can
be pushed partially higher so mmu_idx parameters can be eliminated)
and the performance cost is not great.
So, altogether you'd still prefer run-time checks over having
qemu-system-sun4v (or -sparc64v) ?
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] Poking a sun4v machine
On Tue, May 1, 2012 at 3:54 PM, Blue Swirl blauwir...@gmail.com wrote:
On Tue, May 1, 2012 at 13:33, Artyom Tarasenko atar4q...@gmail.com wrote:
On Tue, May 1, 2012 at 11:19 AM, Blue Swirl blauwir...@gmail.com wrote:
On Mon, Apr 30, 2012 at 16:39, Artyom Tarasenko atar4q...@gmail.com wrote:
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.
I don't think this is needed. The MMU index tells which TLB is used
for guest virtual to host address translations, during tl == MAXTL we
want to use hypervisor mode translations.
Is just about the address translations? I thought it affects TB cache
as well, which has to be invalidated between nucleus, super- and
hypervisor switches?
This is (should be) handled by cpu_get_tb_cpu_state(). But it looks
like only supervisor mode (PS_PRIV) is taken into account, not others.
Also when D/I MMU is enabled or disabled, TLB is flushed, so there's
no need to store these bits.
I'm not sure TL is interesting either.
And what about the MMU contexts? UA2005 does have 3 ones: Primary,
Secondary and Nucleus.
Is it not relevant here?
--
Regards,
Artyom Tarasenko
solaris/sparc under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH 2/3] geometry detection: use HDIO_GETGEO
On 02.05.2012, at 16:27, Christian Borntraeger borntrae...@de.ibm.com wrote: On 02/05/12 14:54, Alexander Graf wrote: On 05/02/2012 01:38 PM, Paolo Bonzini wrote: On 05/02/2012 01:26 PM, Paolo Bonzini wrote: and everyone should be happy :). I would really like to have as little #ifdef TARGET_S390 code in QEMU. And #ifdef __s390__ is even worse, as it means we won't be able to execise that code path on other architectures. True, but how do you exercise that code path with DASD geometry on !__s390__? If we make things a flag for the guessing code, it should work just as well with image files, right? Only when they're not blank. :) I was only thinking of #ifdef __s390__ for the call to HDIO_GETGEO. Well, if guessing is a function guess_size(disk_size, block_size) then we would be able to do the same on an image file. Christian, would that work? I think that the geometry values can not always be guessed correctly based on block_size and disk_size. Stefan, can you clarify that? If we cannot reliably guess the geometry based on blocksize and size, I still think that we should use the host values, e.g. after checking that BIODASDINFO2 returns successfully. Yeah, but only if it's always possible to force a specific geometry through the command line - otherwise reproducability suffers. Alex
Re: [Qemu-devel] Memory API: handling unassigned physical memory
On 5/1/2012 1:48 PM, Mark Cave-Ayland wrote: 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? It would be nice to have the device offsets be relative to the slot. User-pluggable sbus devices should be possible. I've just pushed an update of a dbri sbus device model to github ( https://github.com/breuerr/qemu/commits/dbri-pre2 ). This device was built-in to at least the SS-20, but also available as an sbus add-in card (SunLink ISDN). There's an fcode rom so it can be probed by OBP, and if we could get something like -device SUNW,,DBRIe,slot=1 to work, then a user could add it to most of the sun4m machine models. Bob
Re: [Qemu-devel] [PATCH 2/2] ivshmem: add 64bit option
On 05/02/2012 05:02 PM, Gerd Hoffmann wrote: This patch adds a use64 property which will make the ivshmem driver register a 64bit memory bar when set, so you have something to play with when testing 64bit pci bits. It also allows to have quite big shared memory regions, like this: [root@fedora ~]# lspci -vs1:1 01:01.0 RAM memory: Red Hat, Inc Device 1110 Subsystem: Red Hat, Inc Device 1100 Physical Slot: 1-1 Flags: fast devsel Memory at fd40 (32-bit, non-prefetchable) [disabled] [size=256] Memory at 804000 (64-bit, prefetchable) [size=1G] Suggest making this the default for -M current. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/2] pci: 64bit bits
On 05/02/2012 05:02 PM, Gerd Hoffmann wrote: Hi, seabios (master branch) just got 64bit pci support. When running out of address space in the pci memory window below 4G (0xe000+) seabios will map 64bit pci bars above 4G to make room below 4G. This patch series carries two little patches for qemu to adapt it to the seabios changes. First patch creates a memory window for the 64bit pci bars. Second patch adds a 64bit option to the ivshmem driver, which allows to use huge shared memory chunks. What happens if a pre-64-bit-pci guest boots on such a setup? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] increase BlockConf.min_io_size type from uint16_t to uint32_t
On 02.05.2012 18:35, Kevin Wolf wrote: [] As I already mentioned, the virtio protocol has the same defect (but there it is less serious due to usage of larger units). And that's where the additional overflow needs to be ELIMINATED, not just checked. Ie, the protocol should be changed somehow - the only issue is that I don't know how to do that now, once it has been in use for quite some time. Even if you create a new version of the protocol (introduce a new feature flag or whatever), newer qemus will still have to deal with older guests and vice versa. Sure. And for these, the checks indeed should be done in lower layers. [] But now that you're extending the property value to 32 bits, but only 25 bits can be really used, the property type doesn't even theoretically suffice as a check. So, what do you propose? To add a check into virtio-blk.c (and into whatever else place is using this variable) too? If yes, and indeed it appears to be the right thing to do, care to show me the right place please, I'm not very familiar with that code... [] But I can't see which structure is only used by virtio-blk, though. The min_io_size property is contained in DEFINE_BLOCK_PROPERTIES(), which is used by every qdevified block device. Most of them ignore min_io_size, but virtio-blk and scsi-disk don't. I mean the BlockConf struct. It isn't used anywhere but in virtio-blk.c. But again, since I'm not familiar with the code, I might be wrong. [] That wouldn't be a good interface. Let's just take a 32 bit number and add the checks in the devices. That's fine. The only thing left to do is to find the proper places for the checks. Help? [] Just curious... What values do you want to use? The 32 MB minimum I/O size that are possible with 16 bits and 512 byte sectors already sounds insanely large. I don't use any values. I merely pass whatever is defined on my systems down to the guest. md layer in kernel - raid4, raid5 and raid6 implementation - sets min_io_size to the chunk size, and opt_io_size to stripe size. It is not uncommon at all to have chunk size = 1Mb or more. I've no idea how useful this information is, but at least with it present (my small raid5 array has 256Kb chunk size), xfs created in the guest performs much faster than without this information (which means usual 512 there). This is how I discovered the issue - I wondered why xfs created in the guest is so much slower than the same xfs but created on host. I/O sizes immediately come to min, so I added these, but it was still slow. So I noticed min_io_size isn't passed correctly, increased the size of this type, and voila, xfs created in guest now behaves as fast as created on host. Something like that anyway. There's an obvious bug in there, but it is not obvious for me where/how it should be fixed. Maybe the sizes used by md raid5 are insane, that's arguable ofcourse, but this is what is in use now (and since the day the i/o sizes were added to md to start with), and this is what makes things fly. Thanks, /mjt
Re: [Qemu-devel] [PATCH] vnc: disable VNC password authentication (security type 2) when in FIPS mode
On Wednesday, May 02, 2012 12:54:21 AM Andreas Färber wrote:
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
Thanks, typos fixed.
auth is not allowed in FIPS mode.
Signed-off-by: Paul Moore pmo...@redhat.com
Interesting feature. :)
It would appear that depends on who you ask :)
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
syslog.h is POSIX, but it'll need a guard for mingw32.
Is #ifndef _WIN32 the right guard to use? Both here and where we make the
actual syslog() call?
@@ -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?
It is in the mainline Linux Kernel so fairly standard as far as Linux is
concerned. However, it is Linux only to the best of my knowledge so I've gone
ahead and protected it with __linux__ as you and others have mentioned.
+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.
Yep, I agree.
I'll post a v2 later today; thanks for taking the time to review the patch and
send your comments.
--
paul moore
security and virtualization @ redhat
Re: [Qemu-devel] [PATCH 1/2] pc: add pci64 memory hole
On 05/02/12 17:31, Avi Kivity wrote: On 05/02/2012 05:02 PM, Gerd Hoffmann wrote: This patch adds a address space hole for 64bit PCI ressources. It starts at 0x80 (512 GB) and ends at 0x100 (1 TB), thus has 512 GB in size. This matches what the seabios is doing (latest master branch). We should communicate this to seabios via fwcfg A dsdt entry is needed for the pci64 window (see http://code.coreboot.org/p/seabios/source/commit/482a020ec25f4cec655ddcb16b67c6f38b0844c0/), which makes it non-trivial to turn this into a runtime option ... It should also only apply to -M old. Is this the only reason you want this be runtime-switchable? cheers, Gerd
