Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
On Wed, 2009-11-04 at 18:52 -0600, Anthony Liguori wrote: > I'd rather make the "default" network configurable via a global > configuration file. That way, if a distribution knew that it had a > bridge setup for its users, it could make -net bridge the default. Fair enough. :-Dustin signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
Dustin Kirkland wrote: We address this problem by introducing a new network backend: -net bridge. This backend is less flexible than -net tap because it relies on a helper with elevated privileges to do the heavy lifting of allocating and attaching a tap device to a bridge. We use a special purpose helper because we don't want to elevate the privileges of more generic tools like brctl. From a user perspective, to use bridged networking with a guest, you simply use: qemu -hda linux.img -net bridge -net nic I know that this patch is less than a day old and untested, but would it be reasonable to make this the "default" network configuration at some point in the future? This certainly seems to be what I want 99% of the time when I launch qemu or kvm by hand from the command line. I'd rather make the "default" network configurable via a global configuration file. That way, if a distribution knew that it had a bridge setup for its users, it could make -net bridge the default. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
On Tue, 2009-11-03 at 18:28 -0600, Anthony Liguori wrote: > This series solves a problem that I've been struggling with for a few years > now. > One of the best things about qemu is that it's possible to run guests as an > unprivileged user to improve security. However, if you want to have your > guests > communicate with the outside world, you're pretty much forced to run qemu as > root. > > At least with KVM support, this is probably the most common use case which > means > that most of our users are running qemu as root. That's terrible. Ack. > We address this problem by introducing a new network backend: -net bridge. > This > backend is less flexible than -net tap because it relies on a helper with > elevated privileges to do the heavy lifting of allocating and attaching a tap > device to a bridge. We use a special purpose helper because we don't want > to elevate the privileges of more generic tools like brctl. > > From a user perspective, to use bridged networking with a guest, you simply > use: > > qemu -hda linux.img -net bridge -net nic I know that this patch is less than a day old and untested, but would it be reasonable to make this the "default" network configuration at some point in the future? This certainly seems to be what I want 99% of the time when I launch qemu or kvm by hand from the command line. > And assuming a bridge is defined named qemubr0 and the administrator has setup > permissions accordingly, it will Just Work. My hope is that distributions > will > do this work as part of the qemu packaging process such that for most users, > the out-of-the-box experience will also Just Work. Also, ack. I'll handle the Ubuntu packaging to enable this support in Lucid by the time qemu-0.12-rc1 is available. As Alexander mentions, there's a bit more complexity we'll need to account for (wifi, network manager, multiple nic's). :-Dustin signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] Inquiry:Solaris 8 installation on QEMU
>> The kernels of Solaris 8 & 9 can be boot too, but then they flood >> about spurious irq 10. It seems that the earlier Solaris versions are >> also suffering from spurious interrupts, because the boot process >> takes very long: ~7 hours on e8...@2.66ghz . > > A bug in the system timer implementation? Or in interrupt handling in general. The interrupts seem to be really spurious, not triggered by the timer. I think I fixed one bug in slavio today, but I need more reading slavio docs. With the fix solaris 9 seems to boot as well. Although it still complains about spurious interrupts, but with the fix it happens once a second and not hundreds times a second as before. The performance is not affected, so either the fix is incomplete, or I took a wrong trail
Re: [Qemu-devel] [PATCH] don't call reset functions on cpu initialization\
On Wed, Nov 04, 2009 at 02:51:08PM -0200, Marcelo Tosatti wrote: > On Tue, Nov 03, 2009 at 05:50:05PM -0200, Glauber Costa wrote: > > There is absolutely no need to call reset functions when initializing > > devices. Since we are already registering them, calling qemu_system_reset() > > should suffice. Actually, it is what happens when we reboot the machine, > > and using the same process instead of a special case semantics will even > > allow us to find bugs easier. > > > > Furthermore, the fact that we initialize things like the cpu quite early, > > leads to the need to introduce synchronization stuff like qemu_system_cond. > > This patch removes it entirely. All we need to do is call > > qemu_system_reset() > > only when we're already sure the system is up and running > > > > I tested it with qemu (with and without io-thread) and qemu-kvm, and it > > seems to be doing okay - although qemu-kvm uses a slightly different patch. > > > > Signed-off-by: Glauber Costa > > --- > > hw/apic.c|1 - > > hw/e1000.c |1 - > > hw/hpet.c|1 - > > hw/i8254.c |2 -- > > hw/ide/piix.c|1 - > > hw/piix4.c |1 - > > hw/piix_pci.c|1 - > > hw/rtl8139.c |2 +- > > hw/serial.c |1 - > > hw/usb-ohci.c|1 - > > hw/usb-uhci.c|1 - > > hw/vga.c |1 - > > target-i386/helper.c |1 - > > vl.c | 15 +-- > > 14 files changed, 2 insertions(+), 28 deletions(-) > > > > diff --git a/hw/apic.c b/hw/apic.c > > index c89008e..87e7dc0 100644 > > --- a/hw/apic.c > > +++ b/hw/apic.c > > @@ -981,7 +981,6 @@ int apic_init(CPUState *env) > > s->id = env->cpuid_apic_id; > > s->cpu_env = env; > > > > -apic_reset(s); > > msix_supported = 1; > > > > /* XXX: mapping more APICs at the same memory location */ > > diff --git a/hw/e1000.c b/hw/e1000.c > > index 028afd1..8fb299d 100644 > > --- a/hw/e1000.c > > +++ b/hw/e1000.c > > @@ -1113,7 +1113,6 @@ static int pci_e1000_init(PCIDevice *pci_dev) > > qemu_format_nic_info_str(d->vc, macaddr); > > > > vmstate_register(-1, &vmstate_e1000, d); > > -e1000_reset(d); > > > > if (!pci_dev->qdev.hotplugged) { > > static int loaded = 0; > > diff --git a/hw/hpet.c b/hw/hpet.c > > index 64163bd..6f39711 100644 > > --- a/hw/hpet.c > > +++ b/hw/hpet.c > > @@ -577,7 +577,6 @@ void hpet_init(qemu_irq *irq) { > > HPETTimer *timer = &s->timer[i]; > > timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer); > > } > > -hpet_reset(s); > > vmstate_register(-1, &vmstate_hpet, s); > > qemu_register_reset(hpet_reset, s); > > /* HPET Area */ > > diff --git a/hw/i8254.c b/hw/i8254.c > > index 5c49e6e..faaa884 100644 > > --- a/hw/i8254.c > > +++ b/hw/i8254.c > > @@ -513,7 +513,5 @@ PITState *pit_init(int base, qemu_irq irq) > > register_ioport_write(base, 4, 1, pit_ioport_write, pit); > > register_ioport_read(base, 3, 1, pit_ioport_read, pit); > > > > -pit_reset(pit); > > - > > return pit; > > } > > diff --git a/hw/ide/piix.c b/hw/ide/piix.c > > index a17bf59..60b37a3 100644 > > --- a/hw/ide/piix.c > > +++ b/hw/ide/piix.c > > @@ -120,7 +120,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d) > > pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type > > > > qemu_register_reset(piix3_reset, d); > > -piix3_reset(d); > > > > pci_register_bar(&d->dev, 4, 0x10, PCI_ADDRESS_SPACE_IO, bmdma_map); > > > > diff --git a/hw/piix4.c b/hw/piix4.c > > index a6aea15..f75951b 100644 > > --- a/hw/piix4.c > > +++ b/hw/piix4.c > > @@ -97,7 +97,6 @@ static int piix4_initfn(PCIDevice *d) > > PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // > > header_type = PCI_multifunction, generic > > > > piix4_dev = d; > > -piix4_reset(d); > > qemu_register_reset(piix4_reset, d); > > return 0; > > } > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > > index ed036fe..20d834f 100644 > > --- a/hw/piix_pci.c > > +++ b/hw/piix_pci.c > > @@ -341,7 +341,6 @@ static int piix3_initfn(PCIDevice *dev) > > pci_conf[PCI_HEADER_TYPE] = > > PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // > > header_type = PCI_multifunction, generic > > > > -piix3_reset(d); > > qemu_register_reset(piix3_reset, d); > > return 0; > > } > > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > > index d26df48..27cc618 100644 > > --- a/hw/rtl8139.c > > +++ b/hw/rtl8139.c > > @@ -3331,7 +3331,7 @@ static int pci_rtl8139_init(PCIDevice *dev) > > PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map); > > > > qemu_macaddr_default_if_unset(&s->conf.macaddr); > > -rtl8139_reset(&s->dev.qdev); > > + > > s->vc = qemu_new_vlan_client(NET_CLIENT_TYPE_NIC, > > s->conf.vlan, s->conf.peer, > >
Re: [Qemu-devel] [PATCH] Fix a parallel build failure.
On Wed, Nov 04, 2009 at 09:48:22PM +0100, Stefan Weil wrote: > Maybe a mix of your patch and my patch > (http://patchwork.ozlabs.org/patch/37446/) > would be the best fix for this problem. > > It should also be possible to apply both patches. Thanks, I didn't find your patch. I don't have any preference which is applied, although I'm always in favor of eliminating unnecessary recursive invocations. -- Daniel Jacobowitz CodeSourcery
Re: [Qemu-devel] [PATCH] Fix a parallel build failure.
Daniel Jacobowitz schrieb: > From: Daniel Jacobowitz > > With enough parallelism, make will run all the dependencies of > build-all at the same time: > > build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS) > > So some of the $(TOOLS) will build before config-host.h is finished. > The object files need to depend on it explicitly. Subdirectories are > OK since they are started from the body of build-all, not its > dependencies. > > Signed-off-by: Daniel Jacobowitz > --- > Makefile |2 ++ > rules.mak |2 +- > 2 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/Makefile b/Makefile > index c783aa4..ed9a420 100644 > --- a/Makefile > +++ b/Makefile > @@ -1,5 +1,7 @@ > # Makefile for QEMU. > > +GENERATED_HEADERS = config-host.h config-all-devices.h > + > ifneq ($(wildcard config-host.mak),) > # Put the all: rule here so that config-host.mak can contain dependencies. > all: build-all > diff --git a/rules.mak b/rules.mak > index 5d7e8bb..4eb1f90 100644 > --- a/rules.mak > +++ b/rules.mak > @@ -13,7 +13,7 @@ MAKEFLAGS += -rR > > QEMU_CFLAGS += -MMD -MP -MT $@ > > -%.o: %.c > +%.o: %.c $(GENERATED_HEADERS) > $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<," CC > $(TARGET_DIR)$@") > > %.o: %.S > Maybe a mix of your patch and my patch (http://patchwork.ozlabs.org/patch/37446/) would be the best fix for this problem. It should also be possible to apply both patches. Regards Stefan
Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
Michael S. Tsirkin wrote: Well it doesn't really help with the issue of privileges which is what this series is really about. Regards, Anthony Liguori I note that by default you grant all users all access. If you do that, just give them net cap admin already? By default, I give no users any access. qemu-bridge-helper carries cap_net_admin but it doesn't do everything cap_net_admin does. Since an administrator has to set that capability, the admin is going to make it owned by root so that an unprivileged user cannot change it. Modulo bugs, it's a very restricted subset of cap_net_admin. In order for a user to be able to get a tap device connected to a bridge, the following things must be true: 1) the user must have execute privileges for qemu-bridge-helper 2) the user must have read/write access to /dev/net/tun 3) there must be an /etc/qemu/bridge.conf that is readable by the user 4) the config must have an explicit rule allowing access to the required bridge device So the user is very restricted in what they can do and they must be granted these permissions explicitly by an administrator. By using multiple bridge.conf files, an administrator can also create policies based on filesystem permissions allowing certain user/groups to access only certain bridges. With raw, qemu must carry cap_net_raw. That is definitely not safe for an untrusted user. Allowing an untrusted user to connect a VM to a bridged physical network, on the other hand, seems to be a rather safe thing to do as long as there are strongly ways to control which bridges they can connect to. Regards, Anthony Liguori
[Qemu-devel] [PATCH] Fix a parallel build failure.
From: Daniel Jacobowitz With enough parallelism, make will run all the dependencies of build-all at the same time: build-all: config-host.h config-all-devices.h $(DOCS) $(TOOLS) So some of the $(TOOLS) will build before config-host.h is finished. The object files need to depend on it explicitly. Subdirectories are OK since they are started from the body of build-all, not its dependencies. Signed-off-by: Daniel Jacobowitz --- Makefile |2 ++ rules.mak |2 +- 2 files changed, 3 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index c783aa4..ed9a420 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,7 @@ # Makefile for QEMU. +GENERATED_HEADERS = config-host.h config-all-devices.h + ifneq ($(wildcard config-host.mak),) # Put the all: rule here so that config-host.mak can contain dependencies. all: build-all diff --git a/rules.mak b/rules.mak index 5d7e8bb..4eb1f90 100644 --- a/rules.mak +++ b/rules.mak @@ -13,7 +13,7 @@ MAKEFLAGS += -rR QEMU_CFLAGS += -MMD -MP -MT $@ -%.o: %.c +%.o: %.c $(GENERATED_HEADERS) $(call quiet-command,$(CC) $(QEMU_CFLAGS) $(CFLAGS) -c -o $@ $<," CC $(TARGET_DIR)$@") %.o: %.S -- 1.6.5.2
Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
On Wed, Nov 04, 2009 at 01:48:01PM -0600, Anthony Liguori wrote: > Michael S. Tsirkin wrote: >> On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote: >> >>> This series solves a problem that I've been struggling with for a few years >>> now. >>> One of the best things about qemu is that it's possible to run guests as an >>> unprivileged user to improve security. However, if you want to have your >>> guests >>> communicate with the outside world, you're pretty much forced to run qemu as >>> root. >>> >>> At least with KVM support, this is probably the most common use case which >>> means >>> that most of our users are running qemu as root. That's terrible. >>> >>> We address this problem by introducing a new network backend: -net bridge. >>> This >>> backend is less flexible than -net tap because it relies on a helper with >>> elevated privileges to do the heavy lifting of allocating and attaching a >>> tap >>> device to a bridge. We use a special purpose helper because we don't want >>> to elevate the privileges of more generic tools like brctl. >>> >>> >From a user perspective, to use bridged networking with a guest, you >>> >simply use: >>> >>> qemu -hda linux.img -net bridge -net nic >>> >>> And assuming a bridge is defined named qemubr0 and the administrator has >>> setup >>> permissions accordingly, it will Just Work. My hope is that distributions >>> will >>> do this work as part of the qemu packaging process such that for most users, >>> the out-of-the-box experience will also Just Work. >>> >>> More details are included in individual patches. I broke up the helper into >>> a series of patches to improve reviewabilty. >>> >> >> Would raw backend attached to a bridge mostly do the same? >> > > Well it doesn't really help with the issue of privileges which is what > this series is really about. > > Regards, > > Anthony Liguori I note that by default you grant all users all access. If you do that, just give them net cap admin already? -- MST
[Qemu-devel] [PATCH 8/8] monitor: do_info_balloon(): use QError
Signed-off-by: Luiz Capitulino --- monitor.c |7 --- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/monitor.c b/monitor.c index 0e6e21b..423c6b5 100644 --- a/monitor.c +++ b/monitor.c @@ -1717,10 +1717,11 @@ static void do_info_balloon(Monitor *mon, QObject **ret_data) actual = qemu_balloon_status(); if (kvm_enabled() && !kvm_has_sync_mmu()) -monitor_printf(mon, "Using KVM without synchronous MMU, " - "ballooning disabled\n"); +qemu_error_new(QERR_SER_UNAV, + "Using KVM without synchronous MMU, ballooning disabled", + NULL); else if (actual == 0) -monitor_printf(mon, "Ballooning not activated in VM\n"); +qemu_error_new(QERR_SER_UNAV, "Ballooning not activated in VM", NULL); else *ret_data = QOBJECT(qint_from_int((int)(actual >> 20))); } -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 7/8] qdev: Use QError for not found error
Signed-off-by: Luiz Capitulino --- hw/qdev.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/qdev.c b/hw/qdev.c index c7884d0..db86cb2 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -29,6 +29,7 @@ #include "qdev.h" #include "sysemu.h" #include "monitor.h" +#include "qerror.h" static int qdev_hotplug = 0; @@ -176,8 +177,8 @@ DeviceState *qdev_device_add(QemuOpts *opts) /* find driver */ info = qdev_find_info(NULL, driver); if (!info) { -qemu_error("Device \"%s\" not found. Try -device '?' for a list.\n", - driver); +qemu_error_new(QERR_DEV_NFOUND, "device \"%(name)s\" not found", + "{ 'name': %s }", driver); return NULL; } if (info->no_user) { -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 6/8] monitor: QError support
This commit adds QError support in the Monitor. A QError member is added to the Monitor struct. This new member stores error information and is also used to check if an error has occurred when the called handlers returns. Additionally, a new macro called qemu_error_new() is introduced. It should be used in pace of qemu_error() to report errors. When all conversion to qemu_error_new() is done, qemu_error() can be turned private. Basically, Monitor's error flow is something like this: 1. An error occurs in the handler, it calls qemu_error_new() 2. qemu_error_new() builds a new QError object and stores it in the Monitor struct 3. The handler returns 4. Top level Monitor code checks the Monitor struct and calls qerror_print() to print the error Signed-off-by: Luiz Capitulino --- monitor.c | 43 ++- sysemu.h |7 +++ 2 files changed, 49 insertions(+), 1 deletions(-) diff --git a/monitor.c b/monitor.c index 109ff5c..0e6e21b 100644 --- a/monitor.c +++ b/monitor.c @@ -49,6 +49,7 @@ #include "qlist.h" #include "qdict.h" #include "qstring.h" +#include "qerror.h" //#define DEBUG //#define DEBUG_COMPLETION @@ -103,6 +104,7 @@ struct Monitor { CPUState *mon_cpu; BlockDriverCompletionFunc *password_completion_cb; void *password_opaque; +QError *error; QLIST_HEAD(,mon_fd_t) fds; QLIST_ENTRY(Monitor) entry; }; @@ -3146,6 +3148,18 @@ fail: return NULL; } +static inline int monitor_has_error(const Monitor *mon) +{ +return mon->error != NULL; +} + +static void monitor_print_error(Monitor *mon) +{ +qerror_print(mon->error); +QDECREF(mon->error); +mon->error = NULL; +} + static void monitor_handle_command(Monitor *mon, const char *cmdline) { QDict *qdict; @@ -3171,7 +3185,10 @@ static void monitor_handle_command(Monitor *mon, const char *cmdline) cmd->mhandler.cmd(mon, qdict); } - qemu_errors_to_previous(); +if (monitor_has_error(mon)) +monitor_print_error(mon); + +qemu_errors_to_previous(); out: QDECREF(qdict); @@ -3622,3 +3639,27 @@ void qemu_error(const char *fmt, ...) break; } } + +void qemu_error_full(const char *name, const char *file, int linenr, + const char *desc, const char *fmt, ...) +{ +va_list va; +QError *qerror; + +assert(qemu_error_sink != NULL); + +va_start(va, fmt); +qerror = qerror_from_info(name, file, linenr, desc, fmt, &va); +va_end(va); + +switch (qemu_error_sink->dest) { +case ERR_SINK_FILE: +qerror_print(qerror); +QDECREF(qerror); +break; +case ERR_SINK_MONITOR: +assert(qemu_error_sink->mon->error == NULL); +qemu_error_sink->mon->error = qerror; +break; +} +} diff --git a/sysemu.h b/sysemu.h index 96804b4..20ab709 100644 --- a/sysemu.h +++ b/sysemu.h @@ -7,6 +7,7 @@ #include "qemu-queue.h" #include "qemu-timer.h" #include "qdict.h" +#include "qerror.h" #ifdef _WIN32 #include @@ -71,6 +72,12 @@ void qemu_errors_to_file(FILE *fp); void qemu_errors_to_mon(Monitor *mon); void qemu_errors_to_previous(void); void qemu_error(const char *fmt, ...) __attribute__ ((format(printf, 1, 2))); +void qemu_error_full(const char *name, const char *file, int linenr, + const char *desc, const char *fmt, ...) + __attribute__ ((format(printf, 5, 6))); + +#define qemu_error_new(name, desc, fmt, ...) \ +qemu_error_full(name, __FILE__, __LINE__, desc, fmt, ## __VA_ARGS__) #ifdef _WIN32 /* Polling handling */ -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 5/8] Introduce QError
QError is a high-level data type which represents an exception, it stores the following error information: - name A generic error name (eg. "ServiceUnavailable") - descriptionA detailed error description, which may contain references to run-time error data - filename The file name of where the error occurred - line numberThe exact line number of the error - run-time data Run-time error data The qerror_print() function should be used to properly format and print the stored information to the right place, that is, to stderr if the Monitor is not running, or to the Monitor's device otherwise. The following functions are exported: - qerror_new(): Create a new QError - qerror_from_info(): Create a new QError from the specified error information - qerror_print(): Print the specified QError Signed-off-by: Luiz Capitulino --- Makefile |2 +- qerror.c | 235 + qerror.h | 39 ++ qobject.h |1 + 4 files changed, 276 insertions(+), 1 deletions(-) create mode 100644 qerror.c create mode 100644 qerror.h diff --git a/Makefile b/Makefile index 6fcbcaa..2dfaebd 100644 --- a/Makefile +++ b/Makefile @@ -135,7 +135,7 @@ obj-y += buffered_file.o migration.o migration-tcp.o qemu-sockets.o obj-y += qemu-char.o aio.o savevm.o obj-y += msmouse.o ps2.o obj-y += qdev.o qdev-properties.o -obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o +obj-y += qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qjson.o qerror.o obj-y += qemu-config.o obj-$(CONFIG_BRLAPI) += baum.o diff --git a/qerror.c b/qerror.c new file mode 100644 index 000..7d3137f --- /dev/null +++ b/qerror.c @@ -0,0 +1,235 @@ +/* + * QError: QEMU Error data-type. + * + * Copyright (C) 2009 Red Hat Inc. + * + * Authors: + * Luiz Capitulino + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + */ +#include "qint.h" +#include "qjson.h" +#include "qerror.h" +#include "qstring.h" +#include "sysemu.h" +#include "qemu-common.h" + +static void qerror_destroy_obj(QObject *obj); + +static const QType qerror_type = { +.code = QTYPE_QERROR, +.destroy = qerror_destroy_obj, +}; + +/** + * qerror_new(): Create a new QError + * + * Return strong reference. + */ +QError *qerror_new(void) +{ +QError *qerr; + +qerr = qemu_mallocz(sizeof(*qerr)); +QOBJECT_INIT(qerr, &qerror_type); + +return qerr; +} + +/** + * qerror_from_info(): Create a new QError from error information + * + * The information consists of: + * + * - namegeneric error name + * - filethe file name of where the error occurred + * - linenr the line number of where the error occurred + * - descdetailed description (see below) + * - fmt JSON printf-like format for 'specific data' + * - va va_list of all arguments for 'specific data' + * + * Note that this is a low-level function, it is supposed to be called + * by higher-level functions or macros. + * + * The 'desc' parameter is a printf-like string, the format of the format + * string is: + * + * %(KEY)TYPE + * + * Where KEY is a QDict key and TYPE is the type of its value, KEY and + * its value must be passed to qerror_from_info(). + * + * Valid types are: + * + * s (string) + * d (integer) + * + * Example: + * + * "foo error on device: %(device)s slot: %(slot_nr)d" + * + * A single percent sign can be printed if followed by a second one, + * for example: + * + * "running out of foo: %(foo)d%%" + * + * Return strong reference. + */ +QError *qerror_from_info(const char *name, const char *file, int linenr, + const char *desc, const char *fmt, va_list *va) +{ +QError *qerr; + +qerr = qerror_new(); +qerr->name = name; +qerr->file = file; +qerr->linenr = linenr; +qerr->desc = desc; +if (fmt) { +qerr->data = qobject_from_json_va(fmt, va); +assert(qerr->data != NULL); +} + +return qerr; +} + +static char *get_substr(const char *start, const char *end) +{ +char *str; +size_t length; + +length = end - start + 1; +str = qemu_malloc(length + 1); +memcpy(str, start, length); +str[length] = '\0'; + +return str; +} + +static void qerror_abort(const QError *qerr) +{ +fprintf(stderr, " in '%s' at %s:%d\n", qerr->desc, qerr->file,qerr->linenr); +abort(); +} + +#define ERROR_PREFIX "\n\nqerror: " + +static void type_error(const QError *qerr, int c) +{ +fprintf(stderr, ERROR_PREFIX "invalid type '%c'", c); +qerror_abort(qerr); +} + +static void key_error(const QError *qerr, const char *key) +{ +fprintf(stderr, ERROR_PREFIX "key '%s' not found in QDict ", key); +fprintf(stderr, "call at %s:%d\n", qerr->file, qerr->linenr); +abort(); +} + +static void parse_error(const QError *qerror, int c) +{ +fprintf(stderr, ERROR_PREFIX "expected '%c'", c); +qerror_abort(
[Qemu-devel] [PATCH 4/8] QString: Introduce qstring_append_int()
Signed-off-by: Luiz Capitulino --- qstring.c |8 qstring.h |2 ++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/qstring.c b/qstring.c index e422bd9..ad17769 100644 --- a/qstring.c +++ b/qstring.c @@ -75,6 +75,14 @@ void qstring_append(QString *qstring, const char *str) qstring->string[qstring->length] = 0; } +void qstring_append_int(QString *qstring, int64_t value) +{ +char num[32]; + +snprintf(num, sizeof(num), "%" PRId64, value); +qstring_append(qstring, num); +} + /** * qstring_append_chr(): Append a C char to a QString */ diff --git a/qstring.h b/qstring.h index 43581de..c065331 100644 --- a/qstring.h +++ b/qstring.h @@ -1,6 +1,7 @@ #ifndef QSTRING_H #define QSTRING_H +#include #include "qobject.h" typedef struct QString { @@ -13,6 +14,7 @@ typedef struct QString { QString *qstring_new(void); QString *qstring_from_str(const char *str); const char *qstring_get_str(const QString *qstring); +void qstring_append_int(QString *qstring, int64_t value); void qstring_append(QString *qstring, const char *str); void qstring_append_chr(QString *qstring, int c); QString *qobject_to_qstring(const QObject *obj); -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 3/8] Add qstring_append_chr() unit-test
Signed-off-by: Luiz Capitulino --- check-qstring.c | 17 + 1 files changed, 17 insertions(+), 0 deletions(-) diff --git a/check-qstring.c b/check-qstring.c index ea4dfd0..412038a 100644 --- a/check-qstring.c +++ b/check-qstring.c @@ -55,6 +55,22 @@ START_TEST(qstring_get_str_test) } END_TEST +START_TEST(qstring_append_chr_test) +{ +int i; +QString *qstring; +const char *str = "qstring append char unit-test"; + +qstring = qstring_new(); + +for (i = 0; str[i]; i++) +qstring_append_chr(qstring, str[i]); + +fail_unless(strcmp(str, qstring_get_str(qstring)) == 0); +QDECREF(qstring); +} +END_TEST + START_TEST(qobject_to_qstring_test) { QString *qstring; @@ -78,6 +94,7 @@ static Suite *qstring_suite(void) tcase_add_test(qstring_public_tcase, qstring_from_str_test); tcase_add_test(qstring_public_tcase, qstring_destroy_test); tcase_add_test(qstring_public_tcase, qstring_get_str_test); +tcase_add_test(qstring_public_tcase, qstring_append_chr_test); tcase_add_test(qstring_public_tcase, qobject_to_qstring_test); return s; -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 2/8] QString: Introduce qstring_append_chr()
It appends a C char to a QString. Signed-off-by: Luiz Capitulino --- qstring.c | 24 +++- qstring.h |1 + 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/qstring.c b/qstring.c index 441a9e6..e422bd9 100644 --- a/qstring.c +++ b/qstring.c @@ -53,25 +53,39 @@ QString *qstring_from_str(const char *str) return qstring; } -/* qstring_append(): Append a C string to a QString - */ -void qstring_append(QString *qstring, const char *str) +static void capacity_increase(QString *qstring, size_t len) { -size_t len = strlen(str); - if (qstring->capacity < (qstring->length + len)) { qstring->capacity += len; qstring->capacity *= 2; /* use exponential growth */ qstring->string = qemu_realloc(qstring->string, qstring->capacity + 1); } +} + +/* qstring_append(): Append a C string to a QString + */ +void qstring_append(QString *qstring, const char *str) +{ +size_t len = strlen(str); +capacity_increase(qstring, len); memcpy(qstring->string + qstring->length, str, len); qstring->length += len; qstring->string[qstring->length] = 0; } /** + * qstring_append_chr(): Append a C char to a QString + */ +void qstring_append_chr(QString *qstring, int c) +{ +capacity_increase(qstring, 1); +qstring->string[qstring->length++] = c; +qstring->string[qstring->length] = 0; +} + +/** * qobject_to_qstring(): Convert a QObject to a QString */ QString *qobject_to_qstring(const QObject *obj) diff --git a/qstring.h b/qstring.h index 65905d4..43581de 100644 --- a/qstring.h +++ b/qstring.h @@ -14,6 +14,7 @@ QString *qstring_new(void); QString *qstring_from_str(const char *str); const char *qstring_get_str(const QString *qstring); void qstring_append(QString *qstring, const char *str); +void qstring_append_chr(QString *qstring, int c); QString *qobject_to_qstring(const QObject *obj); #endif /* QSTRING_H */ -- 1.6.5.2.143.g8cc62
[Qemu-devel] [PATCH 1/8] QJSon: Introduce qobject_from_json_va()
Simple wrapper to parse_json() that accepts a va_list, will be used by QError. Signed-off-by: Luiz Capitulino --- qjson.c |5 + qjson.h |2 ++ 2 files changed, 7 insertions(+), 0 deletions(-) diff --git a/qjson.c b/qjson.c index 5f92996..02fcd83 100644 --- a/qjson.c +++ b/qjson.c @@ -723,3 +723,8 @@ QObject *qobject_from_jsonf(const char *string, size_t *length, ...) return obj; } + +QObject *qobject_from_json_va(const char *string, va_list *ap) +{ +return parse_json(string, NULL, ap); +} diff --git a/qjson.h b/qjson.h index 0c94954..da0d653 100644 --- a/qjson.h +++ b/qjson.h @@ -14,10 +14,12 @@ #ifndef QJSON_H #define QJSON_H +#include #include "qobject.h" QObject *qobject_from_json(const char *string, size_t *length); QObject *qobject_from_jsonf(const char *string, size_t *length, ...) __attribute__((__format__ (__printf__, 1, 3))); +QObject *qobject_from_json_va(const char *string, va_list *ap); #endif /* QJSON_H */ -- 1.6.5.2.143.g8cc62
[Qemu-devel] [RFC 0/8]: QError v2
Hi, I can't remember seeing updated versions of a RFC series, but this should prevent Anthony's scripts from merging these patches. This new QError version has two major changes: the static error table has been dropped and I'm using symbolic names instead of error codes. Now, a call to: monitor_printf(mon, "husb: host usb device %d.%d is already open\n", bus_num, addr); Would become something like: qemu_error_new('DeviceAlreadyOpen', "{ 'bus_num': %d, 'addr': %d }", bus_num, addr); Which is basically what Anthony and other people were asking for, the only difference is that I'm not passing the symbolic name through the dictionary. The reason is that I have the impression it's less general (as it becomes mandatory to have a dict) and slightly more complicaded. The symbolic name can be freely defined, but we can have the common ones in qerror.h. Hopefully this version addresses the most important issues. Luiz.
Re: [Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
Michael S. Tsirkin wrote: On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote: This series solves a problem that I've been struggling with for a few years now. One of the best things about qemu is that it's possible to run guests as an unprivileged user to improve security. However, if you want to have your guests communicate with the outside world, you're pretty much forced to run qemu as root. At least with KVM support, this is probably the most common use case which means that most of our users are running qemu as root. That's terrible. We address this problem by introducing a new network backend: -net bridge. This backend is less flexible than -net tap because it relies on a helper with elevated privileges to do the heavy lifting of allocating and attaching a tap device to a bridge. We use a special purpose helper because we don't want to elevate the privileges of more generic tools like brctl. >From a user perspective, to use bridged networking with a guest, you simply use: qemu -hda linux.img -net bridge -net nic And assuming a bridge is defined named qemubr0 and the administrator has setup permissions accordingly, it will Just Work. My hope is that distributions will do this work as part of the qemu packaging process such that for most users, the out-of-the-box experience will also Just Work. More details are included in individual patches. I broke up the helper into a series of patches to improve reviewabilty. Would raw backend attached to a bridge mostly do the same? Well it doesn't really help with the issue of privileges which is what this series is really about. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH] sparc32 fix carry flag handling (Solaris bootblk fix)
On Wed, Nov 4, 2009 at 1:58 AM, Artyom Tarasenko wrote: > The page 108 of the SPARC Version 8 Architecture Manual describes > that addcc and addxcc shall compute carry flag the same way. > The page 110 claims the same about subcc and subxcc instructions. > This patch fixes carry computation in corner cases and removes redundant code. > The most visible effect of the patch is enabling Solaris boot when using OBP. Thanks, applied. Could you describe the steps how to boot Solaris with OBP? I'm sure there are a lot of people who'd like to test if their favorite Sparc Solaris programs work on QEMU.
Re: [Qemu-devel] Inquiry:Solaris 8 installation on QEMU
On Wed, Nov 4, 2009 at 1:50 AM, Artyom Tarasenko wrote: > 2009/9/19 Blue Swirl : >> Even Sparc32 can't boot Solaris for some mysterious reason. > > Not so mysterious anymore! Mitch Bradley found that subcc instruction > was not correctly setting carry flag in the case where both arguments > were 0 and carry flag was previously set. Fixing the bug allowed to > start booting Solaris 2.5.1 and Solaris 2.6 up to /sbin/init. > Afterwards I found more corner cases in add(x)cc and sub(x)cc carry > handling. Now Solaris 2.5.1 ( > http://tyom.blogspot.com/2009/10/greetings-professor-falken.html ) and > 2.6 ( > http://tyom.blogspot.com/2009/11/another-week-another-solaris-version.html > ) can be boot in a single user mode. Awesome! > The kernels of Solaris 8 & 9 can be boot too, but then they flood > about spurious irq 10. It seems that the earlier Solaris versions are > also suffering from spurious interrupts, because the boot process > takes very long: ~7 hours on e8...@2.66ghz . A bug in the system timer implementation?
[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.
On Wed, Nov 04, 2009 at 05:37:13PM +0200, Michael S. Tsirkin wrote: > On Wed, Nov 04, 2009 at 04:17:46PM +0100, Aurelien Jarno wrote: > > On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote: > > > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote: > > > > > --- a/hw/pci_host.c > > > > > +++ b/hw/pci_host.c > > > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## > > > > > __VA_ARGS__); } while (0) > > > > > #define PCI_DPRINTF(fmt, ...) > > > > > #endif > > > > > > > > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t > > > > > addr, > > > > > + uint32_t val) > > > > > +{ > > > > > +PCIHostState *s = opaque; > > > > > + > > > > > +#ifdef TARGET_WORDS_BIGENDIAN > > > > > +val = bswap32(val); > > > > > +#endif > > > > > > > > I know you just copied it, but isn't this just > > > > val = le32_to_cpu(val); > > > > > > > > ? > > > > > > Makes sense. > > > > The original code is actually wrong, but le32_to_cpu(val), will break on > > big endian hosts. > > > > The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all > > big endian machines we emulate, the PCI bus is always connected backward, > > so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN. > > > > -- > > Aurelien Jarno GPG: 1024D/F1BCDB73 > > aurel...@aurel32.net http://www.aurel32.net > > Are you speaking about bit endian hosts with little endian guests? > Ugh ... my head hurts. bswap32 is evil because there's no way to > figure out what is converted to what. big to little? guest to host? > This is not related to the host and guest endianess, but just of the way the PCI bus is connected to the CPU on all the big endian guest we emulate. On theses machine, a byteswap is needed when transferring a word between the PCI bus and the CPU. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH 0/4] net-bridge: rootless bridge support for qemu
On Tue, Nov 03, 2009 at 06:28:01PM -0600, Anthony Liguori wrote: > This series solves a problem that I've been struggling with for a few years > now. > One of the best things about qemu is that it's possible to run guests as an > unprivileged user to improve security. However, if you want to have your > guests > communicate with the outside world, you're pretty much forced to run qemu as > root. > > At least with KVM support, this is probably the most common use case which > means > that most of our users are running qemu as root. That's terrible. > > We address this problem by introducing a new network backend: -net bridge. > This > backend is less flexible than -net tap because it relies on a helper with > elevated privileges to do the heavy lifting of allocating and attaching a tap > device to a bridge. We use a special purpose helper because we don't want > to elevate the privileges of more generic tools like brctl. > > >From a user perspective, to use bridged networking with a guest, you simply > >use: > > qemu -hda linux.img -net bridge -net nic > > And assuming a bridge is defined named qemubr0 and the administrator has setup > permissions accordingly, it will Just Work. My hope is that distributions > will > do this work as part of the qemu packaging process such that for most users, > the out-of-the-box experience will also Just Work. > > More details are included in individual patches. I broke up the helper into > a series of patches to improve reviewabilty. Would raw backend attached to a bridge mostly do the same? -- MST
Re: [Qemu-devel] [PATCH] don't call reset functions on cpu initialization\
On Tue, Nov 03, 2009 at 05:50:05PM -0200, Glauber Costa wrote: > There is absolutely no need to call reset functions when initializing > devices. Since we are already registering them, calling qemu_system_reset() > should suffice. Actually, it is what happens when we reboot the machine, > and using the same process instead of a special case semantics will even > allow us to find bugs easier. > > Furthermore, the fact that we initialize things like the cpu quite early, > leads to the need to introduce synchronization stuff like qemu_system_cond. > This patch removes it entirely. All we need to do is call qemu_system_reset() > only when we're already sure the system is up and running > > I tested it with qemu (with and without io-thread) and qemu-kvm, and it > seems to be doing okay - although qemu-kvm uses a slightly different patch. > > Signed-off-by: Glauber Costa > --- > hw/apic.c|1 - > hw/e1000.c |1 - > hw/hpet.c|1 - > hw/i8254.c |2 -- > hw/ide/piix.c|1 - > hw/piix4.c |1 - > hw/piix_pci.c|1 - > hw/rtl8139.c |2 +- > hw/serial.c |1 - > hw/usb-ohci.c|1 - > hw/usb-uhci.c|1 - > hw/vga.c |1 - > target-i386/helper.c |1 - > vl.c | 15 +-- > 14 files changed, 2 insertions(+), 28 deletions(-) > > diff --git a/hw/apic.c b/hw/apic.c > index c89008e..87e7dc0 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -981,7 +981,6 @@ int apic_init(CPUState *env) > s->id = env->cpuid_apic_id; > s->cpu_env = env; > > -apic_reset(s); > msix_supported = 1; > > /* XXX: mapping more APICs at the same memory location */ > diff --git a/hw/e1000.c b/hw/e1000.c > index 028afd1..8fb299d 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -1113,7 +1113,6 @@ static int pci_e1000_init(PCIDevice *pci_dev) > qemu_format_nic_info_str(d->vc, macaddr); > > vmstate_register(-1, &vmstate_e1000, d); > -e1000_reset(d); > > if (!pci_dev->qdev.hotplugged) { > static int loaded = 0; > diff --git a/hw/hpet.c b/hw/hpet.c > index 64163bd..6f39711 100644 > --- a/hw/hpet.c > +++ b/hw/hpet.c > @@ -577,7 +577,6 @@ void hpet_init(qemu_irq *irq) { > HPETTimer *timer = &s->timer[i]; > timer->qemu_timer = qemu_new_timer(vm_clock, hpet_timer, timer); > } > -hpet_reset(s); > vmstate_register(-1, &vmstate_hpet, s); > qemu_register_reset(hpet_reset, s); > /* HPET Area */ > diff --git a/hw/i8254.c b/hw/i8254.c > index 5c49e6e..faaa884 100644 > --- a/hw/i8254.c > +++ b/hw/i8254.c > @@ -513,7 +513,5 @@ PITState *pit_init(int base, qemu_irq irq) > register_ioport_write(base, 4, 1, pit_ioport_write, pit); > register_ioport_read(base, 3, 1, pit_ioport_read, pit); > > -pit_reset(pit); > - > return pit; > } > diff --git a/hw/ide/piix.c b/hw/ide/piix.c > index a17bf59..60b37a3 100644 > --- a/hw/ide/piix.c > +++ b/hw/ide/piix.c > @@ -120,7 +120,6 @@ static int pci_piix_ide_initfn(PCIIDEState *d) > pci_conf[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL; // header_type > > qemu_register_reset(piix3_reset, d); > -piix3_reset(d); > > pci_register_bar(&d->dev, 4, 0x10, PCI_ADDRESS_SPACE_IO, bmdma_map); > > diff --git a/hw/piix4.c b/hw/piix4.c > index a6aea15..f75951b 100644 > --- a/hw/piix4.c > +++ b/hw/piix4.c > @@ -97,7 +97,6 @@ static int piix4_initfn(PCIDevice *d) > PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // > header_type = PCI_multifunction, generic > > piix4_dev = d; > -piix4_reset(d); > qemu_register_reset(piix4_reset, d); > return 0; > } > diff --git a/hw/piix_pci.c b/hw/piix_pci.c > index ed036fe..20d834f 100644 > --- a/hw/piix_pci.c > +++ b/hw/piix_pci.c > @@ -341,7 +341,6 @@ static int piix3_initfn(PCIDevice *dev) > pci_conf[PCI_HEADER_TYPE] = > PCI_HEADER_TYPE_NORMAL | PCI_HEADER_TYPE_MULTI_FUNCTION; // > header_type = PCI_multifunction, generic > > -piix3_reset(d); > qemu_register_reset(piix3_reset, d); > return 0; > } > diff --git a/hw/rtl8139.c b/hw/rtl8139.c > index d26df48..27cc618 100644 > --- a/hw/rtl8139.c > +++ b/hw/rtl8139.c > @@ -3331,7 +3331,7 @@ static int pci_rtl8139_init(PCIDevice *dev) > PCI_ADDRESS_SPACE_MEM, rtl8139_mmio_map); > > qemu_macaddr_default_if_unset(&s->conf.macaddr); > -rtl8139_reset(&s->dev.qdev); > + > s->vc = qemu_new_vlan_client(NET_CLIENT_TYPE_NIC, > s->conf.vlan, s->conf.peer, > dev->qdev.info->name, dev->qdev.id, > diff --git a/hw/serial.c b/hw/serial.c > index 9353201..fa12dcc 100644 > --- a/hw/serial.c > +++ b/hw/serial.c > @@ -725,7 +725,6 @@ static void serial_init_core(SerialState *s) > s->transmit_timer = qemu_new_timer(vm_clock, (QEMUTimerCB *) > serial_xmit, s); > > qemu_r
Re: [Qemu-devel] Re: [PATCH] Add VirtIO Frame Buffer Support
Vincent Hanquez wrote: On Tue, Nov 03, 2009 at 11:38:18AM +0200, Avi Kivity wrote: On 11/03/2009 01:25 PM, Vincent Hanquez wrote: not sure if i'm missing the point here, but couldn't it be hypothetically extended to stuff 3d (or video& more 2d accel ?) commands too ? I can't imagine the cirrus or stdvga driver be able to do that ever ;) cirrus has pretty good 2d acceleration. 3D is a mega-project though. absolutely huge indeed, but still alexander's code is pretty much the only way, to start such a project. with maybe added benefits on more and easier 2d acceleration. or otherwise wait for SR-IOV graphics cards (or similar tech)... I think the real question is do we paravirtualize a VGA device or a framebuffer. Obviously, the advantage of doing a framebuffer is that it works for s390. A VGA device has better backwards compatibility on PCs although it's obviously more complex. In an ideal world, we could expose the virtio framebuffer device as part of PCI device that was also VGA capable (virtio-pci-vga transport?). But then there's QXL on the horizon which complicates matters further. I'd say that virtio-fb should just focus on the s390 use case for now. Let things evolve as needed. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Wed) Nov 04 2009 [15:30:21], Jan Kiszka wrote: > > I've nothing against your patch, specifically as it removes an obviously > no longer needed workaround, not a feature. I just want to make sure > that the current behavior is not only there by chance. Sure; I understand that. I'll be looking more at this code to free it from any surprises. Amit
Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu
Alexander Graf wrote: Well I'm not that familiar with the bridging stuff as I'm rather scared by it myself, but last time I tried if I # brctl addif br0 eth0 # ifconfig br0 up eth0 stopped working, so I had to stop network manager, assign an IP to br0 manually and hope network manager doesn't kick in again. I don't see how we can solve that easily, as most people will want to use NM. netcf will address this properly because it will allow NM to understand bridges and provide a common framework for NM to know about when changes are made to static networking configuration. Regards, Anthony Liguori
[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.
On Wed, Nov 04, 2009 at 04:17:46PM +0100, Aurelien Jarno wrote: > On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote: > > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote: > > > > --- a/hw/pci_host.c > > > > +++ b/hw/pci_host.c > > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## > > > > __VA_ARGS__); } while (0) > > > > #define PCI_DPRINTF(fmt, ...) > > > > #endif > > > > > > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t > > > > addr, > > > > + uint32_t val) > > > > +{ > > > > +PCIHostState *s = opaque; > > > > + > > > > +#ifdef TARGET_WORDS_BIGENDIAN > > > > +val = bswap32(val); > > > > +#endif > > > > > > I know you just copied it, but isn't this just > > > val = le32_to_cpu(val); > > > > > > ? > > > > Makes sense. > > The original code is actually wrong, but le32_to_cpu(val), will break on > big endian hosts. > > The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all > big endian machines we emulate, the PCI bus is always connected backward, > so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN. > > -- > Aurelien JarnoGPG: 1024D/F1BCDB73 > aurel...@aurel32.net http://www.aurel32.net Are you speaking about bit endian hosts with little endian guests? Ugh ... my head hurts. bswap32 is evil because there's no way to figure out what is converted to what. big to little? guest to host? -- MST
[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.
On Wed, Nov 04, 2009 at 03:14:26PM +0900, Isaku Yamahata wrote: > On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote: > > > --- a/hw/pci_host.c > > > +++ b/hw/pci_host.c > > > @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); > > > } while (0) > > > #define PCI_DPRINTF(fmt, ...) > > > #endif > > > > > > +static void pci_host_config_writel(void *opaque, target_phys_addr_t addr, > > > + uint32_t val) > > > +{ > > > +PCIHostState *s = opaque; > > > + > > > +#ifdef TARGET_WORDS_BIGENDIAN > > > +val = bswap32(val); > > > +#endif > > > > I know you just copied it, but isn't this just > > val = le32_to_cpu(val); > > > > ? > > Makes sense. The original code is actually wrong, but le32_to_cpu(val), will break on big endian hosts. The fact is that QEMU doesn't emulate byteswap on buses. Hopefully on all big endian machines we emulate, the PCI bus is always connected backward, so we can simply do the byteswap depending on TARGET_WORDS_BIGENDIAN. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH] ARM NEON shift emulation fix
Hi, friendly reminder about this patch. Thanks, Daniel. Daniel Gutson wrote: Any update on this? Thanks, Daniel. Daniel Gutson wrote: Hi, the attached patch fixes a bug that caused some NEON shift operations to shift a wrong amount of bytes. The problem was that a variable holding an immediate value representing the amount of bits to shift was later overwritten with another value (used for something different) within a loop. Please commit this for me if approved, since I don't have write access. Thanks! Daniel. --- 2009-08-21 Daniel Gutson * target-arm/translate.c (disas_neon_data_insn): Fixed shift operand. -- Daniel Gutson CodeSourcery www.codesourcery.com
Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu
Anthony Liguori wrote: > Alexander Graf wrote: >> Yeah. Worse than the "run as root" part is the "it's hard" part >> though. I hate how I feel when I try to explain someone how to use >> non-slirp networking :-(. >> >> The response to that is then usually "oh whatever, it's too >> complicated anyways". > > I agree and it's a problem I would like to solve too. > >>> And assuming a bridge is defined named qemubr0 and the administrator >>> has setup >>> permissions accordingly, it will Just Work. My hope is that >>> distributions will >>> do this work as part of the qemu packaging process such that for >>> most users, >>> the out-of-the-box experience will also Just Work. >> >> Yeah, that won't work as easily. >> >> When your customer has 2 NICs this will already break. But let's >> imagine we only have a single NIC. >> >> So that NIC is a wifi card. When I set up the qemubr0 bridge for that >> one now, how does network manager configure my wifi access? It can't >> use the bridge device, as that doesn't have wifi extensions. It also >> can't use the wifi device, because setting up networking on that will >> break. >> >> IMHO the only customer friendly choice I see is the ugly way. The way >> VMware and Vbox do it. >> To make it a bit less ugly, maybe we could create some way to "glue" >> a tap device and an eth together, the same way the bridge code does, >> just without the extra device. > > I don't think that helps either. At the end of the day, this is > really a policy decision. You want to expose the subset of > functionality that the majority of your users require. Inevitably, > you'll leave some users who need more complex setups on their own. > > For instance, users with one NIC are certainly common. Two or more > NICs are also pretty common. But what about people that require > guests to be on separate VLANs? > > This is where management tools come in. When running qemu by hand, > the management tool is the distro more or less as they will be > choosing the default policies. What we need to think about is how to > make sure we can seamlessly integrate with whatever policies they want > to implement. > > I think the one thing we could add is a configurable message to print > when a user tries to use a bridge that isn't configured yet. For > instance, if a user runs qemu without a bridge setup, what would be > nice is the following: > > $ qemu -net bridge -net nic -hda ~/images/linux.img > qemu: error running bridge helper > > You currently do not have 'qemubr0' configured. To setup qemubr0, run > (as root): > zypper install qemu-network-setup > qemu-network-setup qemubr0 > > $ sudo qemu-network-setup > Which interface do you want to configure [qemubr0]: qemubr0 > Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge > Which physical interface do you want to bridge qemubr0 to [eth0]: eth0 > qemubr0 is now configured. > > I expect that this is going to be different for every distro. netcf > my make this easier but for now, I think this is the most realistic > approach. Well I'm not that familiar with the bridging stuff as I'm rather scared by it myself, but last time I tried if I # brctl addif br0 eth0 # ifconfig br0 up eth0 stopped working, so I had to stop network manager, assign an IP to br0 manually and hope network manager doesn't kick in again. I don't see how we can solve that easily, as most people will want to use NM. Alex
Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu
Alexander Graf wrote: Yeah. Worse than the "run as root" part is the "it's hard" part though. I hate how I feel when I try to explain someone how to use non-slirp networking :-(. The response to that is then usually "oh whatever, it's too complicated anyways". I agree and it's a problem I would like to solve too. And assuming a bridge is defined named qemubr0 and the administrator has setup permissions accordingly, it will Just Work. My hope is that distributions will do this work as part of the qemu packaging process such that for most users, the out-of-the-box experience will also Just Work. Yeah, that won't work as easily. When your customer has 2 NICs this will already break. But let's imagine we only have a single NIC. So that NIC is a wifi card. When I set up the qemubr0 bridge for that one now, how does network manager configure my wifi access? It can't use the bridge device, as that doesn't have wifi extensions. It also can't use the wifi device, because setting up networking on that will break. IMHO the only customer friendly choice I see is the ugly way. The way VMware and Vbox do it. To make it a bit less ugly, maybe we could create some way to "glue" a tap device and an eth together, the same way the bridge code does, just without the extra device. I don't think that helps either. At the end of the day, this is really a policy decision. You want to expose the subset of functionality that the majority of your users require. Inevitably, you'll leave some users who need more complex setups on their own. For instance, users with one NIC are certainly common. Two or more NICs are also pretty common. But what about people that require guests to be on separate VLANs? This is where management tools come in. When running qemu by hand, the management tool is the distro more or less as they will be choosing the default policies. What we need to think about is how to make sure we can seamlessly integrate with whatever policies they want to implement. I think the one thing we could add is a configurable message to print when a user tries to use a bridge that isn't configured yet. For instance, if a user runs qemu without a bridge setup, what would be nice is the following: $ qemu -net bridge -net nic -hda ~/images/linux.img qemu: error running bridge helper You currently do not have 'qemubr0' configured. To setup qemubr0, run (as root): zypper install qemu-network-setup qemu-network-setup qemubr0 $ sudo qemu-network-setup Which interface do you want to configure [qemubr0]: qemubr0 Do you want to configure qemubr0 as NAT or Bridge [NAT]: Bridge Which physical interface do you want to bridge qemubr0 to [eth0]: eth0 qemubr0 is now configured. I expect that this is going to be different for every distro. netcf my make this easier but for now, I think this is the most realistic approach. Regards, Anthony Liguori
RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper
Hello Anthony, > -Original Message- > From: Anthony Liguori [mailto:anth...@codemonkey.ws] > Sent: Wednesday, November 04, 2009 8:23 AM > To: Krumme, Chris > Cc: qemu-devel@nongnu.org; Mark McLoughlin; Arnd Bergmann; > Michael Tsirkin; Juan Quintela; Dustin Kirkland > Subject: Re: [Qemu-devel] [PATCH 2/4] Add access control > support toqemu-bridge-helper > > Krumme, Chris wrote: > > Hello Anthony, > > > > Cool patch series. > > > > Thanks. > > >> +cmd = ptr; > >> +arg = strchr(cmd, ' '); > >> +if (arg == NULL) { > >> +arg = strchr(cmd, '\t'); > >> +} > >> + > >> +if (arg == NULL) { > >> +fprintf(stderr, "Invalid config line:\n %s\n", line); > >> +fclose(f); > >> +errno = EINVAL; > >> +return -1; > >> +} > >> + > >> +*arg = 0; > >> > > > > No check is made for arg being in bounds. > > > > I don't get it. arg is either going to be NULL (no ' ' or > '\t' found in > the string) or it will point to the first ' ' or '\t' in the > string. It My concern is that the first space or tab may not be in the first sizeof(line) characters. Thanks Chris > will always be in bound in this second case and the first case is > handled by the if(). > > Regards, > > Anthony Liguori >
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
Amit Shah wrote: > On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote: >> Amit Shah wrote: >>> On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote: Amit Shah wrote: > On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: >> On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: >>> Amit Shah wrote: The initial_reset sent to chardevs doesn't do much other than setting a bool to true. Char devices are interested in the open event and that gets sent whenever the device is opened. >>> Have you checked with the monitor in all use cases (dedicated & muxed >>> console, stdio & SDL console, etc.)? It was introduced once to fix a >> I've checked with -monitor stdio, monitor in SDL and also chardevs using >> unix sockets. >> >> I've not tried mux yet; I'll try that and report back. BTW if it ends up >> not working with this patch, it'd be broken in the current master as >> well. > I tried with: > > -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux > > The monitor prompt shows up as does the serial output. > > (btw I've also tried closing and opening char devs and that works fine > too) That sounds good. Then something must have changed since 2970a6c943, do you see what? >>> I think that depended on the resets being sent. I've now removed the >>> need for resets altogether. >> No, this is in fact the reason why we no longer need it: >> >> 9a1e948129 (Introduce contexts for asynchronous callbacks) >> >> As the initial reset of the char device that is marked pending on open >> is now no longer consumed by the IDE initialization, we can actually >> drop the later regeneration via qemu_chr_initial_reset. I just hope this >> stays like it is... > > I tested this even on a tree that doesn't have this patch. The whole things was (and maybe still is) fragile. So you have to take care what further patches with side effects you include. If you revert 9a1e948129 and remove qemu_chr_initial_reset, you get the old bug back again, ie. no prompt. > > I haven't really delved deep to see why this was added earlier -- the > commit log only says very little. Plus my testing with the current tree > works fine so I'm happy to mention these things in the commit log. I've nothing against your patch, specifically as it removes an obviously no longer needed workaround, not a feature. I just want to make sure that the current behavior is not only there by chance. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 4/4] Add support for -net bridge
Krumme, Chris wrote: Do you need to mention the default name qemubr0 here? Good suggestion. Regards, Anthony Liguori
Re: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper
Krumme, Chris wrote: Hello Anthony, Cool patch series. Thanks. +cmd = ptr; +arg = strchr(cmd, ' '); +if (arg == NULL) { +arg = strchr(cmd, '\t'); +} + +if (arg == NULL) { +fprintf(stderr, "Invalid config line:\n %s\n", line); +fclose(f); +errno = EINVAL; +return -1; +} + +*arg = 0; No check is made for arg being in bounds. I don't get it. arg is either going to be NULL (no ' ' or '\t' found in the string) or it will point to the first ' ' or '\t' in the string. It will always be in bound in this second case and the first case is handled by the if(). Regards, Anthony Liguori
Re: [Qemu-devel] Re: [PATCH] Add VirtIO Frame Buffer Support
On Tue, Nov 03, 2009 at 11:38:18AM +0200, Avi Kivity wrote: > On 11/03/2009 01:25 PM, Vincent Hanquez wrote: >> not sure if i'm missing the point here, but couldn't it be hypothetically >> extended to stuff 3d (or video& more 2d accel ?) commands too ? I can't >> imagine the cirrus or stdvga driver be able to do that ever ;) >> > > cirrus has pretty good 2d acceleration. 3D is a mega-project though. absolutely huge indeed, but still alexander's code is pretty much the only way, to start such a project. with maybe added benefits on more and easier 2d acceleration. or otherwise wait for SR-IOV graphics cards (or similar tech)... -- Vincent
RE: [Qemu-devel] [PATCH 4/4] Add support for -net bridge
Hello Anthony, Now that I have read the whole series I say again great patch. > -Original Message- > From: > qemu-devel-bounces+chris.krumme=windriver@nongnu.org > [mailto:qemu-devel-bounces+chris.krumme=windriver@nongnu.o > rg] On Behalf Of Anthony Liguori > Sent: Tuesday, November 03, 2009 6:28 PM > To: qemu-devel@nongnu.org > Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin > Kirkland; Michael Tsirkin; Juan Quintela > Subject: [Qemu-devel] [PATCH 4/4] Add support for -net bridge > > The most common use of -net tap is to connect a tap device to > a bridge. This > requires the use of a script and running qemu as root in > order to allocate a > tap device to pass to the script. > > This model is great for portability and flexibility but it's > incredibly > difficult to eliminate the need to run qemu as root. The > only really viable > mechanism is to use tunctl to create a tap device, attach it > to a bridge as > root, and then hand that tap device to qemu. The problem > with this mechanism > is that it requires administrator intervention whenever a > user wants to create > a guest. > > By essentially writing a helper that implements the most > common qemu-ifup > script that can be safely given cap_net_admin, we can > dramatically simplify > things for non-privileged users. We still support -net tap > as a mechanism > for advanced users and backwards compatibility. > > Currently, this is very Linux centric but there's really no > reason why it > couldn't be extended for other Unixes. > > A typical invocation of -net bridge would be: > > qemu -net bridge -net nic,model=virtio > > The default bridge that we attach to is qemubr0. The > thinking is that a distro > could preconfigure such an interface to allow out-of-the-box > bridged networking. > > Alternatively, if a user wants to use a different bridge, > they can say: > > qemu -net bridge,br=br0 -net nic,model=virtio > > Signed-off-by: Anthony Liguori > --- > configure |1 + > net.c | 20 +++- > net/tap.c | 142 > +++ > net/tap.h |2 + > qemu-options.hx |3 + > 5 files changed, 167 insertions(+), 1 deletions(-) > > diff --git a/configure b/configure > index 7c9d3a2..55a1a4f 100755 > --- a/configure > +++ b/configure > @@ -1896,6 +1896,7 @@ echo >> $config_host_mak > > echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> > $config_host_mak > echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak > +echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak > > case "$cpu" in > > i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p > pc|ppc64|s390|sparc|sparc64) > diff --git a/net.c b/net.c > index 37662c6..54a7a5b 100644 > --- a/net.c > +++ b/net.c > @@ -2541,6 +2541,22 @@ static struct { > }, > { /* end of list */ } > }, > +}, { > +.type = "bridge", > +.init = net_init_bridge, > +.desc = { > +NET_COMMON_PARAMS_DESC, > +{ > +.name = "br", > +.type = QEMU_OPT_STRING, > +.help = "bridge name", > +}, { > +.name = "helper", > +.type = QEMU_OPT_STRING, > +.help = "command to execute to configure bridge", > +}, > +{ /* end of list */ } > +}, > }, > { /* end of list */ } > }; > @@ -2565,7 +2581,8 @@ int net_client_init(Monitor *mon, > QemuOpts *opts, int is_netdev) > #ifdef CONFIG_VDE > strcmp(type, "vde") != 0 && > #endif > -strcmp(type, "socket") != 0) { > +strcmp(type, "socket") != 0 && > +strcmp(type, "bridge") != 0) { > qemu_error("The '%s' network backend type is not > valid with -netdev\n", > type); > return -1; > @@ -2641,6 +2658,7 @@ static int net_host_check_device(const > char *device) > #ifdef CONFIG_VDE > ,"vde" > #endif > + , "bridge" > }; > for (i = 0; i < sizeof(valid_param_list) / sizeof(char *); i++) { > if (!strncmp(valid_param_list[i], device, > diff --git a/net/tap.c b/net/tap.c > index bdb4a15..f5abed6 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -436,3 +436,145 @@ int net_init_tap(QemuOpts *opts, > Monitor *mon, const char *name, VLANState *vlan > > return 0; > } > + > +#define DEFAULT_BRIDGE_INTERFACE "qemubr0" > +#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR > "/qemu-bridge-helper" > + > +static int recv_fd(int c) > +{ > +int fd; > +uint8_t msgbuf[CMSG_SPACE(sizeof(fd))]; > +struct msghdr msg = { > +.msg_control = msgbuf, > +.msg_controllen = sizeof(msgbuf), > +}; > +struct cmsghdr *cmsg; > +struct iovec iov; > +uint8_t req[1]; > +ssize_
RE: [Qemu-devel] [PATCH 2/4] Add access control support toqemu-bridge-helper
Hello Anthony, Cool patch series. > -Original Message- > From: > qemu-devel-bounces+chris.krumme=windriver@nongnu.org > [mailto:qemu-devel-bounces+chris.krumme=windriver@nongnu.o > rg] On Behalf Of Anthony Liguori > Sent: Tuesday, November 03, 2009 6:28 PM > To: qemu-devel@nongnu.org > Cc: Mark McLoughlin; Anthony Liguori; Arnd Bergmann; Dustin > Kirkland; Michael Tsirkin; Juan Quintela > Subject: [Qemu-devel] [PATCH 2/4] Add access control support > toqemu-bridge-helper > > We go to great lengths to restrict ourselves to just > cap_net_admin as an OS > enforced security mechanism. However, we further restrict > what we allow users > to do to simply adding a tap device to a bridge interface by > virtue of the fact > that this is the only functionality we expose. > > This is not good enough though. An administrator is likely > to want to restrict > the bridges that an unprivileged user can access, in > particular, to restrict > an unprivileged user from putting a guest on what should be > isolated networks. > > This patch implements a ACL mechanism that is enforced by > qemu-bridge-helper. > The ACLs are fairly simple whitelist/blacklist mechanisms > with a wildcard of > 'all'. > > An interesting feature of this ACL mechanism is that you can > include external > ACL files. The main reason to support this is so that you > can set different > file system permissions on those external ACL files. This allows an > administrator to implement rather sophisicated ACL policies > based on user/group > policies via the file system. > > If we fail to include an acl file, we are silent about it > making this mechanism > work pretty seamlessly. As an example: > > /etc/qemu/bridge.conf root:qemu 0640 > > deny all > allow br0 > include /etc/qemu/alice.conf > include /etc/qemu/bob.conf > > /etc/qemu/alice.conf root:alice 0640 > allow br1 > > /etc/qemu/bob.conf root:bob 0640 > allow br2 > > This ACL pattern allows any user in the qemu group to get a tap device > connected to br0 (which is bridged to the physical network). > > Users in the alice group can additionally get a tap device > connected to br1. > This allows br1 to act as a private bridge for the alice group. > > Users in the bob group can additionally get a tap device > connected to br2. > This allows br2 to act as a private bridge for the bob group. > > Under no circumstance can the bob group get access to br1 or > can the alice > group get access to br2. > > Signed-off-by: Anthony Liguori > --- > configure|1 + > qemu-bridge-helper.c | 138 > ++ > 2 files changed, 139 insertions(+), 0 deletions(-) > > diff --git a/configure b/configure > index a341e77..7c98257 100755 > --- a/configure > +++ b/configure > @@ -1864,6 +1864,7 @@ printf " '%s'" "$0" "$@" >> $config_host_mak > echo >> $config_host_mak > > echo "CONFIG_QEMU_SHAREDIR=\"$prefix$datasuffix\"" >> > $config_host_mak > +echo "CONFIG_QEMU_CONFDIR=\"/etc/qemu\"" >> $config_host_mak > > case "$cpu" in > > i386|x86_64|alpha|cris|hppa|ia64|m68k|microblaze|mips|mips64|p > pc|ppc64|s390|sparc|sparc64) > diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c > index f10d37c..0d059ed 100644 > --- a/qemu-bridge-helper.c > +++ b/qemu-bridge-helper.c > @@ -33,6 +33,106 @@ > > #include "net/tap-linux.h" > > +#define MAX_ACLS (128) > +#define DEFAULT_ACL_FILE CONFIG_QEMU_CONFDIR "/bridge.conf" > + > +enum { > +ACL_ALLOW = 0, > +ACL_ALLOW_ALL, > +ACL_DENY, > +ACL_DENY_ALL, > +}; > + > +typedef struct ACLRule > +{ > +int type; > +char iface[IFNAMSIZ]; > +} ACLRule; > + > +static int parse_acl_file(const char *filename, ACLRule > *acls, int *pacl_count) > +{ > +int acl_count = *pacl_count; > +FILE *f; > +char line[4096]; > + > +f = fopen(filename, "r"); > +if (f == NULL) { > +return -1; > +} > + > +while (acl_count != MAX_ACLS && > + fgets(line, sizeof(line), f) != NULL) { > +char *ptr = line; > +char *cmd, *arg, *argend; > + > +while (isspace(*ptr)) { > +ptr++; > +} > + > +/* skip comments and empty lines */ > +if (*ptr == '#' || *ptr == 0) { > +continue; > +} > + > +cmd = ptr; > +arg = strchr(cmd, ' '); > +if (arg == NULL) { > +arg = strchr(cmd, '\t'); > +} > + > +if (arg == NULL) { > +fprintf(stderr, "Invalid config line:\n %s\n", line); > +fclose(f); > +errno = EINVAL; > +return -1; > +} > + > +*arg = 0; No check is made for arg being in bounds. Thanks Chris > +arg++; > +while (isspace(*arg)) { > +arg++; > +} > + > +argend = arg + strlen(arg); > +while (arg != argend && isspace(*(argend - 1))) { > +argend--; > +} > +*argend =
[Qemu-devel][PATCH v2] Added read access to several e1000 hw registers
The e1000 hardware allows read access to several registers which qemu doesn't allow yet. Signed-off-by: Kay Ackermann --- hw/e1000.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index 028afd1..3987e70 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -789,6 +789,8 @@ static uint32_t (*macreg_readops[])(E1000State *, int) = { getreg(MANC), getreg(MDIC), getreg(SWSM), getreg(STATUS), getreg(TORL), getreg(TOTL), getreg(IMS), getreg(TCTL), getreg(RDH), getreg(RDT), getreg(VET), getreg(ICS), + getreg(TDBAL), getreg(TDBAH), getreg(RDBAH), getreg(RDBAL), + getreg(TDLEN), getreg(RDLEN), [TOTH] = mac_read_clr8, [TORH] = mac_read_clr8, [GPRC] = mac_read_clr4, [GPTC] = mac_read_clr4, [TPR] = mac_read_clr4, [TPT] = mac_read_clr4, -- 1.6.4.4
Re: [Qemu-devel] Re: [PATCH V6 17/32] pci: 64bit bar support.
On Wed, Nov 04, 2009 at 03:20:02PM +0900, Isaku Yamahata wrote: > On Tue, Nov 03, 2009 at 04:09:16PM +0200, Michael S. Tsirkin wrote: > > > > Long term, we should fix all devices and *then* they can claim 64 bit > > > > support always. As a nice side effect, we'll be able to avoid > > > > rebuilding devices. > > > > > > Are you claiming that (PCI) devices emulation shouldn't depend on > > > target_phys_addr_t? That sounds a good idea. > > > > Yes. Maybe we can stop devices from mapping memory, have pci > > core do it for them. > > IIRC Avi proposed such a patch, link? > but it didn't get merged. I wonder why - currently mapping is done by devices but unmapping is done by pci core. That's been always bothering me. > -- > yamahata
Re: [Qemu-devel] [PATCH 0/4] net-bridge: rootless bridge support for qemu
On 04.11.2009, at 01:28, Anthony Liguori wrote: This series solves a problem that I've been struggling with for a few years now. One of the best things about qemu is that it's possible to run guests as an unprivileged user to improve security. However, if you want to have your guests communicate with the outside world, you're pretty much forced to run qemu as root. At least with KVM support, this is probably the most common use case which means that most of our users are running qemu as root. That's terrible. Yeah. Worse than the "run as root" part is the "it's hard" part though. I hate how I feel when I try to explain someone how to use non- slirp networking :-(. The response to that is then usually "oh whatever, it's too complicated anyways". We address this problem by introducing a new network backend: -net bridge. This backend is less flexible than -net tap because it relies on a helper with elevated privileges to do the heavy lifting of allocating and attaching a tap device to a bridge. We use a special purpose helper because we don't want to elevate the privileges of more generic tools like brctl. From a user perspective, to use bridged networking with a guest, you simply use: qemu -hda linux.img -net bridge -net nic And assuming a bridge is defined named qemubr0 and the administrator has setup permissions accordingly, it will Just Work. My hope is that distributions will do this work as part of the qemu packaging process such that for most users, the out-of-the-box experience will also Just Work. Yeah, that won't work as easily. When your customer has 2 NICs this will already break. But let's imagine we only have a single NIC. So that NIC is a wifi card. When I set up the qemubr0 bridge for that one now, how does network manager configure my wifi access? It can't use the bridge device, as that doesn't have wifi extensions. It also can't use the wifi device, because setting up networking on that will break. IMHO the only customer friendly choice I see is the ugly way. The way VMware and Vbox do it. To make it a bit less ugly, maybe we could create some way to "glue" a tap device and an eth together, the same way the bridge code does, just without the extra device. Alex
[Qemu-devel] Re: [PATCH V6 13/32] pci_host: consolidate pci config address access.
On 04.11.2009, at 07:14, Isaku Yamahata wrote: On Tue, Nov 03, 2009 at 03:45:12PM +0200, Michael S. Tsirkin wrote: --- a/hw/pci_host.c +++ b/hw/pci_host.c @@ -32,6 +32,114 @@ do { printf("pci_host_data: " fmt , ## __VA_ARGS__); } while (0) #define PCI_DPRINTF(fmt, ...) #endif +static void pci_host_config_writel(void *opaque, target_phys_addr_t addr, + uint32_t val) +{ +PCIHostState *s = opaque; + +#ifdef TARGET_WORDS_BIGENDIAN +val = bswap32(val); +#endif I know you just copied it, but isn't this just val = le32_to_cpu(val); ? Makes sense. +static void pci_host_config_writel_noswap(void *opaque, + target_phys_addr_t addr, + uint32_t val) +{ +PCIHostState *s = opaque; + +PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n", +__func__, addr, val); +s->config_reg = val; +} + +static uint32_t pci_host_config_readl_noswap(void *opaque, + target_phys_addr_t addr) +{ +PCIHostState *s = opaque; +uint32_t val = s->config_reg; + +PCI_DPRINTF("%s addr " TARGET_FMT_plx " val %"PRIx32"\n", +__func__, addr, val); +return val; +} + +static CPUWriteMemoryFunc * const pci_host_config_write_noswap[] = { +&pci_host_config_writel_noswap, +&pci_host_config_writel_noswap, +&pci_host_config_writel_noswap, +}; + +static CPUReadMemoryFunc * const pci_host_config_read_noswap[] = { +&pci_host_config_readl_noswap, +&pci_host_config_readl_noswap, +&pci_host_config_readl_noswap, +}; + +int pci_host_config_register_io_memory_noswap(PCIHostState *s) This name is clearly too long, as a result when you use it you run over the 80 character limit. Let's not fix it, let's make name shorter. io_memory -> memory? +{ +return cpu_register_io_memory(pci_host_config_read_noswap, + pci_host_config_write_noswap, s); +} Do you happen to know whether no swap is really needed? I suspect some of these places really only happen to work because everyone uses intel, so they simply would not work on ppc host. I just followed the original code not to break it. I dug into the commit logs a bit. Liu, Aurelian and Alexander, could you give me a comment on whether those functions needs byte swap (le32_to_cpu()) or not. - ppcie500_cfgaddr_{write, read}l() in ppce500_pci.c - pci_unin_config_{write, read}l() in unin_pci.c ppce500_pci.c case: The related commit is 74c62ba88902575be9ac3badf95d773470884b1c. The comment on the top in the file says that it is derived from ppc4xx_pci.c. ppc4xx_pci.c has byte swap, on the other hand ppce500_pci.c doesn't. So I'm inclined to suspect that the byte swap was dropped accidently. However I don't know its pci host bridge spec. unin_pci.c case: I don't know its spec neither. The following changeset seems to remove the byte swap deliberately. Alexander, could you please give us comment on it? Phew - I frankly don't remember. Something broke because some bits were strangely mangled and the way it's not it worked more than it did before :-). The whole Uninorth thing is still utterly broken, so please don't take anything from there as example. I do know that Hollis was working a lot about LE/BE issues on PPC in Qemu, so he's probably the best person to CC and ask here. Alex
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
On (Wed) Nov 04 2009 [10:39:39], Jan Kiszka wrote: > Amit Shah wrote: > > On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote: > >> Amit Shah wrote: > >>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: > On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: > > Amit Shah wrote: > >> The initial_reset sent to chardevs doesn't do much other than setting > >> a bool to true. Char devices are interested in the open event and > >> that gets sent whenever the device is opened. > > Have you checked with the monitor in all use cases (dedicated & muxed > > console, stdio & SDL console, etc.)? It was introduced once to fix a > I've checked with -monitor stdio, monitor in SDL and also chardevs using > unix sockets. > > I've not tried mux yet; I'll try that and report back. BTW if it ends up > not working with this patch, it'd be broken in the current master as > well. > >>> I tried with: > >>> > >>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux > >>> > >>> The monitor prompt shows up as does the serial output. > >>> > >>> (btw I've also tried closing and opening char devs and that works fine > >>> too) > >> That sounds good. Then something must have changed since 2970a6c943, do > >> you see what? > > > > I think that depended on the resets being sent. I've now removed the > > need for resets altogether. > > No, this is in fact the reason why we no longer need it: > > 9a1e948129 (Introduce contexts for asynchronous callbacks) > > As the initial reset of the char device that is marked pending on open > is now no longer consumed by the IDE initialization, we can actually > drop the later regeneration via qemu_chr_initial_reset. I just hope this > stays like it is... I tested this even on a tree that doesn't have this patch. I haven't really delved deep to see why this was added earlier -- the commit log only says very little. Plus my testing with the current tree works fine so I'm happy to mention these things in the commit log. Amit
[Qemu-devel] Re: [PATCH 2/3] char: Remove special init_reset handling
Amit Shah wrote: > On (Tue) Nov 03 2009 [19:53:43], Jan Kiszka wrote: >> Amit Shah wrote: >>> On (Tue) Nov 03 2009 [23:25:52], Amit Shah wrote: On (Tue) Nov 03 2009 [18:08:57], Jan Kiszka wrote: > Amit Shah wrote: >> The initial_reset sent to chardevs doesn't do much other than setting >> a bool to true. Char devices are interested in the open event and >> that gets sent whenever the device is opened. > Have you checked with the monitor in all use cases (dedicated & muxed > console, stdio & SDL console, etc.)? It was introduced once to fix a I've checked with -monitor stdio, monitor in SDL and also chardevs using unix sockets. I've not tried mux yet; I'll try that and report back. BTW if it ends up not working with this patch, it'd be broken in the current master as well. >>> I tried with: >>> >>> -chardev stdio,mux=on,id=mux -monitor chardev:mux -serial chardev:mux >>> >>> The monitor prompt shows up as does the serial output. >>> >>> (btw I've also tried closing and opening char devs and that works fine >>> too) >> That sounds good. Then something must have changed since 2970a6c943, do >> you see what? > > I think that depended on the resets being sent. I've now removed the > need for resets altogether. No, this is in fact the reason why we no longer need it: 9a1e948129 (Introduce contexts for asynchronous callbacks) As the initial reset of the char device that is marked pending on open is now no longer consumed by the IDE initialization, we can actually drop the later regeneration via qemu_chr_initial_reset. I just hope this stays like it is... Jan signature.asc Description: OpenPGP digital signature