[Qemu-devel] [PATCH] fix MinGW compilation when --enable-vnc-jpeg is specified
This patch fix conflicting types for 'INT32' in basetsd.h in including qemu-common.h first. Sign-off-by: Roy Tam -- diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 87fdf35..1591df0 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -28,6 +28,8 @@ #include "config-host.h" +#include "qemu-common.h" + #ifdef CONFIG_VNC_PNG #include #endif @@ -36,8 +38,6 @@ #include #endif -#include "qemu-common.h" - #include "bswap.h" #include "qint.h" #include "vnc.h"
[Qemu-devel] [Bug 799036] [NEW] NIC assignment order in command line make some NIC can't work
Public bug reported: Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):Linux kvm.git Commit:681fb677ace0754589792c92b8dbee9884d07158 qemu-kvm Commit:05f1737582ab6c075476bde931c5eafbc62a9349 Host Kernel Version:3.0.0-rc2+ Hardware: Westmere-EP platform Bug detailed description: -- When using qemu-system-x86_64 to create a linux guest with a 82586 NIC and a 82572EI NIC statically assigned, the two NIC's order in command line may make 82586 NIC can't work in guest. command1: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0e:00.1 -device pci-assign,host=0c:00.0 -net none -hda qcow-rhel6.img command2: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0c:00.0 -device pci-assign,host=0e:00.1 -net none -hda qcow-rhel6.img Using command1 to create a guest, both two NICs works well in guest. While using command2 to create a guest, 82576(kawela) NIC 0c:00.0 cannot get IP in guest. BDF 0c:00.0 is a 82576(kawela) NIC, while BDF 0e:00.1 is 82572EI NIC. The only difference of the two command lines is the order of NICs assignment. And only the 82576(kawela) NIC have this problem, the other one 82572EI always works well. And sriov VF doesn't have this issue. A VF and a 82576 PF assigned to a guest always work well. I don't know the order of NIC assignment in qemu-system-x86_64 will make this difference in guest. Maybe it's a qemu bug ? Reproduce steps: 1.pci-stub the two NIC 2.create a guest: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0c:00.0 -device pci-assign,host=0e:00.1 -net none -hda qcow-rhel6.img (0c:00.0 is a 82576 NIC, and 0e:00.1 is another NIC). Current result: 82576 PF and the other NIC in guest will work well regardless of what the order of assignment is. Expected result: ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/799036 Title: NIC assignment order in command line make some NIC can't work Status in QEMU: New Bug description: Environment: Host OS (ia32/ia32e/IA64):All Guest OS (ia32/ia32e/IA64):ia32e Guest OS Type (Linux/Windows):Linux kvm.git Commit:681fb677ace0754589792c92b8dbee9884d07158 qemu-kvm Commit:05f1737582ab6c075476bde931c5eafbc62a9349 Host Kernel Version:3.0.0-rc2+ Hardware: Westmere-EP platform Bug detailed description: -- When using qemu-system-x86_64 to create a linux guest with a 82586 NIC and a 82572EI NIC statically assigned, the two NIC's order in command line may make 82586 NIC can't work in guest. command1: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0e:00.1 -device pci-assign,host=0c:00.0 -net none -hda qcow-rhel6.img command2: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0c:00.0 -device pci-assign,host=0e:00.1 -net none -hda qcow-rhel6.img Using command1 to create a guest, both two NICs works well in guest. While using command2 to create a guest, 82576(kawela) NIC 0c:00.0 cannot get IP in guest. BDF 0c:00.0 is a 82576(kawela) NIC, while BDF 0e:00.1 is 82572EI NIC. The only difference of the two command lines is the order of NICs assignment. And only the 82576(kawela) NIC have this problem, the other one 82572EI always works well. And sriov VF doesn't have this issue. A VF and a 82576 PF assigned to a guest always work well. I don't know the order of NIC assignment in qemu-system-x86_64 will make this difference in guest. Maybe it's a qemu bug ? Reproduce steps: 1.pci-stub the two NIC 2.create a guest: qemu-system-x86_64 -m 1024 -smp 2 -device pci-assign,host=0c:00.0 -device pci-assign,host=0e:00.1 -net none -hda qcow-rhel6.img (0c:00.0 is a 82576 NIC, and 0e:00.1 is another NIC). Current result: 82576 PF and the other NIC in guest will work well regardless of what the order of assignment is. Expected result: To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/799036/+subscriptions
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On (Fri) 17 Jun 2011 [15:08:11], Luiz Capitulino wrote: > > > if (!cpkt.value) { > > > -error_report("virtio-serial-bus: Guest failure in adding > > > device %s\n", > > > - vser->bus.qbus.name); > > > -break; > > > +error_report("virtio-serial-bus: Guest failure in adding > > > device %s\n", vser->bus.qbus.name); > > > +return; > > > > The line split should remain -- else it goes beyond 80 chars. > > It's already beyond 80 chars to me. I prefer to not break strings that get printed out -- makes it easier for greppers to find out the source of the string. > > > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial > > > *vser, void *buf, size_t len) > > > QTAILQ_FOREACH(port, &vser->ports, next) { > > > send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); > > > } > > > -break; > > > +return; > > > +} > > > > Makes me think of one case (totally unrelated to what you found)where > > the guest can fool us: by sending multiple VIRTIO_CONSOLE_DEVICE_READY > > messages. > > It will be handled just fine, no? We'll send out the VIRTIO_CONSOLE_PORT_ADD events for each port (again). That's the case now. No idea how the code might change in the future and we could end up doing something else in addition which might be bad. Anyway, all this is for a buggy or a bad guest. Amit
Re: [Qemu-devel] [PATCH v5 2/5] guest agent: qemu-ga daemon
On Fri, 17 Jun 2011 16:25:32 -0500 Michael Roth wrote: > On 06/17/2011 03:13 PM, Luiz Capitulino wrote: > > On Fri, 17 Jun 2011 14:21:31 -0500 > > Michael Roth wrote: > > > >> On 06/16/2011 01:42 PM, Luiz Capitulino wrote: > >>> On Tue, 14 Jun 2011 15:06:22 -0500 > >>> Michael Roth wrote: > >>> > This is the actual guest daemon, it listens for requests over a > virtio-serial/isa-serial/unix socket channel and routes them through > to dispatch routines, and writes the results back to the channel in > a manner similar to QMP. > > A shorthand invocation: > > qemu-ga -d > > Is equivalent to: > > qemu-ga -c virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent \ > -p /var/run/qemu-guest-agent.pid -d > > Signed-off-by: Michael Roth > >>> > >>> Would be nice to have a more complete description, like explaining how to > >>> do a simple test. > >>> > >>> And this can't be built... > >>> > --- > qemu-ga.c | 631 > > qga/guest-agent-core.h |4 + > 2 files changed, 635 insertions(+), 0 deletions(-) > create mode 100644 qemu-ga.c > > diff --git a/qemu-ga.c b/qemu-ga.c > new file mode 100644 > index 000..df08d8c > --- /dev/null > +++ b/qemu-ga.c > @@ -0,0 +1,631 @@ > +/* > + * QEMU Guest Agent > + * > + * Copyright IBM Corp. 2011 > + * > + * Authors: > + * Adam Litke > + * Michael Roth > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > + * See the COPYING file in the top-level directory. > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "qemu_socket.h" > +#include "json-streamer.h" > +#include "json-parser.h" > +#include "qint.h" > +#include "qjson.h" > +#include "qga/guest-agent-core.h" > +#include "qga-qmp-commands.h" > +#include "module.h" > + > +#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent" > +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid" > +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ > +#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */ > + > +struct GAState { > +const char *proxy_path; > >>> > >>> Where is this used? > >>> > >> > >> Nowhere actually. Will remove. > >> > +JSONMessageParser parser; > +GMainLoop *main_loop; > +guint conn_id; > +GSocket *conn_sock; > +GIOChannel *conn_channel; > +guint listen_id; > +GSocket *listen_sock; > +GIOChannel *listen_channel; > +const char *path; > +const char *method; > +bool virtio; /* fastpath to check for virtio to deal with poll() > quirks */ > +GACommandState *command_state; > +GLogLevelFlags log_level; > +FILE *log_file; > +bool logging_enabled; > +}; > + > +static void usage(const char *cmd) > +{ > +printf( > +"Usage: %s -c\n" > +"QEMU Guest Agent %s\n" > +"\n" > +" -c, --channel channel method: one of unix-connect, > virtio-serial, or\n" > +"isa-serial (virtio-serial is the default)\n" > +" -p, --pathchannel path (%s is the default for > virtio-serial)\n" > +" -l, --logfile set logfile path, logs to stderr by default\n" > +" -f, --pidfile specify pidfile (default is %s)\n" > +" -v, --verbose log extra debugging information\n" > +" -V, --version print version information and exit\n" > +" -d, --daemonize become a daemon\n" > +" -h, --helpdisplay this help and exit\n" > +"\n" > +"Report bugs to\n" > +, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT); > +} > + > +static void conn_channel_close(GAState *s); > + > +static const char *ga_log_level_str(GLogLevelFlags level) > +{ > +switch (level& G_LOG_LEVEL_MASK) { > +case G_LOG_LEVEL_ERROR: > +return "error"; > +case G_LOG_LEVEL_CRITICAL: > +return "critical"; > +case G_LOG_LEVEL_WARNING: > +return "warning"; > +case G_LOG_LEVEL_MESSAGE: > +return "message"; > +case G_LOG_LEVEL_INFO: > +return "info"; > +case G_LOG_LEVEL_DEBUG: > +return "debug"; > +default: > +return "user"; > +} > +} > + > +bool ga_logging_enabled(GAState *s) > +{ > +return s->logging_enabled; > +} > + > +void ga_disable_logging(GAState *
Re: [Qemu-devel] [PATCH v5 3/5] guest agent: add guest agent RPCs/commands
On Fri, 17 Jun 2011 15:19:56 -0500 Michael Roth wrote: > On 06/16/2011 01:52 PM, Luiz Capitulino wrote: > > On Tue, 14 Jun 2011 15:06:23 -0500 > > Michael Roth wrote: > > > >> This adds the initial set of QMP/QAPI commands provided by the guest > >> agent: > >> > >> guest-sync > >> guest-ping > >> guest-info > >> guest-shutdown > >> guest-file-open > >> guest-file-read > >> guest-file-write > >> guest-file-seek > >> guest-file-close > >> guest-fsfreeze-freeze > >> guest-fsfreeze-thaw > >> guest-fsfreeze-status > >> > >> The input/output specification for these commands are documented in the > >> schema. > >> > >> Signed-off-by: Michael Roth > >> --- > >> qerror.c |4 + > >> qerror.h |3 + > >> qga/guest-agent-commands.c | 522 > >> > >> qga/guest-agent-core.h |1 + > >> 4 files changed, 530 insertions(+), 0 deletions(-) > >> create mode 100644 qga/guest-agent-commands.c > >> > >> diff --git a/qerror.c b/qerror.c > >> index d7fcd93..24f0c48 100644 > >> --- a/qerror.c > >> +++ b/qerror.c > >> @@ -213,6 +213,10 @@ static const QErrorStringTable qerror_table[] = { > >> .error_fmt = QERR_VNC_SERVER_FAILED, > >> .desc = "Could not start VNC server on %(target)", > >> }, > >> +{ > >> +.error_fmt = QERR_QGA_LOGGING_FAILED, > >> +.desc = "failed to write log statement due to logging being > >> disabled", > >> +}, > >> {} > >> }; > >> > >> diff --git a/qerror.h b/qerror.h > >> index 7a89a50..bf3d5a9 100644 > >> --- a/qerror.h > >> +++ b/qerror.h > >> @@ -184,4 +184,7 @@ QError *qobject_to_qerror(const QObject *obj); > >> #define QERR_FEATURE_DISABLED \ > >> "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }" > >> > >> +#define QERR_QGA_LOGGING_FAILED \ > >> +"{ 'class': 'QgaLoggingFailed', 'data': {} }" > >> + > >> #endif /* QERROR_H */ > >> diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c > >> new file mode 100644 > >> index 000..6f9886a > >> --- /dev/null > >> +++ b/qga/guest-agent-commands.c > >> @@ -0,0 +1,522 @@ > >> +/* > >> + * QEMU Guest Agent commands > >> + * > >> + * Copyright IBM Corp. 2011 > >> + * > >> + * Authors: > >> + * Michael Roth > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or > >> later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> + > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include "qga/guest-agent-core.h" > >> +#include "qga-qmp-commands.h" > >> +#include "qerror.h" > >> + > >> +static GAState *ga_state; > >> + > >> +static bool logging_enabled(void) > >> +{ > >> +return ga_logging_enabled(ga_state); > >> +} > >> + > >> +static void disable_logging(void) > >> +{ > >> +ga_disable_logging(ga_state); > >> +} > >> + > >> +static void enable_logging(void) > >> +{ > >> +ga_enable_logging(ga_state); > >> +} > >> + > >> +/* Note: in some situations, like with the fsfreeze, logging may be > >> + * temporarilly disabled. if it is necessary that a command be able > >> + * to log for accounting purposes, check logging_enabled() beforehand, > >> + * and use the QERR_QGA_LOGGING_DISABLED to generate an error > >> + */ > >> +static void slog(const char *fmt, ...) > >> +{ > >> +va_list ap; > >> + > >> +va_start(ap, fmt); > >> +g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap); > >> +va_end(ap); > >> +} > >> + > >> +int64_t qmp_guest_sync(int64_t id, Error **errp) > >> +{ > >> +return id; > >> +} > >> + > >> +void qmp_guest_ping(Error **err) > >> +{ > >> +slog("guest-ping called"); > >> +} > >> + > >> +struct GuestAgentInfo *qmp_guest_info(Error **err) > >> +{ > >> +GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo)); > >> + > >> +info->version = g_strdup(QGA_VERSION); > >> + > >> +return info; > >> +} > >> + > >> +void qmp_guest_shutdown(const char *shutdown_mode, Error **err) > > > > I'd call the argument just 'mode'. > > > >> +{ > >> +int ret; > >> +const char *shutdown_flag; > >> + > >> +if (!logging_enabled()) { > >> +error_set(err, QERR_QGA_LOGGING_FAILED); > >> +return; > >> +} > >> + > >> +slog("guest-shutdown called, shutdown_mode: %s", shutdown_mode); > >> +if (strcmp(shutdown_mode, "halt") == 0) { > >> +shutdown_flag = "-H"; > >> +} else if (strcmp(shutdown_mode, "powerdown") == 0) { > >> +shutdown_flag = "-P"; > >> +} else if (strcmp(shutdown_mode, "reboot") == 0) { > >> +shutdown_flag = "-r"; > >> +} else { > >> +error_set(err, QERR_INVALID_PARAMETER_VALUE, "shutdown_mode", > >> + "halt|powerdown|reboot"); > >> +return; > >> +} > >> + > >> +ret = fork(); > >> +if (ret == 0) { > >> +/* child, start the shutdown */ > >> +setsid(); > >> +fclose(stdin); > >> +fclose(stdout); > >> +
Re: [Qemu-devel] [PATCH 3/3] ppc: booke206: add "info tlb" support
On 17.06.2011, at 22:39, Scott Wood wrote: > Signed-off-by: Scott Wood > --- > hmp-commands.hx |2 +- > monitor.c |5 ++- > target-ppc/cpu.h|2 + > target-ppc/helper.c | 89 +++ > 4 files changed, 95 insertions(+), 3 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 6ad8806..014a4fb 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1306,7 +1306,7 @@ show i8259 (PIC) state > @item info pci > show emulated PCI device info > @item info tlb > -show virtual to physical memory mappings (i386, SH4 and SPARC only) > +show virtual to physical memory mappings (i386, SH4, SPARC, and PPC only) > @item info mem > show the active virtual memory mappings (i386 only) > @item info jit > diff --git a/monitor.c b/monitor.c > index 6af6a4d..68e94af 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -2408,7 +2408,7 @@ static void tlb_info(Monitor *mon) > > #endif > > -#if defined(TARGET_SPARC) > +#if defined(TARGET_SPARC) || defined(TARGET_PPC) > static void tlb_info(Monitor *mon) > { > CPUState *env1 = mon_get_cpu(); > @@ -2901,7 +2901,8 @@ static const mon_cmd_t info_cmds[] = { > .user_print = do_pci_info_print, > .mhandler.info_new = do_pci_info, > }, > -#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) > +#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \ > +defined(TARGET_PPC) > { > .name = "tlb", > .args_type = "", > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index 5d80b1b..48f7e2c 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -2020,4 +2020,6 @@ static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState > *env, const int tlbn, > > extern void (*cpu_ppc_hypercall)(CPUState *); > > +void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env); > + > #endif /* !defined (__CPU_PPC_H__) */ > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index 5d007c5..e22b1cb 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c > @@ -1451,6 +1451,95 @@ found_tlb: > return ret; > } > > +static const char *book3e_tsize_to_str[32] = { > +"1K", "2K", "4K", "8K", "16K", "32K", "64K", "128K", "256K", "512K", > +"1M", "2M", "4M", "8M", "16M", "32M", "64M", "128M", "256M", "512M", > +"1G", "2G", "4G", "8G", "16G", "32G", "64G", "128G", "256G", "512G", > +"1T", "2T" > +}; > + > +static void mmubooke206_dump_one_tlb(FILE *f, fprintf_function cpu_fprintf, > + CPUState *env, int tlbn, int offset, > + int tlbsize) > +{ > +ppcmas_tlb_t *entry; > +int i; > + > +cpu_fprintf(f, "\nTLB%d:\n", tlbn); > +cpu_fprintf(f, "Effective Physical Size TID TS SRWX > URWX WIMGE U0123\n"); > + > +entry = &env->tlb.tlbm[offset]; > +for (i = 0; i < tlbsize; i++, entry++) { > +target_phys_addr_t ea, pa, size; > +int tsize; > + > +/* valid? */ > +if (!(entry->mas1 & MAS1_VALID)) { > +continue; > +} > + > +tsize = (entry->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; > +size = 1024ULL << tsize; Isn't that what booke206_tlb_to_page_size() is there for? Hrm - but you still need tsize. I see. Fair enough - I can't think of a cleaner way atm either. Alex
Re: [Qemu-devel] [PATCH 1/3] kvm: ppc: booke206: use MMU API
On 17.06.2011, at 22:39, Scott Wood wrote: > Share the TLB array with KVM. This allows us to set the initial TLB > both on initial boot and reset, is useful for debugging, and could > eventually be used to support migration. > > Signed-off-by: Scott Wood > --- > hw/ppce500_mpc8544ds.c |2 + > target-ppc/cpu.h |2 + > target-ppc/kvm.c | 85 > 3 files changed, 89 insertions(+), 0 deletions(-) > > diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c > index 5ac8843..3cdeb43 100644 > --- a/hw/ppce500_mpc8544ds.c > +++ b/hw/ppce500_mpc8544ds.c > @@ -192,6 +192,8 @@ static void mmubooke_create_initial_mapping(CPUState *env, > tlb->mas2 = va & TARGET_PAGE_MASK; > tlb->mas7_3 = pa & TARGET_PAGE_MASK; > tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX; > + > +env->tlb_dirty = true; > } > > static void mpc8544ds_cpu_reset(void *opaque) > diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h > index 46d86be..8191ed2 100644 > --- a/target-ppc/cpu.h > +++ b/target-ppc/cpu.h > @@ -921,6 +921,8 @@ struct CPUPPCState { > ppc_tlb_t tlb; /* TLB is optional. Allocate them only if needed > */ > /* 403 dedicated access protection registers */ > target_ulong pb[4]; > +bool tlb_dirty; /* Set to non-zero when modifying TLB > */ > +bool kvm_sw_tlb; /* non-zero if KVM SW TLB API is active > */ > #endif > > /* Other registers */ > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index e7b1b10..9a88fc9 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -122,6 +122,51 @@ static int kvm_arch_sync_sregs(CPUState *cenv) > return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs); > } > > +static int kvm_booke206_tlb_init(CPUState *env) > +{ > +#if defined(KVM_CAP_SW_TLB) && defined(KVM_MMU_FSL_BOOKE_NOHV) Those hopefully shouldn't be required anymore soon - when Jan's patches make it upstream. Jan, how's progress on that front? > +struct kvm_book3e_206_tlb_params params = {}; Hrm - I'm not familiar with that initialization. What exactly does it do? Set the struct contents to 0? Is this properly standardized? Usually, I see memset(0)s for that. > +struct kvm_config_tlb cfg = {}; > +size_t array_len; > +unsigned int entries = 0; > +int ret, i; > + > +if (!kvm_enabled() || > +!kvm_check_extension(env->kvm_state, KVM_CAP_SW_TLB)) { > +return 0; > +} > + > +for (i = 0; i < ARRAY_SIZE(params.tlb_sizes); i++) { Please make this MAX(..., BOOKE206_MAX_TLBN) - I'd hope the compiler is clever enough to figure out we're dealing with a constant here and that way the code looks more secure (even though it's the same in practice). Alternatively, you could just do an assert(... == BOOKE206_MAX_TLBN); before. > +params.tlb_sizes[i] = booke206_tlb_size(env, i); > +params.tlb_ways[i] = booke206_tlb_ways(env, i); > +entries += params.tlb_sizes[i]; > +} > + > +if (entries != env->nb_tlb) { > +cpu_abort(env, "%s: nb_tlb mismatch\n", __func__); > +} > + assert(sizeof(struct kvm_book3e_206_tlb_entry) == sizeof(ppcmas_tlb_t)); > +array_len = sizeof(struct kvm_book3e_206_tlb_entry) * entries; > +env->tlb_dirty = true; > + > +cfg.array = (uintptr_t)env->tlb.tlbm; > +cfg.array_len = sizeof(ppcmas_tlb_t) * entries; > +cfg.params = (uintptr_t)¶ms; > +cfg.mmu_type = KVM_MMU_FSL_BOOKE_NOHV; > + > +ret = kvm_vcpu_ioctl(env, KVM_CONFIG_TLB, &cfg); > +if (ret < 0) { > +fprintf(stderr, "%s: couldn't KVM_CONFIG_TLB: %s\n", > +__func__, strerror(-ret)); > +return ret; > +} > + > +env->kvm_sw_tlb = true; > +#endif > + > +return 0; > +} > + > int kvm_arch_init_vcpu(CPUState *cenv) > { > int ret; > @@ -133,6 +178,14 @@ int kvm_arch_init_vcpu(CPUState *cenv) > > idle_timer = qemu_new_timer_ns(vm_clock, kvm_kick_env, cenv); > Please add a comment here, explaining the occasional reader what we're doing here > +switch (cenv->mmu_model) { > +case POWERPC_MMU_BOOKE206: > +ret = kvm_booke206_tlb_init(cenv); > +break; > +default: > +break; > +} > + > return ret; > } > > @@ -140,6 +193,33 @@ void kvm_arch_reset_vcpu(CPUState *env) > { > } > > +static void kvm_sw_tlb_put(CPUState *env) > +{ > +#if defined(KVM_CAP_SW_TLB) See above > +struct kvm_dirty_tlb dirty_tlb; > +unsigned char *bitmap; > +int ret; > + > +if (!env->kvm_sw_tlb) { > +return; > +} > + > +bitmap = qemu_malloc((env->nb_tlb + 7) / 8); > +memset(bitmap, 0xFF, (env->nb_tlb + 7) / 8); > + > +dirty_tlb.bitmap = (uintptr_t)bitmap; > +dirty_tlb.num_dirty = env->nb_tlb; Pretty simple for now, but I like the idea of starting simple :). Would it make sense to keep the bitmap allocated throughout the lifetime of env? Alex
Re: [Qemu-devel] [PATCH 2/9] target-ppc: Handle memory-forced I/O controller access
On 17.06.2011, at 21:34, Andreas Färber wrote: > Am 17.06.2011 um 16:43 schrieb Alexander Graf: > >> From: Hervé Poussineau >> >> On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F >> is special-cased as memory-forced I/O controller access. It is supposed >> to be checked immediately if T=1, bypassing all protection mechanisms >> and acting cache-inhibited and global. >> >> Signed-off-by: Hervé Poussineau >> >> Simplified by avoiding reindentation. Added explanatory comments. >> >> Cc: Alexander Graf >> Signed-off-by: Andreas Färber >> Signed-off-by: Alexander Graf > > Feel free to remove the Cc: after adding a corresponding Sob or similar line. Hrm - I usually just use git am -s for the sob line. If you find the CC irritating, please just pass it to git send-email --cc and omit it from the patch. It's more or less unuseful information anyways :) Alex
Re: [Qemu-devel] [PATCH v5 2/5] guest agent: qemu-ga daemon
On 06/17/2011 03:13 PM, Luiz Capitulino wrote: On Fri, 17 Jun 2011 14:21:31 -0500 Michael Roth wrote: On 06/16/2011 01:42 PM, Luiz Capitulino wrote: On Tue, 14 Jun 2011 15:06:22 -0500 Michael Roth wrote: This is the actual guest daemon, it listens for requests over a virtio-serial/isa-serial/unix socket channel and routes them through to dispatch routines, and writes the results back to the channel in a manner similar to QMP. A shorthand invocation: qemu-ga -d Is equivalent to: qemu-ga -c virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent \ -p /var/run/qemu-guest-agent.pid -d Signed-off-by: Michael Roth Would be nice to have a more complete description, like explaining how to do a simple test. And this can't be built... --- qemu-ga.c | 631 qga/guest-agent-core.h |4 + 2 files changed, 635 insertions(+), 0 deletions(-) create mode 100644 qemu-ga.c diff --git a/qemu-ga.c b/qemu-ga.c new file mode 100644 index 000..df08d8c --- /dev/null +++ b/qemu-ga.c @@ -0,0 +1,631 @@ +/* + * QEMU Guest Agent + * + * Copyright IBM Corp. 2011 + * + * Authors: + * Adam Litke + * Michael Roth + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include "qemu_socket.h" +#include "json-streamer.h" +#include "json-parser.h" +#include "qint.h" +#include "qjson.h" +#include "qga/guest-agent-core.h" +#include "qga-qmp-commands.h" +#include "module.h" + +#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent" +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid" +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ +#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */ + +struct GAState { +const char *proxy_path; Where is this used? Nowhere actually. Will remove. +JSONMessageParser parser; +GMainLoop *main_loop; +guint conn_id; +GSocket *conn_sock; +GIOChannel *conn_channel; +guint listen_id; +GSocket *listen_sock; +GIOChannel *listen_channel; +const char *path; +const char *method; +bool virtio; /* fastpath to check for virtio to deal with poll() quirks */ +GACommandState *command_state; +GLogLevelFlags log_level; +FILE *log_file; +bool logging_enabled; +}; + +static void usage(const char *cmd) +{ +printf( +"Usage: %s -c\n" +"QEMU Guest Agent %s\n" +"\n" +" -c, --channel channel method: one of unix-connect, virtio-serial, or\n" +"isa-serial (virtio-serial is the default)\n" +" -p, --pathchannel path (%s is the default for virtio-serial)\n" +" -l, --logfile set logfile path, logs to stderr by default\n" +" -f, --pidfile specify pidfile (default is %s)\n" +" -v, --verbose log extra debugging information\n" +" -V, --version print version information and exit\n" +" -d, --daemonize become a daemon\n" +" -h, --helpdisplay this help and exit\n" +"\n" +"Report bugs to\n" +, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT); +} + +static void conn_channel_close(GAState *s); + +static const char *ga_log_level_str(GLogLevelFlags level) +{ +switch (level& G_LOG_LEVEL_MASK) { +case G_LOG_LEVEL_ERROR: +return "error"; +case G_LOG_LEVEL_CRITICAL: +return "critical"; +case G_LOG_LEVEL_WARNING: +return "warning"; +case G_LOG_LEVEL_MESSAGE: +return "message"; +case G_LOG_LEVEL_INFO: +return "info"; +case G_LOG_LEVEL_DEBUG: +return "debug"; +default: +return "user"; +} +} + +bool ga_logging_enabled(GAState *s) +{ +return s->logging_enabled; +} + +void ga_disable_logging(GAState *s) +{ +s->logging_enabled = false; +} + +void ga_enable_logging(GAState *s) +{ +s->logging_enabled = true; +} Just to check I got this right, this is needed because of the fsfreeze command, correct? Isn't it better to have a more descriptive name, like fsfrozen? First I thought this was about a log file. Then I realized this was probably about letting the user log in, but we don't seem to have this functionality so I got confused. Yup, this is currently due to fsfreeze support. From the perspective of the fsfreeze command the explicit "is_frozen" check makes more sense, but the reason it affects other RPCs is because because we can't log stuff in that state. If an RPC shoots itself in the foot by writing to a frozen filesystem we don't really care so much, and up until recently that case was handled with a pthread_cancel timeout mechanism (was removed for the time being, will re-implement using a child process most likely). What we don't want to do is give a host a way to bypass the expectation we set for guest owners by allowing RPCs to be l
[Qemu-devel] [PATCH 2/3] ppc: booke206: use MAV=2.0 TSIZE definition, fix 4G pages
This definition is backward compatible with MAV=1.0 as long as the guest does not set reserved bits in MAS1/MAS4. Also, fix the shift in booke206_tlb_to_page_size -- it's the base that should be able to hold a 4G page size, not the shift count. Signed-off-by: Scott Wood --- I ran into this when converting the info tlb patch to use the symbolic MAS defines -- figured it would be better to add support for this element of MAV=2.0 rather than remove it from "info tlb". hw/ppce500_mpc8544ds.c |2 +- target-ppc/cpu.h |4 ++-- target-ppc/helper.c|5 +++-- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c index 3cdeb43..a5e9378 100644 --- a/hw/ppce500_mpc8544ds.c +++ b/hw/ppce500_mpc8544ds.c @@ -177,7 +177,7 @@ out: /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) { -return (ffs(size >> 10) - 1) >> 1; +return ffs(size >> 10) - 1; } static void mmubooke_create_initial_mapping(CPUState *env, diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 8191ed2..5d80b1b 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -654,8 +654,8 @@ enum { #define MAS0_ATSEL_TLB 0 #define MAS0_ATSEL_LRATMAS0_ATSEL -#define MAS1_TSIZE_SHIFT 8 -#define MAS1_TSIZE_MASK(0xf << MAS1_TSIZE_SHIFT) +#define MAS1_TSIZE_SHIFT 7 +#define MAS1_TSIZE_MASK(0x1f << MAS1_TSIZE_SHIFT) #define MAS1_TS_SHIFT 12 #define MAS1_TS(1 << MAS1_TS_SHIFT) diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 4e9b98a..5d007c5 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -1278,7 +1278,7 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) { uint32_t tlbncfg; int tlbn = booke206_tlbm_to_tlbn(env, tlb); -target_phys_addr_t tlbm_size; +int tlbm_size; tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn]; @@ -1286,9 +1286,10 @@ target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb) tlbm_size = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; } else { tlbm_size = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT; +tlbm_size <<= 1; } -return (1 << (tlbm_size << 1)) << 10; +return 1024ULL << tlbm_size; } /* TLB check function for MAS based SoftTLBs */ -- 1.7.4.1
[Qemu-devel] [PATCH 1/3] kvm: ppc: booke206: use MMU API
Share the TLB array with KVM. This allows us to set the initial TLB both on initial boot and reset, is useful for debugging, and could eventually be used to support migration. Signed-off-by: Scott Wood --- hw/ppce500_mpc8544ds.c |2 + target-ppc/cpu.h |2 + target-ppc/kvm.c | 85 3 files changed, 89 insertions(+), 0 deletions(-) diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c index 5ac8843..3cdeb43 100644 --- a/hw/ppce500_mpc8544ds.c +++ b/hw/ppce500_mpc8544ds.c @@ -192,6 +192,8 @@ static void mmubooke_create_initial_mapping(CPUState *env, tlb->mas2 = va & TARGET_PAGE_MASK; tlb->mas7_3 = pa & TARGET_PAGE_MASK; tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX; + +env->tlb_dirty = true; } static void mpc8544ds_cpu_reset(void *opaque) diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 46d86be..8191ed2 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -921,6 +921,8 @@ struct CPUPPCState { ppc_tlb_t tlb; /* TLB is optional. Allocate them only if needed*/ /* 403 dedicated access protection registers */ target_ulong pb[4]; +bool tlb_dirty; /* Set to non-zero when modifying TLB */ +bool kvm_sw_tlb; /* non-zero if KVM SW TLB API is active*/ #endif /* Other registers */ diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index e7b1b10..9a88fc9 100644 --- a/target-ppc/kvm.c +++ b/target-ppc/kvm.c @@ -122,6 +122,51 @@ static int kvm_arch_sync_sregs(CPUState *cenv) return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs); } +static int kvm_booke206_tlb_init(CPUState *env) +{ +#if defined(KVM_CAP_SW_TLB) && defined(KVM_MMU_FSL_BOOKE_NOHV) +struct kvm_book3e_206_tlb_params params = {}; +struct kvm_config_tlb cfg = {}; +size_t array_len; +unsigned int entries = 0; +int ret, i; + +if (!kvm_enabled() || +!kvm_check_extension(env->kvm_state, KVM_CAP_SW_TLB)) { +return 0; +} + +for (i = 0; i < ARRAY_SIZE(params.tlb_sizes); i++) { +params.tlb_sizes[i] = booke206_tlb_size(env, i); +params.tlb_ways[i] = booke206_tlb_ways(env, i); +entries += params.tlb_sizes[i]; +} + +if (entries != env->nb_tlb) { +cpu_abort(env, "%s: nb_tlb mismatch\n", __func__); +} + +array_len = sizeof(struct kvm_book3e_206_tlb_entry) * entries; +env->tlb_dirty = true; + +cfg.array = (uintptr_t)env->tlb.tlbm; +cfg.array_len = sizeof(ppcmas_tlb_t) * entries; +cfg.params = (uintptr_t)¶ms; +cfg.mmu_type = KVM_MMU_FSL_BOOKE_NOHV; + +ret = kvm_vcpu_ioctl(env, KVM_CONFIG_TLB, &cfg); +if (ret < 0) { +fprintf(stderr, "%s: couldn't KVM_CONFIG_TLB: %s\n", +__func__, strerror(-ret)); +return ret; +} + +env->kvm_sw_tlb = true; +#endif + +return 0; +} + int kvm_arch_init_vcpu(CPUState *cenv) { int ret; @@ -133,6 +178,14 @@ int kvm_arch_init_vcpu(CPUState *cenv) idle_timer = qemu_new_timer_ns(vm_clock, kvm_kick_env, cenv); +switch (cenv->mmu_model) { +case POWERPC_MMU_BOOKE206: +ret = kvm_booke206_tlb_init(cenv); +break; +default: +break; +} + return ret; } @@ -140,6 +193,33 @@ void kvm_arch_reset_vcpu(CPUState *env) { } +static void kvm_sw_tlb_put(CPUState *env) +{ +#if defined(KVM_CAP_SW_TLB) +struct kvm_dirty_tlb dirty_tlb; +unsigned char *bitmap; +int ret; + +if (!env->kvm_sw_tlb) { +return; +} + +bitmap = qemu_malloc((env->nb_tlb + 7) / 8); +memset(bitmap, 0xFF, (env->nb_tlb + 7) / 8); + +dirty_tlb.bitmap = (uintptr_t)bitmap; +dirty_tlb.num_dirty = env->nb_tlb; + +ret = kvm_vcpu_ioctl(env, KVM_DIRTY_TLB, &dirty_tlb); +if (ret) { +fprintf(stderr, "%s: KVM_DIRTY_TLB: %s\n", +__func__, strerror(-ret)); +} + +qemu_free(bitmap); +#endif +} + int kvm_arch_put_registers(CPUState *env, int level) { struct kvm_regs regs; @@ -177,6 +257,11 @@ int kvm_arch_put_registers(CPUState *env, int level) if (ret < 0) return ret; +if (env->tlb_dirty) { +kvm_sw_tlb_put(env); +env->tlb_dirty = false; +} + return ret; } -- 1.7.4.1
Re: [Qemu-devel] [PATCH] Fix ioapic vmstate
Am 17.06.2011 um 09:30 schrieb Pavel Dovgaluk: This patch fixes save/restore vmstate of IOAPIC. When irr member of IOAPICState is not saved and loaded, restoring becomes non-deterministic, because irr is kept from state of VM that was before loading. Signed-off-by: Pavel Dovgalyuk --- hw/ioapic.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/ioapic.c b/hw/ioapic.c index 2109568..e583284 100644 --- a/hw/ioapic.c +++ b/hw/ioapic.c @@ -207,6 +207,7 @@ static const VMStateDescription vmstate_ioapic = { .fields = (VMStateField []) { VMSTATE_UINT8(id, IOAPICState), VMSTATE_UINT8(ioregsel, IOAPICState), +VMSTATE_UINT32(irr, IOAPICState), VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS), VMSTATE_END_OF_LIST() } You can't just add a field to VMState. Either the version_id must be incremented, or a subsection must be used. Andreas
[Qemu-devel] [PATCH 3/3] ppc: booke206: add "info tlb" support
Signed-off-by: Scott Wood --- hmp-commands.hx |2 +- monitor.c |5 ++- target-ppc/cpu.h|2 + target-ppc/helper.c | 89 +++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 6ad8806..014a4fb 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1306,7 +1306,7 @@ show i8259 (PIC) state @item info pci show emulated PCI device info @item info tlb -show virtual to physical memory mappings (i386, SH4 and SPARC only) +show virtual to physical memory mappings (i386, SH4, SPARC, and PPC only) @item info mem show the active virtual memory mappings (i386 only) @item info jit diff --git a/monitor.c b/monitor.c index 6af6a4d..68e94af 100644 --- a/monitor.c +++ b/monitor.c @@ -2408,7 +2408,7 @@ static void tlb_info(Monitor *mon) #endif -#if defined(TARGET_SPARC) +#if defined(TARGET_SPARC) || defined(TARGET_PPC) static void tlb_info(Monitor *mon) { CPUState *env1 = mon_get_cpu(); @@ -2901,7 +2901,8 @@ static const mon_cmd_t info_cmds[] = { .user_print = do_pci_info_print, .mhandler.info_new = do_pci_info, }, -#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) +#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \ +defined(TARGET_PPC) { .name = "tlb", .args_type = "", diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 5d80b1b..48f7e2c 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -2020,4 +2020,6 @@ static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int tlbn, extern void (*cpu_ppc_hypercall)(CPUState *); +void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env); + #endif /* !defined (__CPU_PPC_H__) */ diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 5d007c5..e22b1cb 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -1451,6 +1451,95 @@ found_tlb: return ret; } +static const char *book3e_tsize_to_str[32] = { +"1K", "2K", "4K", "8K", "16K", "32K", "64K", "128K", "256K", "512K", +"1M", "2M", "4M", "8M", "16M", "32M", "64M", "128M", "256M", "512M", +"1G", "2G", "4G", "8G", "16G", "32G", "64G", "128G", "256G", "512G", +"1T", "2T" +}; + +static void mmubooke206_dump_one_tlb(FILE *f, fprintf_function cpu_fprintf, + CPUState *env, int tlbn, int offset, + int tlbsize) +{ +ppcmas_tlb_t *entry; +int i; + +cpu_fprintf(f, "\nTLB%d:\n", tlbn); +cpu_fprintf(f, "Effective Physical Size TID TS SRWX URWX WIMGE U0123\n"); + +entry = &env->tlb.tlbm[offset]; +for (i = 0; i < tlbsize; i++, entry++) { +target_phys_addr_t ea, pa, size; +int tsize; + +/* valid? */ +if (!(entry->mas1 & MAS1_VALID)) { +continue; +} + +tsize = (entry->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT; +size = 1024ULL << tsize; +ea = entry->mas2 & ~(size - 1); +pa = entry->mas7_3 & ~(size - 1); + +cpu_fprintf(f, "0x%016" PRIx64 " 0x%016" PRIx64 " %4s %-5u %1u S%c%c%c U%c%c%c %c%c%c%c%c U%c%c%c%c\n", +(uint64_t)ea, (uint64_t)pa, +book3e_tsize_to_str[tsize], +(entry->mas1 & MAS1_TID_MASK) >> MAS1_TID_SHIFT, +(entry->mas1 & MAS1_TS) >> MAS1_TS_SHIFT, +entry->mas7_3 & MAS3_SR ? 'R' : '-', +entry->mas7_3 & MAS3_SW ? 'W' : '-', +entry->mas7_3 & MAS3_SX ? 'X' : '-', +entry->mas7_3 & MAS3_UR ? 'R' : '-', +entry->mas7_3 & MAS3_UW ? 'W' : '-', +entry->mas7_3 & MAS3_UX ? 'X' : '-', +entry->mas2 & MAS2_W ? 'W' : '-', +entry->mas2 & MAS2_I ? 'I' : '-', +entry->mas2 & MAS2_M ? 'M' : '-', +entry->mas2 & MAS2_G ? 'G' : '-', +entry->mas2 & MAS2_E ? 'E' : '-', +entry->mas7_3 & MAS3_U0 ? '0' : '-', +entry->mas7_3 & MAS3_U1 ? '1' : '-', +entry->mas7_3 & MAS3_U2 ? '2' : '-', +entry->mas7_3 & MAS3_U3 ? '3' : '-'); +} +} + +static void mmubooke206_dump_mmu(FILE *f, fprintf_function cpu_fprintf, + CPUState *env) +{ +int offset = 0; +int i; + +if (kvm_enabled() && !env->kvm_sw_tlb) { +cpu_fprintf(f, "Cannot access KVM TLB\n"); +return; +} + +for (i = 0; i < BOOKE206_MAX_TLBN; i++) { +int size = booke206_tlb_size(env, i); + +if (size == 0) { +continue; +} + +mmubooke206_dump_one_tlb(f, cpu_fprintf, env, i, offset, size); +offset += size; +} +} + +void dump_mmu(FILE *f, fprintf_function cpu_fprintf, CPUState *env) +{ +switch (env->mmu_model) {
[Qemu-devel] [PATCH 0/3] ppc: booke206: KVM MMU API and info tlb
This depends on these qemu patches: http://patchwork.ozlabs.org/patch/100826/ (PPC: E500: Use MAS registers instead of internal TLB representation) http://patchwork.ozlabs.org/patch/100821/ (PPC: move TLBs to their own arrays) For this functionality to work with KVM, this kernel patch is required: http://www.spinics.net/lists/kvm-ppc/msg02843.html Scott Wood (3): kvm: ppc: booke206: use MMU API ppc: booke206: use MAV=2.0 TSIZE definition, fix 4G pages ppc: booke206: add "info tlb" support hmp-commands.hx|2 +- hw/ppce500_mpc8544ds.c |4 ++- monitor.c |5 ++- target-ppc/cpu.h |8 +++- target-ppc/helper.c| 94 ++- target-ppc/kvm.c | 85 +++ 6 files changed, 190 insertions(+), 8 deletions(-) -- 1.7.4.1
Re: [Qemu-devel] [PATCH 2/9] target-ppc: Handle memory-forced I/O controller access
Am 17.06.2011 um 16:43 schrieb Alexander Graf: From: Hervé Poussineau On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F is special-cased as memory-forced I/O controller access. It is supposed to be checked immediately if T=1, bypassing all protection mechanisms and acting cache-inhibited and global. Signed-off-by: Hervé Poussineau Simplified by avoiding reindentation. Added explanatory comments. Cc: Alexander Graf Signed-off-by: Andreas Färber Signed-off-by: Alexander Graf Feel free to remove the Cc: after adding a corresponding Sob or similar line.
Re: [Qemu-devel] [PATCH v5 2/5] guest agent: qemu-ga daemon
On Fri, 17 Jun 2011 14:21:31 -0500 Michael Roth wrote: > On 06/16/2011 01:42 PM, Luiz Capitulino wrote: > > On Tue, 14 Jun 2011 15:06:22 -0500 > > Michael Roth wrote: > > > >> This is the actual guest daemon, it listens for requests over a > >> virtio-serial/isa-serial/unix socket channel and routes them through > >> to dispatch routines, and writes the results back to the channel in > >> a manner similar to QMP. > >> > >> A shorthand invocation: > >> > >>qemu-ga -d > >> > >> Is equivalent to: > >> > >>qemu-ga -c virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent \ > >>-p /var/run/qemu-guest-agent.pid -d > >> > >> Signed-off-by: Michael Roth > > > > Would be nice to have a more complete description, like explaining how to > > do a simple test. > > > > And this can't be built... > > > >> --- > >> qemu-ga.c | 631 > >> > >> qga/guest-agent-core.h |4 + > >> 2 files changed, 635 insertions(+), 0 deletions(-) > >> create mode 100644 qemu-ga.c > >> > >> diff --git a/qemu-ga.c b/qemu-ga.c > >> new file mode 100644 > >> index 000..df08d8c > >> --- /dev/null > >> +++ b/qemu-ga.c > >> @@ -0,0 +1,631 @@ > >> +/* > >> + * QEMU Guest Agent > >> + * > >> + * Copyright IBM Corp. 2011 > >> + * > >> + * Authors: > >> + * Adam Litke > >> + * Michael Roth > >> + * > >> + * This work is licensed under the terms of the GNU GPL, version 2 or > >> later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include > >> +#include "qemu_socket.h" > >> +#include "json-streamer.h" > >> +#include "json-parser.h" > >> +#include "qint.h" > >> +#include "qjson.h" > >> +#include "qga/guest-agent-core.h" > >> +#include "qga-qmp-commands.h" > >> +#include "module.h" > >> + > >> +#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent" > >> +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid" > >> +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ > >> +#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */ > >> + > >> +struct GAState { > >> +const char *proxy_path; > > > > Where is this used? > > > > Nowhere actually. Will remove. > > >> +JSONMessageParser parser; > >> +GMainLoop *main_loop; > >> +guint conn_id; > >> +GSocket *conn_sock; > >> +GIOChannel *conn_channel; > >> +guint listen_id; > >> +GSocket *listen_sock; > >> +GIOChannel *listen_channel; > >> +const char *path; > >> +const char *method; > >> +bool virtio; /* fastpath to check for virtio to deal with poll() > >> quirks */ > >> +GACommandState *command_state; > >> +GLogLevelFlags log_level; > >> +FILE *log_file; > >> +bool logging_enabled; > >> +}; > >> + > >> +static void usage(const char *cmd) > >> +{ > >> +printf( > >> +"Usage: %s -c\n" > >> +"QEMU Guest Agent %s\n" > >> +"\n" > >> +" -c, --channel channel method: one of unix-connect, virtio-serial, > >> or\n" > >> +"isa-serial (virtio-serial is the default)\n" > >> +" -p, --pathchannel path (%s is the default for virtio-serial)\n" > >> +" -l, --logfile set logfile path, logs to stderr by default\n" > >> +" -f, --pidfile specify pidfile (default is %s)\n" > >> +" -v, --verbose log extra debugging information\n" > >> +" -V, --version print version information and exit\n" > >> +" -d, --daemonize become a daemon\n" > >> +" -h, --helpdisplay this help and exit\n" > >> +"\n" > >> +"Report bugs to\n" > >> +, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT); > >> +} > >> + > >> +static void conn_channel_close(GAState *s); > >> + > >> +static const char *ga_log_level_str(GLogLevelFlags level) > >> +{ > >> +switch (level& G_LOG_LEVEL_MASK) { > >> +case G_LOG_LEVEL_ERROR: > >> +return "error"; > >> +case G_LOG_LEVEL_CRITICAL: > >> +return "critical"; > >> +case G_LOG_LEVEL_WARNING: > >> +return "warning"; > >> +case G_LOG_LEVEL_MESSAGE: > >> +return "message"; > >> +case G_LOG_LEVEL_INFO: > >> +return "info"; > >> +case G_LOG_LEVEL_DEBUG: > >> +return "debug"; > >> +default: > >> +return "user"; > >> +} > >> +} > >> + > >> +bool ga_logging_enabled(GAState *s) > >> +{ > >> +return s->logging_enabled; > >> +} > >> + > >> +void ga_disable_logging(GAState *s) > >> +{ > >> +s->logging_enabled = false; > >> +} > >> + > >> +void ga_enable_logging(GAState *s) > >> +{ > >> +s->logging_enabled = true; > >> +} > > > > Just to check I got this right, this is needed because of the fsfreeze > > command, correct? Isn't it better to have a more descriptive name, like > > fsfrozen? > > > > First I thought this was about a log file. Then I realized this was > > probably about letting th
Re: [Qemu-devel] [PATCH v5 3/5] guest agent: add guest agent RPCs/commands
On 06/16/2011 01:52 PM, Luiz Capitulino wrote: On Tue, 14 Jun 2011 15:06:23 -0500 Michael Roth wrote: This adds the initial set of QMP/QAPI commands provided by the guest agent: guest-sync guest-ping guest-info guest-shutdown guest-file-open guest-file-read guest-file-write guest-file-seek guest-file-close guest-fsfreeze-freeze guest-fsfreeze-thaw guest-fsfreeze-status The input/output specification for these commands are documented in the schema. Signed-off-by: Michael Roth --- qerror.c |4 + qerror.h |3 + qga/guest-agent-commands.c | 522 qga/guest-agent-core.h |1 + 4 files changed, 530 insertions(+), 0 deletions(-) create mode 100644 qga/guest-agent-commands.c diff --git a/qerror.c b/qerror.c index d7fcd93..24f0c48 100644 --- a/qerror.c +++ b/qerror.c @@ -213,6 +213,10 @@ static const QErrorStringTable qerror_table[] = { .error_fmt = QERR_VNC_SERVER_FAILED, .desc = "Could not start VNC server on %(target)", }, +{ +.error_fmt = QERR_QGA_LOGGING_FAILED, +.desc = "failed to write log statement due to logging being disabled", +}, {} }; diff --git a/qerror.h b/qerror.h index 7a89a50..bf3d5a9 100644 --- a/qerror.h +++ b/qerror.h @@ -184,4 +184,7 @@ QError *qobject_to_qerror(const QObject *obj); #define QERR_FEATURE_DISABLED \ "{ 'class': 'FeatureDisabled', 'data': { 'name': %s } }" +#define QERR_QGA_LOGGING_FAILED \ +"{ 'class': 'QgaLoggingFailed', 'data': {} }" + #endif /* QERROR_H */ diff --git a/qga/guest-agent-commands.c b/qga/guest-agent-commands.c new file mode 100644 index 000..6f9886a --- /dev/null +++ b/qga/guest-agent-commands.c @@ -0,0 +1,522 @@ +/* + * QEMU Guest Agent commands + * + * Copyright IBM Corp. 2011 + * + * Authors: + * Michael Roth + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include +#include +#include +#include +#include +#include "qga/guest-agent-core.h" +#include "qga-qmp-commands.h" +#include "qerror.h" + +static GAState *ga_state; + +static bool logging_enabled(void) +{ +return ga_logging_enabled(ga_state); +} + +static void disable_logging(void) +{ +ga_disable_logging(ga_state); +} + +static void enable_logging(void) +{ +ga_enable_logging(ga_state); +} + +/* Note: in some situations, like with the fsfreeze, logging may be + * temporarilly disabled. if it is necessary that a command be able + * to log for accounting purposes, check logging_enabled() beforehand, + * and use the QERR_QGA_LOGGING_DISABLED to generate an error + */ +static void slog(const char *fmt, ...) +{ +va_list ap; + +va_start(ap, fmt); +g_logv("syslog", G_LOG_LEVEL_INFO, fmt, ap); +va_end(ap); +} + +int64_t qmp_guest_sync(int64_t id, Error **errp) +{ +return id; +} + +void qmp_guest_ping(Error **err) +{ +slog("guest-ping called"); +} + +struct GuestAgentInfo *qmp_guest_info(Error **err) +{ +GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo)); + +info->version = g_strdup(QGA_VERSION); + +return info; +} + +void qmp_guest_shutdown(const char *shutdown_mode, Error **err) I'd call the argument just 'mode'. +{ +int ret; +const char *shutdown_flag; + +if (!logging_enabled()) { +error_set(err, QERR_QGA_LOGGING_FAILED); +return; +} + +slog("guest-shutdown called, shutdown_mode: %s", shutdown_mode); +if (strcmp(shutdown_mode, "halt") == 0) { +shutdown_flag = "-H"; +} else if (strcmp(shutdown_mode, "powerdown") == 0) { +shutdown_flag = "-P"; +} else if (strcmp(shutdown_mode, "reboot") == 0) { +shutdown_flag = "-r"; +} else { +error_set(err, QERR_INVALID_PARAMETER_VALUE, "shutdown_mode", + "halt|powerdown|reboot"); +return; +} + +ret = fork(); +if (ret == 0) { +/* child, start the shutdown */ +setsid(); +fclose(stdin); +fclose(stdout); +fclose(stderr); Would be nice to have a log file, whose descriptor is passed to the child. + +sleep(5); Why sleep()? Want to give the agent time to send a response. It's still racy, but less so that immediately issuing the shutdown. Ideal we'd push the sleep() into shutdown's time param, but that only has minute resolution. +ret = execl("/sbin/shutdown", "shutdown", shutdown_flag, "+0", +"hypervisor initiated shutdown", (char*)NULL); +exit(!!ret); +} else if (ret< 0) { +error_set(err, QERR_UNDEFINED_ERROR); +} Doesn't have the parent process wait for the child, so that exec() errors can be reported? The exec() won't return until the shutdown is executed, so the RPC's behavior would be racy. At some point I documented that the shutdown is an async "request" that may or may not complete but that wa
Re: [Qemu-devel] [PATCH v5 2/5] guest agent: qemu-ga daemon
On 06/16/2011 01:42 PM, Luiz Capitulino wrote: On Tue, 14 Jun 2011 15:06:22 -0500 Michael Roth wrote: This is the actual guest daemon, it listens for requests over a virtio-serial/isa-serial/unix socket channel and routes them through to dispatch routines, and writes the results back to the channel in a manner similar to QMP. A shorthand invocation: qemu-ga -d Is equivalent to: qemu-ga -c virtio-serial -p /dev/virtio-ports/org.qemu.guest_agent \ -p /var/run/qemu-guest-agent.pid -d Signed-off-by: Michael Roth Would be nice to have a more complete description, like explaining how to do a simple test. And this can't be built... --- qemu-ga.c | 631 qga/guest-agent-core.h |4 + 2 files changed, 635 insertions(+), 0 deletions(-) create mode 100644 qemu-ga.c diff --git a/qemu-ga.c b/qemu-ga.c new file mode 100644 index 000..df08d8c --- /dev/null +++ b/qemu-ga.c @@ -0,0 +1,631 @@ +/* + * QEMU Guest Agent + * + * Copyright IBM Corp. 2011 + * + * Authors: + * Adam Litke + * Michael Roth + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include "qemu_socket.h" +#include "json-streamer.h" +#include "json-parser.h" +#include "qint.h" +#include "qjson.h" +#include "qga/guest-agent-core.h" +#include "qga-qmp-commands.h" +#include "module.h" + +#define QGA_VIRTIO_PATH_DEFAULT "/dev/virtio-ports/org.qemu.guest_agent" +#define QGA_PIDFILE_DEFAULT "/var/run/qemu-va.pid" +#define QGA_BAUDRATE_DEFAULT B38400 /* for isa-serial channels */ +#define QGA_TIMEOUT_DEFAULT 30*1000 /* ms */ + +struct GAState { +const char *proxy_path; Where is this used? Nowhere actually. Will remove. +JSONMessageParser parser; +GMainLoop *main_loop; +guint conn_id; +GSocket *conn_sock; +GIOChannel *conn_channel; +guint listen_id; +GSocket *listen_sock; +GIOChannel *listen_channel; +const char *path; +const char *method; +bool virtio; /* fastpath to check for virtio to deal with poll() quirks */ +GACommandState *command_state; +GLogLevelFlags log_level; +FILE *log_file; +bool logging_enabled; +}; + +static void usage(const char *cmd) +{ +printf( +"Usage: %s -c\n" +"QEMU Guest Agent %s\n" +"\n" +" -c, --channel channel method: one of unix-connect, virtio-serial, or\n" +"isa-serial (virtio-serial is the default)\n" +" -p, --pathchannel path (%s is the default for virtio-serial)\n" +" -l, --logfile set logfile path, logs to stderr by default\n" +" -f, --pidfile specify pidfile (default is %s)\n" +" -v, --verbose log extra debugging information\n" +" -V, --version print version information and exit\n" +" -d, --daemonize become a daemon\n" +" -h, --helpdisplay this help and exit\n" +"\n" +"Report bugs to\n" +, cmd, QGA_VERSION, QGA_VIRTIO_PATH_DEFAULT, QGA_PIDFILE_DEFAULT); +} + +static void conn_channel_close(GAState *s); + +static const char *ga_log_level_str(GLogLevelFlags level) +{ +switch (level& G_LOG_LEVEL_MASK) { +case G_LOG_LEVEL_ERROR: +return "error"; +case G_LOG_LEVEL_CRITICAL: +return "critical"; +case G_LOG_LEVEL_WARNING: +return "warning"; +case G_LOG_LEVEL_MESSAGE: +return "message"; +case G_LOG_LEVEL_INFO: +return "info"; +case G_LOG_LEVEL_DEBUG: +return "debug"; +default: +return "user"; +} +} + +bool ga_logging_enabled(GAState *s) +{ +return s->logging_enabled; +} + +void ga_disable_logging(GAState *s) +{ +s->logging_enabled = false; +} + +void ga_enable_logging(GAState *s) +{ +s->logging_enabled = true; +} Just to check I got this right, this is needed because of the fsfreeze command, correct? Isn't it better to have a more descriptive name, like fsfrozen? First I thought this was about a log file. Then I realized this was probably about letting the user log in, but we don't seem to have this functionality so I got confused. Yup, this is currently due to fsfreeze support. From the perspective of the fsfreeze command the explicit "is_frozen" check makes more sense, but the reason it affects other RPCs is because because we can't log stuff in that state. If an RPC shoots itself in the foot by writing to a frozen filesystem we don't really care so much, and up until recently that case was handled with a pthread_cancel timeout mechanism (was removed for the time being, will re-implement using a child process most likely). What we don't want to do is give a host a way to bypass the expectation we set for guest owners by allowing RPCs to be logged. So that's what the check is based on, rather than lower level details like *why* logging is disabl
[Qemu-devel] [PATCH v2] virtio-serial: Fix segfault on guest boot
If I start qemu with: # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ -device virtio-serial \ -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ -device virtserialport,chardev=foo,name=org.qemu.guest_agent I get a segfault when booting a Fedora 14 guest. The backtrace says: Program terminated with signal 11, Segmentation fault. #0 0x00420850 in handle_control_message (vser=0x3732bd0, buf=0x2c173e0, len=8) at /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); What's happening is VIRTIO_CONSOLE_DEVICE_READY is a message for the whole device, not for an individual port. So port is NULL. This bug was introduced by commit a15bb0d6a981de749452a5180fc8084d625671da. This commit fixes that by making the port returned by find_port_by_id() be used only by the VIRTIO_CONSOLE_PORT_READY and VIRTIO_CONSOLE_PORT_OPEN messages. Signed-off-by: Luiz Capitulino --- hw/virtio-serial-bus.c | 24 ++-- 1 files changed, 14 insertions(+), 10 deletions(-) diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 9a12104..33a6f61 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -328,18 +328,11 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len) cpkt.event = lduw_p(&gcpkt->event); cpkt.value = lduw_p(&gcpkt->value); -port = find_port_by_id(vser, ldl_p(&gcpkt->id)); -if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) -return; - -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); - -switch(cpkt.event) { -case VIRTIO_CONSOLE_DEVICE_READY: +if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) { if (!cpkt.value) { error_report("virtio-serial-bus: Guest failure in adding device %s\n", vser->bus.qbus.name); -break; +return; } /* * The device is up, we can now tell the device about all the @@ -348,8 +341,19 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len) QTAILQ_FOREACH(port, &vser->ports, next) { send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); } -break; +return; +} +port = find_port_by_id(vser, ldl_p(&gcpkt->id)); +if (!port) { +error_report("virtio-serial-bus: Expected port id %d for device %s\n", + ldl_p(&gcpkt->id), vser->bus.qbus.name); +return; +} + +info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); + +switch(cpkt.event) { case VIRTIO_CONSOLE_PORT_READY: if (!cpkt.value) { error_report("virtio-serial-bus: Guest failure in adding port %u for device %s\n", -- 1.7.4.4
Re: [Qemu-devel] QEMU API for modelling of user's hardware
I've tried to collect some information from the QEMU sources (I've built my local LXR service with QEMU sources tom make it easier ;-) ), and from different web sources. As a result I've prepared a model of Bus Mastering PCI device which is a simple AES256 accelerator. Sources are available on alt.sources (subject:"WZENC1 - Model of Bus Mastering PCI AES256 accelerator for QEM") or in Google archive: http://groups.google.com/group/alt.sources/browse_thread/thread/cc80f25d573813f5 I hope, that it will be useful as didactic aid for implementing of device drivers, but may also be a skeleton for model of real hardware... -- Regards, Wojtek
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On Fri, 17 Jun 2011 21:39:26 +0530 Amit Shah wrote: > On (Fri) 17 Jun 2011 [10:16:44], Luiz Capitulino wrote: > > On Fri, 17 Jun 2011 12:17:36 +0530 > > Amit Shah wrote: > > > > > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: > > > > If I start qemu with: > > > > > > > > # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ > > > > -device virtio-serial \ > > > > -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ > > > > -device virtserialport,chardev=foo,name=org.qemu.guest_agent > > > > > > > > I get a segfault when booting a Fedora 14 guest. The backtrace says: > > > > > > > > Program terminated with signal 11, Segmentation fault. > > > > #0 0x00420850 in handle_control_message (vser=0x3732bd0, > > > > buf=0x2c173e0, len=8) at > > > > /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 > > > > 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > > > > Strange, I've not seen it so far in my testing (neither in the daily > > > test runs of the virtio-serial testsuite). > > > > > > > I've also bisected this and git points out to commit: > > > > > > > > commit a15bb0d6a981de749452a5180fc8084d625671da > > > > Author: Markus Armbruster > > > > Date: Wed May 25 14:21:13 2011 +0200 > > > > > > > > virtio-serial: Drop redundant VirtIOSerialPort member info > > > > > > > > I think what's happening is that the device is not initialized on a > > > > VIRTIO_CONSOLE_DEVICE_READY event. Moving the DO_UPCAST() call to > > > > the other events fixes the problem to me. > > > > > > > > Signed-off-by: Luiz Capitulino > > > > --- > > > > hw/virtio-serial-bus.c |4 ++-- > > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > > index 9a12104..579f676 100644 > > > > --- a/hw/virtio-serial-bus.c > > > > +++ b/hw/virtio-serial-bus.c > > > > @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial > > > > *vser, void *buf, size_t len) > > > > if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > > > > return; > > > > > > > > -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > > - > > > > > > Ah - this missed the !port check. It should be possible to do this in > > > a 'if (port)' block instead of replicating in the individual case > > > statements. > > > > > > Thanks for the debugging and patch; please update with the above and > > > I'll apply it to the virtio-serial tree. > > > > What about moving the VIRTIO_CONSOLE_DEVICE_READY handling out of the > > switch, like the patch below? This way the function is divided in a way > > that related events are handled together. > > > > I'll implement your first suggestion if you don't like this... > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > index 579f676..5f96245 100644 > > --- a/hw/virtio-serial-bus.c > > +++ b/hw/virtio-serial-bus.c > > @@ -325,19 +325,12 @@ static void handle_control_message(VirtIOSerial > > *vser, void *buf, size_t len) > > return; > > } > > > > -cpkt.event = lduw_p(&gcpkt->event); > > cpkt.value = lduw_p(&gcpkt->value); > > - > > -port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > > -if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > > -return; > > - > > -switch(cpkt.event) { > > -case VIRTIO_CONSOLE_DEVICE_READY: > > +cpkt.event = lduw_p(&gcpkt->event); > > +if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) { > > if (!cpkt.value) { > > -error_report("virtio-serial-bus: Guest failure in adding > > device %s\n", > > - vser->bus.qbus.name); > > -break; > > +error_report("virtio-serial-bus: Guest failure in adding > > device %s\n", vser->bus.qbus.name); > > +return; > > The line split should remain -- else it goes beyond 80 chars. It's already beyond 80 chars to me. > > > } > > /* > > * The device is up, we can now tell the device about all the > > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial *vser, > > void *buf, size_t len) > > QTAILQ_FOREACH(port, &vser->ports, next) { > > send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); > > } > > -break; > > +return; > > +} > > Makes me think of one case (totally unrelated to what you found)where > the guest can fool us: by sending multiple VIRTIO_CONSOLE_DEVICE_READY > messages. It will be handled just fine, no? > > > +port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > > +assert(port != NULL); > > I doubt if assert is the right thing: if the guest sends bad data, we > shouldn't just kill it. It's easier to ignore such data, and perhaps > just log it. Right. > > > + > > +switch(cpkt.event) { > > case VIRTIO_CONSOLE_PORT_READY: > > if (!cpkt.va
Re: [Qemu-devel] [PATCH v2] Register Linux dyntick timer as per-thread signal
On 06/17/2011 02:25 AM, Jan Kiszka wrote: > Derived from kvm-tool patch > http://thread.gmane.org/gmane.comp.emulators.kvm.devel/74309 > > Ingo Molnar pointed out that sending the timer signal to the whole > process, just blocking it everywhere, is suboptimal with an increasing > number of threads. QEMU is also using this pattern so far. > > Linux provides a (non-portable) way to restrict the signal to a single > thread: We can use SIGEV_THREAD_ID unless we are forced to emulate > signalfd via an additional thread. That case could theoretically be > optimized as well, but it doesn't look worth bothering. > > Signed-off-by: Jan Kiszka Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change
Resending block_set command patch to avoid mismatch between "paramname" defined for human monitor and "name" used in do_block_set for reading name of block device parameter. --- New command "block_set" added for dynamically changing any of the block device parameters. For now, dynamic setting of hostcache params using block_set is implemented. Other block device parameter changes can be integrated in similar lines. Signed-off-by: Supriya Kannery --- block.c | 41 + block.h |2 ++ blockdev.c | 32 blockdev.h |1 + hmp-commands.hx | 15 +++ qmp-commands.hx | 30 +- 6 files changed, 120 insertions(+), 1 deletion(-) Index: qemu/hmp-commands.hx === --- qemu.orig/hmp-commands.hx +++ qemu/hmp-commands.hx @@ -70,6 +70,21 @@ but should be used with extreme caution. resizes image files, it can not resize block devices like LVM volumes. ETEXI + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "On/Off block device parameters like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +STEXI +@item block_set +@findex block_set +Change block device params (eg:"hostcache"=on/off) while guest is running. +ETEXI + { .name = "eject", Index: qemu/qmp-commands.hx === --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -693,7 +693,35 @@ Example: EQMP -{ + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "Enable/Disable block device params like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +SQMP +block_set +- + +Change various block device parameters like hostcache. + +Arguments: + +- "device": the device's ID, must be unique (json-string) +- "name": name of the parameter to be changed" (json-string) +- "enable": value to be set for the parameter, 'true' or 'false' (json-bool) + +Example: + +-> { "execute": "block_set", "arguments": { "device": "ide0-hd0", "name": "hostcache", "enable": true } } +<- { "return": {} } + +EQMP + + { .name = "balloon", .args_type = "value:M", .params = "target", Index: qemu/blockdev.c === --- qemu.orig/blockdev.c +++ qemu/blockdev.c @@ -797,3 +797,35 @@ int do_block_resize(Monitor *mon, const return 0; } + + +/* + * Handle changes to block device settings, like hostcache, + * while guest is running. +*/ +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *name = qdict_get_str(qdict, "name"); + int enable = qdict_get_bool(qdict, "enable"); + BlockDriverState *bs; + + bs = bdrv_find(device); + if (!bs) { + qerror_report(QERR_DEVICE_NOT_FOUND, device); + return -1; + } + + if (!(strcmp(name, "hostcache"))) { + if (bdrv_is_inserted(bs)) { + /* cache change applicable only if device inserted */ + return bdrv_change_hostcache(bs, enable); + } else { + qerror_report(QERR_DEVICE_NOT_INSERTED, device); + return -1; + } + } + + return 0; +} + Index: qemu/block.c === --- qemu.orig/block.c +++ qemu/block.c @@ -651,6 +651,33 @@ unlink_and_fail: return ret; } +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) +{ + BlockDriver *drv = bs->drv; + int ret = 0; + + /* No need to reopen as no change in flags */ + if (bdrv_flags == bs->open_flags) + return 0; + + /* Quiesce IO for the given block device */ + qemu_aio_flush(); + bdrv_flush(bs); + + bdrv_close(bs); + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); + + /* + * A failed attempt to reopen the image file must lead to 'abort()' + */ + if (ret != 0) { + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); + abort(); + } + + return ret; +} + void bdrv_close(BlockDriverState *bs) { if (bs->drv) { @@ -691,6 +718,20 @@ void bdrv_close_all(void) } } +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache) +{ + int bdrv_flags = bs->open_flags; + +
[Qemu-devel] [V3 2/3] Error classes for file reopen and device insertion
New error classes defined for cases where device not inserted and file reopen failed. Signed-off-by: Supriya Kannery --- qerror.c |8 qerror.h |6 ++ 2 files changed, 14 insertions(+) Index: qemu/qerror.c === --- qemu.orig/qerror.c +++ qemu/qerror.c @@ -96,6 +96,14 @@ static const QErrorStringTable qerror_ta .error_fmt = QERR_DEVICE_NOT_REMOVABLE, .desc = "Device '%(device)' is not removable", }, + { + .error_fmt = QERR_DEVICE_NOT_INSERTED, + .desc = "Device '%(device)' has not been inserted", + }, + { + .error_fmt = QERR_REOPEN_FILE_FAILED, + .desc = "Could not reopen '%(filename)'", + }, { .error_fmt = QERR_DEVICE_NO_BUS, .desc = "Device '%(device)' has no child bus", Index: qemu/qerror.h === --- qemu.orig/qerror.h +++ qemu/qerror.h @@ -85,6 +85,9 @@ QError *qobject_to_qerror(const QObject #define QERR_DEVICE_NOT_FOUND \ "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }" +#define QERR_DEVICE_NOT_INSERTED \ + "{ 'class': 'DeviceNotInserted', 'data': { 'device': %s } }" + #define QERR_DEVICE_NOT_REMOVABLE \ "{ 'class': 'DeviceNotRemovable', 'data': { 'device': %s } }" @@ -139,6 +142,9 @@ QError *qobject_to_qerror(const QObject #define QERR_OPEN_FILE_FAILED \ "{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }" +#define QERR_REOPEN_FILE_FAILED \ + "{ 'class': 'ReopenFileFailed', 'data': { 'filename': %s } }" + #define QERR_PROPERTY_NOT_FOUND \ "{ 'class': 'PropertyNotFound', 'data': { 'device': %s, 'property': %s } }"
[Qemu-devel] [V2 3/3] Command "block_set" for dynamic block params change
New command "block_set" added for dynamically changing any of the block device parameters. For now, dynamic setting of hostcache params using this command is implemented. Other block device parameters, can be integrated in similar lines. Signed-off-by: Supriya Kannery --- block.c | 41 + block.h |2 ++ blockdev.c | 32 blockdev.h |1 + hmp-commands.hx | 15 +++ qmp-commands.hx | 30 +- 6 files changed, 120 insertions(+), 1 deletion(-) Index: qemu/hmp-commands.hx === --- qemu.orig/hmp-commands.hx +++ qemu/hmp-commands.hx @@ -70,6 +70,21 @@ but should be used with extreme caution. resizes image files, it can not resize block devices like LVM volumes. ETEXI + { + .name = "block_set", + .args_type = "device:B,paramname:s,enable:b", + .params = "device paramname enable", + .help = "On/Off block device parameters like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +STEXI +@item block_set +@findex block_set +Change block device params (eg:"hostcache"=on/off) while guest is running. +ETEXI + { .name = "eject", Index: qemu/qmp-commands.hx === --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -693,7 +693,35 @@ Example: EQMP -{ + { + .name = "block_set", + .args_type = "device:B,name:s,enable:b", + .params = "device name enable", + .help = "Enable/Disable block device params like hostcache", + .user_print = monitor_user_noop, + .mhandler.cmd_new = do_block_set, + }, + +SQMP +block_set +- + +Change various block device parameters like hostcache. + +Arguments: + +- "device": the device's ID, must be unique (json-string) +- "name": name of the parameter to be changed" (json-string) +- "enable": value to be set for the parameter, 'true' or 'false' (json-bool) + +Example: + +-> { "execute": "block_set", "arguments": { "device": "ide0-hd0", "name": "hostcache", "enable": true } } +<- { "return": {} } + +EQMP + + { .name = "balloon", .args_type = "value:M", .params = "target", Index: qemu/blockdev.c === --- qemu.orig/blockdev.c +++ qemu/blockdev.c @@ -797,3 +797,35 @@ int do_block_resize(Monitor *mon, const return 0; } + + +/* + * Handle changes to block device settings, like hostcache, + * while guest is running. +*/ +int do_block_set(Monitor *mon, const QDict *qdict, QObject **ret_data) +{ + const char *device = qdict_get_str(qdict, "device"); + const char *name = qdict_get_str(qdict, "name"); + int enable = qdict_get_bool(qdict, "enable"); + BlockDriverState *bs; + + bs = bdrv_find(device); + if (!bs) { + qerror_report(QERR_DEVICE_NOT_FOUND, device); + return -1; + } + + if (!(strcmp(name, "hostcache"))) { + if (bdrv_is_inserted(bs)) { + /* cache change applicable only if device inserted */ + return bdrv_change_hostcache(bs, enable); + } else { + qerror_report(QERR_DEVICE_NOT_INSERTED, device); + return -1; + } + } + + return 0; +} + Index: qemu/block.c === --- qemu.orig/block.c +++ qemu/block.c @@ -651,6 +651,33 @@ unlink_and_fail: return ret; } +int bdrv_reopen(BlockDriverState *bs, int bdrv_flags) +{ + BlockDriver *drv = bs->drv; + int ret = 0; + + /* No need to reopen as no change in flags */ + if (bdrv_flags == bs->open_flags) + return 0; + + /* Quiesce IO for the given block device */ + qemu_aio_flush(); + bdrv_flush(bs); + + bdrv_close(bs); + ret = bdrv_open(bs, bs->filename, bdrv_flags, drv); + + /* + * A failed attempt to reopen the image file must lead to 'abort()' + */ + if (ret != 0) { + qerror_report(QERR_REOPEN_FILE_FAILED, bs->filename); + abort(); + } + + return ret; +} + void bdrv_close(BlockDriverState *bs) { if (bs->drv) { @@ -691,6 +718,20 @@ void bdrv_close_all(void) } } +int bdrv_change_hostcache(BlockDriverState *bs, bool enable_host_cache) +{ + int bdrv_flags = bs->open_flags; + + /* set hostcache flags (without changing WCE/flush bits) */ + if (enable_host_cache) + bdrv_flags &= ~BDRV_O_NOCACHE; + else +
[Qemu-devel] [PATCH 8/9] PPC: 440: Use 440 style MMU as default, so Qemu knows the MMU type
We have some KVM interaction code in Qemu that tries to be clever and ignore some capabilities when running on BookE style MMUs. Unfortunately, the default CPU bamboo was defaulting to was not a BookE-style MMU, resulting in the check to fail. With this patch, guests can run again on 440 with -enable-kvm. Signed-off-by: Alexander Graf --- hw/ppc440.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/ppc440.c b/hw/ppc440.c index 1ed001a..90abc91 100644 --- a/hw/ppc440.c +++ b/hw/ppc440.c @@ -45,8 +45,9 @@ CPUState *ppc440ep_init(ram_addr_t *ram_size, PCIBus **pcip, qemu_irq *irqs; qemu_irq *pci_irqs; -if (cpu_model == NULL) -cpu_model = "405"; // XXX: should be 440EP +if (cpu_model == NULL) { +cpu_model = "440-Xilinx"; // XXX: should be 440EP +} env = cpu_init(cpu_model); if (!env) { fprintf(stderr, "Unable to initialize CPU!\n"); -- 1.6.0.2
[Qemu-devel] [V3 0/3]block: Dynamically change hostcache setting using new "block_set" command
Currently host page cache setting for a block device cannot be changed without restarting a running VM. Following patchset [V3] is for enabling dynamic change of hostcache setting for block devices through qemu monitor and QMP. Code changes are based on patches from Christoph Hellwig and Prerna Saxena. Changes from patchset V2: 1. Command "block_set" added for changing block device params dynamically 2. Enhanced info-block to display hostcache setting of block device 3. Added qmp interfaces for setting and querying hostcache New block command added: "block_set" -- Sets block device parameters while guest is running. Usage: block_set = block device = parameter (say, "hostcache") = on/off 1/3 Enhance "info block" to display hostcache setting 2/3 New error classes for file reopen and device insertion 3/3 Command "block_set" for dynamic params change for block device qemu/block.c | 62 + + qemu/block.h |2 ++ qemu/blockdev.c | 32 qemu/blockdev.h |1 + qemu/hmp-commands.hx | 15 +++ qemu/qerror.c|8 qemu/qerror.h|6 ++ qemu/qmp-commands.hx |2 ++ qmp-commands.hx | 30 +- 9 files changed, 153 insertions(+), 5 deletions(-) ~ ~ ~ ~
Re: [Qemu-devel] how to verify virtio is being used?
Am 17.06.2011 13:39, schrieb al pat: > Hi Kevin, > > Thanks! > > Yes, one disk is visible in guest as sdb (partitioned to sdb1), > mounted and I write to it. > > The virtio disk is visible as /dev/vda, (partitioned to vda1), mounted > and I write to it. Then it's using virtio-blk. > Kernel log on guest - do you mean dmesg? > > I was trying to trace through the virt io calls to confirm. and > determine the invocation sequence. > > My lspci output: > 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) > 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] > 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] > 00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB > [Natoma/Triton II] (rev 01) > 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) > 00:02.0 VGA compatible controller: Cirrus Logic GD 5446 > 00:03.0 Ethernet controller: Qumranet, Inc. Unknown device 1000 > 00:04.0 RAM memory: Qumranet, Inc. Unknown device 1002 > 00:06.0 SCSI storage controller: Qumranet, Inc. Unknown device 1001 This unknown device 1001 is your virtio-blk disk. Kevin > > lspci -k --- to show kernel drivers associated with the device does > not work in the guest. > > Thanks > a > > > On Fri, Jun 17, 2011 at 7:15 AM, Kevin Wolf wrote: >> Am 16.06.2011 20:57, schrieb al pat: >>> I have posted this on kvm alias, but have not heard back. seeing some >>> inputs. >>> >>> seeking some pointers/guidance as to how to determine virtio is being >>> used... >>> >>> I configured a VM to use block device with if=virtio (create a 1GB >>> disk using dd I exported this disk to the VM and am now doing scp from >>> host to the >>> guest after creating partition/mkfs. >>> >>> I created another 1GB disk and export it as a IDE disk. I use the same >>> scp command from host to guest after creating partition/mkfs. >>> >>> I am trying to determine if my block IO is indeed using virtio in the >>> first case. >>> >>> Empirically, I observe that with if=virtio, the throughput is about >>> 30% more (in terms of mbps) and time taken is about 40% less than >>> for the case where I passed the disk as a IDE disk. >>> >>> My scp happens over virbr0 interface (and currently I am not concerned >>> if networking is using virtio) >>> >>> How do I confirm that virtio is being used? Are there any debugs that >>> I can enable to do that. >> >> Have a look at the guest kernel logs, lspci output or just at the device >> name: IDE disks are called /dev/sda etc. whereas virtio-blk disks are >> called /dev/vda etc. >> >> Kevin >>
[Qemu-devel] [V3 1/3] Enhance "info block" to display hostcache setting
Enhance "info block" to display hostcache setting for each block device. Example: (qemu) info block ide0-hd0: type=hd removable=0 file=../rhel6-32.qcow2 ro=0 drv=qcow2 encrypted=0 Enhanced to display "hostcache" setting: (qemu) info block ide0-hd0: type=hd removable=0 hostcache=true file=../rhel6-32.qcow2 ro=0 drv=qcow2 encrypted=0 Signed-off-by: Supriya Kannery --- block.c | 21 + qmp-commands.hx |2 ++ 2 files changed, 19 insertions(+), 4 deletions(-) Index: qemu/block.c === --- qemu.orig/block.c +++ qemu/block.c @@ -1694,6 +1694,14 @@ static void bdrv_print_dict(QObject *obj monitor_printf(mon, " locked=%d", qdict_get_bool(bs_dict, "locked")); } + if (qdict_haskey(bs_dict, "open_flags")) { + int open_flags = qdict_get_int(bs_dict, "open_flags"); + if (open_flags & BDRV_O_NOCACHE) + monitor_printf(mon, " hostcache=false"); + else + monitor_printf(mon, " hostcache=true"); + } + if (qdict_haskey(bs_dict, "inserted")) { QDict *qdict = qobject_to_qdict(qdict_get(bs_dict, "inserted")); @@ -1730,13 +1738,18 @@ void bdrv_info(Monitor *mon, QObject **r QObject *bs_obj; bs_obj = qobject_from_jsonf("{ 'device': %s, 'type': 'unknown', " -"'removable': %i, 'locked': %i }", -bs->device_name, bs->removable, -bs->locked); + "'removable': %i, 'locked': %i, " + "'hostcache': %s }", + bs->device_name, bs->removable, + bs->locked, + (bs->open_flags & BDRV_O_NOCACHE) ? + "false" : "true"); + + QDict *bs_dict = qobject_to_qdict(bs_obj); + qdict_put(bs_dict, "open_flags", qint_from_int(bs->open_flags)); if (bs->drv) { QObject *obj; -QDict *bs_dict = qobject_to_qdict(bs_obj); obj = qobject_from_jsonf("{ 'file': %s, 'ro': %i, 'drv': %s, " "'encrypted': %i }", Index: qemu/qmp-commands.hx === --- qemu.orig/qmp-commands.hx +++ qemu/qmp-commands.hx @@ -1070,6 +1070,7 @@ Each json-object contain the following: - Possible values: "unknown" - "removable": true if the device is removable, false otherwise (json-bool) - "locked": true if the device is locked, false otherwise (json-bool) +- "hostcache": true if hostcache enabled, false otherwise (json-bool) - "inserted": only present if the device is inserted, it is a json-object containing the following: - "file": device file name (json-string) @@ -1091,6 +1092,7 @@ Example: { "device":"ide0-hd0", "locked":false, + "hostcache":false, "removable":false, "inserted":{ "ro":false,
Re: [Qemu-devel] Image streaming and live block copy
On Fri, Jun 17, 2011 at 10:36:21AM +0200, Kevin Wolf wrote: > Am 16.06.2011 16:52, schrieb Marcelo Tosatti: > > On Thu, Jun 16, 2011 at 03:08:30PM +0200, Kevin Wolf wrote: > >> Am 16.06.2011 14:49, schrieb Avi Kivity: > >>> On 06/16/2011 03:35 PM, Kevin Wolf wrote: > * Image streaming is a normal image file plus copy-on-read plus a > background task that copies data from the source image > >>> > >>> Or a block-mirror started in degraded mode. > >> > >> At least not in the same configuration as with live block copy: You > >> don't want to write to the source, you only want to read from it when > >> the destination doesn't have the data yet. > >> > * Live block copy is a block-mirror of two normal image files plus a > background task that copies data from the source image > >>> > >>> = block-mirror started in degraded mode > >> > The right solution is probably to implement COR and the background task > in generic block layer code (no reason to restrict it to QED) and use it > for both image streaming and live block copy. (This is a bit more > complicated than it may sound here because guest writes must always take > precedence over a copy - but doing complicated things is an even better > reason to do it in a common place instead of duplicating) > >>> > >>> Or in a block-mirror block format driver - generic code need not be > >>> involved. > >> > >> Might be an option. In this case generic code is only involved with the > >> stacking of BlockDriverStates, which is already implemented (but > >> requires -blockdev for a sane way to configure things). > >> > >> Kevin > > > > What are the disadvantages of such an approach for image streaming, > > versus the current QED approach? > > > > blkstream block driver: > > > > - Maintain in memory whether given block is allocated in local image, > > if not, read from remote, write to local. Set block as local. > > Local and remote simply two block drivers from image streaming driver > > POV. > > Why maintain it in memory? We already have mechanisms to track this in > COW image formats, so that you can even continue after a crash. > > We can still add a raw-cow driver that maintains the COW data in memory > for allowing raw copies, if this is needed. Well, then image streaming is not for generic-format anymore. OK, the uptodate information can live in disk if supported by the lower level format. > > - Once all blocks are local, notify mgmt so it can switch to local > > copy. > > - Writes are mirrored to source and destination, minding guest writes > > over copy writes. > > Image streaming shouldn't write to the source. But adding a flag for > this isn't a major problem. OK, block copy does write to the source. > > Over this scheme, you'd have: > > > > 1) Block copy. > > Reopen image to be copied with > > blkstream:/path/to/current-image:/path/to/destination-image, > > background read sectors 0...N. > > > > 2) Image stream: > > blkstream:remote-image:/path/to/local-image, > > background read sectors 0...N. > > > > Where remote-image is remote accessible image such as NBD. > > I think that should work. > > By the way, we'll get problems with the colon syntax. Without -blockdev > we'll have to invent a new syntax, maybe with brackets: > > blkstream:[nbd:localhost]:out.qcow2 > > Kevin
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On (Fri) 17 Jun 2011 [10:16:44], Luiz Capitulino wrote: > On Fri, 17 Jun 2011 12:17:36 +0530 > Amit Shah wrote: > > > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: > > > If I start qemu with: > > > > > > # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ > > > -device virtio-serial \ > > > -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ > > > -device virtserialport,chardev=foo,name=org.qemu.guest_agent > > > > > > I get a segfault when booting a Fedora 14 guest. The backtrace says: > > > > > > Program terminated with signal 11, Segmentation fault. > > > #0 0x00420850 in handle_control_message (vser=0x3732bd0, > > > buf=0x2c173e0, len=8) at > > > /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 > > > 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > > Strange, I've not seen it so far in my testing (neither in the daily > > test runs of the virtio-serial testsuite). > > > > > I've also bisected this and git points out to commit: > > > > > > commit a15bb0d6a981de749452a5180fc8084d625671da > > > Author: Markus Armbruster > > > Date: Wed May 25 14:21:13 2011 +0200 > > > > > > virtio-serial: Drop redundant VirtIOSerialPort member info > > > > > > I think what's happening is that the device is not initialized on a > > > VIRTIO_CONSOLE_DEVICE_READY event. Moving the DO_UPCAST() call to > > > the other events fixes the problem to me. > > > > > > Signed-off-by: Luiz Capitulino > > > --- > > > hw/virtio-serial-bus.c |4 ++-- > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > index 9a12104..579f676 100644 > > > --- a/hw/virtio-serial-bus.c > > > +++ b/hw/virtio-serial-bus.c > > > @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial > > > *vser, void *buf, size_t len) > > > if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > > > return; > > > > > > -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > - > > > > Ah - this missed the !port check. It should be possible to do this in > > a 'if (port)' block instead of replicating in the individual case > > statements. > > > > Thanks for the debugging and patch; please update with the above and > > I'll apply it to the virtio-serial tree. > > What about moving the VIRTIO_CONSOLE_DEVICE_READY handling out of the > switch, like the patch below? This way the function is divided in a way > that related events are handled together. > > I'll implement your first suggestion if you don't like this... > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index 579f676..5f96245 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -325,19 +325,12 @@ static void handle_control_message(VirtIOSerial *vser, > void *buf, size_t len) > return; > } > > -cpkt.event = lduw_p(&gcpkt->event); > cpkt.value = lduw_p(&gcpkt->value); > - > -port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > -if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > -return; > - > -switch(cpkt.event) { > -case VIRTIO_CONSOLE_DEVICE_READY: > +cpkt.event = lduw_p(&gcpkt->event); > +if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) { > if (!cpkt.value) { > -error_report("virtio-serial-bus: Guest failure in adding device > %s\n", > - vser->bus.qbus.name); > -break; > +error_report("virtio-serial-bus: Guest failure in adding device > %s\n", vser->bus.qbus.name); > +return; The line split should remain -- else it goes beyond 80 chars. > } > /* > * The device is up, we can now tell the device about all the > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial *vser, > void *buf, size_t len) > QTAILQ_FOREACH(port, &vser->ports, next) { > send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); > } > -break; > +return; > +} Makes me think of one case (totally unrelated to what you found)where the guest can fool us: by sending multiple VIRTIO_CONSOLE_DEVICE_READY messages. > +port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > +assert(port != NULL); I doubt if assert is the right thing: if the guest sends bad data, we shouldn't just kill it. It's easier to ignore such data, and perhaps just log it. > + > +switch(cpkt.event) { > case VIRTIO_CONSOLE_PORT_READY: > if (!cpkt.value) { > error_report("virtio-serial-bus: Guest failure in adding port %u > for device %s\n", I'm fine with this approach. Amit
[Qemu-devel] [PATCH 7/9] PPC: E500: Use MAS registers instead of internal TLB representation
The natural format for e500 cores to do TLB manipulation with are the MAS registers. Instead of converting them into some internal representation and back again when the guest reads them, we can just keep the data identical to the way the guest passed it to us. The main advantage of this approach is that we're getting closer to being able to share MMU data with KVM using shared memory, so that we don't need to copy lots of MMU data back and forth all the time. For this to work however, another patch is required that gets rid of the TLB union, as that destroys our memory layout that needs to be identical with the kernel one. Signed-off-by: Alexander Graf --- hw/ppce500_mpc8544ds.c | 21 --- target-ppc/cpu.h | 32 +++--- target-ppc/helper.c| 155 +--- target-ppc/op_helper.c | 148 ++ 4 files changed, 213 insertions(+), 143 deletions(-) diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c index 073de3c..b739ce2 100644 --- a/hw/ppce500_mpc8544ds.c +++ b/hw/ppce500_mpc8544ds.c @@ -185,18 +185,23 @@ out: } /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */ +static inline target_phys_addr_t booke206_page_size_to_tlb(uint64_t size) +{ +return (ffs(size >> 10) - 1) >> 1; +} + static void mmubooke_create_initial_mapping(CPUState *env, target_ulong va, target_phys_addr_t pa) { -ppcemb_tlb_t *tlb = booke206_get_tlbe(env, 1, 0, 0); - -tlb->attr = 0; -tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4); -tlb->size = 256 * 1024 * 1024; -tlb->EPN = va & TARGET_PAGE_MASK; -tlb->RPN = pa & TARGET_PAGE_MASK; -tlb->PID = 0; +ppcmas_tlb_t *tlb = booke206_get_tlbm(env, 1, 0, 0); +target_phys_addr_t size; + +size = (booke206_page_size_to_tlb(256 * 1024 * 1024) << MAS1_TSIZE_SHIFT); +tlb->mas1 = MAS1_VALID | size; +tlb->mas2 = va & TARGET_PAGE_MASK; +tlb->mas7_3 = pa & TARGET_PAGE_MASK; +tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX; } static void mpc8544ds_cpu_reset(void *opaque) diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 8e4582f..758c554 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -360,9 +360,17 @@ struct ppcemb_tlb_t { uint32_t attr; /* Storage attributes */ }; +typedef struct ppcmas_tlb_t { + uint32_t mas8; + uint32_t mas1; + uint64_t mas2; + uint64_t mas7_3; +} ppcmas_tlb_t; + union ppc_tlb_t { ppc6xx_tlb_t tlb6; ppcemb_tlb_t tlbe; +ppcmas_tlb_t tlbm; }; #endif @@ -1075,9 +1083,13 @@ void store_40x_sler (CPUPPCState *env, uint32_t val); void store_booke_tcr (CPUPPCState *env, target_ulong val); void store_booke_tsr (CPUPPCState *env, target_ulong val); void booke206_flush_tlb(CPUState *env, int flags, const int check_iprot); +target_phys_addr_t booke206_tlb_to_page_size(CPUState *env, ppcmas_tlb_t *tlb); int ppcemb_tlb_check(CPUState *env, ppcemb_tlb_t *tlb, target_phys_addr_t *raddrp, target_ulong address, uint32_t pid, int ext, int i); +int ppcmas_tlb_check(CPUState *env, ppcmas_tlb_t *tlb, + target_phys_addr_t *raddrp, target_ulong address, + uint32_t pid); void ppc_tlb_invalidate_all (CPUPPCState *env); void ppc_tlb_invalidate_one (CPUPPCState *env, target_ulong addr); #if defined(TARGET_PPC64) @@ -1927,12 +1939,12 @@ static inline void cpu_set_tls(CPUState *env, target_ulong newtls) } #if !defined(CONFIG_USER_ONLY) -static inline int booke206_tlbe_id(CPUState *env, ppcemb_tlb_t *tlbe) +static inline int booke206_tlbm_id(CPUState *env, ppcmas_tlb_t *tlbm) { -uintptr_t tlbel = (uintptr_t)tlbe; +uintptr_t tlbml = (uintptr_t)tlbm; uintptr_t tlbl = (uintptr_t)env->tlb; -return (tlbel - tlbl) / sizeof(env->tlb[0]); +return (tlbml - tlbl) / sizeof(env->tlb[0]); } static inline int booke206_tlb_size(CPUState *env, int tlbn) @@ -1949,9 +1961,9 @@ static inline int booke206_tlb_ways(CPUState *env, int tlbn) return r; } -static inline int booke206_tlbe_to_tlbn(CPUState *env, ppcemb_tlb_t *tlbe) +static inline int booke206_tlbm_to_tlbn(CPUState *env, ppcmas_tlb_t *tlbm) { -int id = booke206_tlbe_id(env, tlbe); +int id = booke206_tlbm_id(env, tlbm); int end = 0; int i; @@ -1966,14 +1978,14 @@ static inline int booke206_tlbe_to_tlbn(CPUState *env, ppcemb_tlb_t *tlbe) return 0; } -static inline int booke206_tlbe_to_way(CPUState *env, ppcemb_tlb_t *tlb) +static inline int booke206_tlbm_to_way(CPUState *env, ppcmas_tlb_t *tlb) { -int tlbn = booke206_tlbe_to_tlbn(env, tlb); -int tlbid = booke206_tlbe_id(env, tlb); +int tlbn = booke206_tlbm_to_tlbn(env, tlb); +int tlbid = booke206_tlbm_id(env, tlb); return tlbid & (booke206_tlb_ways(env, tlbn) - 1); } -static i
Re: [Qemu-devel] Image streaming and live block copy
On Thu, Jun 16, 2011 at 04:30:18PM +0100, Stefan Hajnoczi wrote: > On Thu, Jun 16, 2011 at 11:52:43AM -0300, Marcelo Tosatti wrote: > This approach does not use the backing file feature? > > > blkstream block driver: > > > > - Maintain in memory whether given block is allocated in local image, > > if not, read from remote, write to local. Set block as local. > > Local and remote simply two block drivers from image streaming driver > > POV. > > - Once all blocks are local, notify mgmt so it can switch to local > > copy. > > - Writes are mirrored to source and destination, minding guest writes > > over copy writes. > > We open the remote file read-only for image streaming and do not want to > mirror writes. Why not? Is there any disadvantage of mirroring writes? > If QEMU crashes or there is a power failure we need to restart the > streaming process carefully - local blocks must not be overwritten. > Perhaps this is the tricky part. Under the proposed scheme, if QEMU crashes you'd restart streaming from zero. In that case, the remote image is consistent due to mirrored writes. That is one disadvantage of keeping the local/remote status of blocks in memory: in case of a crash you'd have to restart from zero. But this should be an uncommon case (and there is not much of an option for generic-format image streaming without keeping metadata). Do you see a problem with that? > > Over this scheme, you'd have: > > > > 1) Block copy. > > Reopen image to be copied with > > blkstream:/path/to/current-image:/path/to/destination-image, > > background read sectors 0...N. > > > > 2) Image stream: > > blkstream:remote-image:/path/to/local-image, > > background read sectors 0...N. > > Stefan
[Qemu-devel] [PATCH 3/9] PPC: calculate kernel, initrd, cmdline locations dynamically
During testing, I was generating a vmlinux binary that easily occupied more than 20MB of RAM. Since the current -kernel code loads the initrd at a fixed address behind the kernel, we were overwriting kernel data when the kernel got too big. To finally get rid of the issue, let's calculate the initrd and cmdline addresses relative to the kernel size, so we can have kernels and initrds that are as big as they want to - as long as they fit in RAM. Signed-off-by: Alexander Graf --- hw/ppc_mac.h |3 +-- hw/ppc_newworld.c | 15 +++ hw/ppc_oldworld.c | 15 +++ 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/hw/ppc_mac.h b/hw/ppc_mac.h index ea87593..68dade7 100644 --- a/hw/ppc_mac.h +++ b/hw/ppc_mac.h @@ -35,8 +35,7 @@ #define PROM_ADDR 0xfff0 #define KERNEL_LOAD_ADDR 0x0100 -#define CMDLINE_ADDR 0x027ff000 -#define INITRD_LOAD_ADDR 0x0280 +#define KERNEL_GAP 0x0010 #define ESCC_CLOCK 3686400 diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c index 86f1cfb..5bce709 100644 --- a/hw/ppc_newworld.c +++ b/hw/ppc_newworld.c @@ -120,6 +120,11 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr) return (addr & 0x0fff) + KERNEL_LOAD_ADDR; } +static target_phys_addr_t round_page(target_phys_addr_t addr) +{ +return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK; +} + /* PowerPC Mac99 hardware initialisation */ static void ppc_core99_init (ram_addr_t ram_size, const char *boot_device, @@ -134,7 +139,7 @@ static void ppc_core99_init (ram_addr_t ram_size, int unin_memory; int linux_boot, i; ram_addr_t ram_offset, bios_offset; -uint32_t kernel_base, initrd_base; +target_phys_addr_t kernel_base, initrd_base, cmdline_base = 0; long kernel_size, initrd_size; PCIBus *pci_bus; MacIONVRAMState *nvr; @@ -220,7 +225,7 @@ static void ppc_core99_init (ram_addr_t ram_size, } /* load initrd */ if (initrd_filename) { -initrd_base = INITRD_LOAD_ADDR; +initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP); initrd_size = load_image_targphys(initrd_filename, initrd_base, ram_size - initrd_base); if (initrd_size < 0) { @@ -228,9 +233,11 @@ static void ppc_core99_init (ram_addr_t ram_size, initrd_filename); exit(1); } +cmdline_base = round_page(initrd_base + initrd_size); } else { initrd_base = 0; initrd_size = 0; +cmdline_base = round_page(kernel_base + kernel_size + KERNEL_GAP); } ppc_boot_device = 'm'; } else { @@ -373,8 +380,8 @@ static void ppc_core99_init (ram_addr_t ram_size, fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, kernel_base); fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, kernel_size); if (kernel_cmdline) { -fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, CMDLINE_ADDR); -pstrcpy_targphys("cmdline", CMDLINE_ADDR, TARGET_PAGE_SIZE, kernel_cmdline); +fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, cmdline_base); +pstrcpy_targphys("cmdline", cmdline_base, TARGET_PAGE_SIZE, kernel_cmdline); } else { fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_CMDLINE, 0); } diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c index 75a3127..20cd8e1 100644 --- a/hw/ppc_oldworld.c +++ b/hw/ppc_oldworld.c @@ -59,6 +59,11 @@ static uint64_t translate_kernel_address(void *opaque, uint64_t addr) return (addr & 0x0fff) + KERNEL_LOAD_ADDR; } +static target_phys_addr_t round_page(target_phys_addr_t addr) +{ +return (addr + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK; +} + static void ppc_heathrow_init (ram_addr_t ram_size, const char *boot_device, const char *kernel_filename, @@ -71,7 +76,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size, qemu_irq *pic, **heathrow_irqs; int linux_boot, i; ram_addr_t ram_offset, bios_offset; -uint32_t kernel_base, initrd_base; +uint32_t kernel_base, initrd_base, cmdline_base = 0; int32_t kernel_size, initrd_size; PCIBus *pci_bus; MacIONVRAMState *nvr; @@ -157,7 +162,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size, } /* load initrd */ if (initrd_filename) { -initrd_base = INITRD_LOAD_ADDR; +initrd_base = round_page(kernel_base + kernel_size + KERNEL_GAP); initrd_size = load_image_targphys(initrd_filename, initrd_base, ram_size - initrd_base); if (initrd_size < 0) { @@ -165,9 +170,11 @@ static void ppc_heathrow_init (ram_addr_t ram_size, initrd_filename); exit(1); } +cmdline_base = round_page
Re: [Qemu-devel] [PATCH 09/14] exec: last_first_tb was only used in !ONLY_USER case
On 2 June 2011 12:53, Juan Quintela wrote: > Once there, use a better variable name. > > Signed-off-by: Juan Quintela Reviewed-by: Peter Maydell > --- > exec.c | 10 +++--- > 1 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/exec.c b/exec.c > index 8529390..4b1afec 100644 > --- a/exec.c > +++ b/exec.c > @@ -1208,12 +1208,16 @@ static inline void tb_alloc_page(TranslationBlock *tb, > unsigned int n, tb_page_addr_t page_addr) > { > PageDesc *p; > - TranslationBlock *last_first_tb; > +#ifndef CONFIG_USER_ONLY > + bool page_already_protected; > +#endif > > tb->page_addr[n] = page_addr; > p = page_find_alloc(page_addr >> TARGET_PAGE_BITS, 1); > tb->page_next[n] = p->first_tb; > - last_first_tb = p->first_tb; > +#ifndef CONFIG_USER_ONLY > + page_already_protected = p->first_tb != NULL; > +#endif > p->first_tb = (TranslationBlock *)((long)tb | n); > invalidate_page_bitmap(p); > > @@ -1249,7 +1253,7 @@ static inline void tb_alloc_page(TranslationBlock *tb, > /* if some code is already present, then the pages are already > protected. So we handle the case where only the first TB is > allocated in a physical page */ > - if (!last_first_tb) { > + if (!page_already_protected) { > tlb_protect_code(page_addr); > } > #endif > -- > 1.7.5.2 > > > -- 12345678901234567890123456789012345678901234567890123456789012345678901234567890 1 2 3 4 5 6 7 8
[Qemu-devel] [PATCH 4/9] PPC: mpc8544ds: Add hypervisor node
When running a PPC guest with KVM that can do PV operations, we need to indicate the guest which instructions to use for a hypercall and that it is running as KVM guest. This logic was available on openbios based machines already. This patch also adds said functionality to the mpc8544ds machine. Signed-off-by: Alexander Graf Acked-by: Scott Wood --- hw/ppce500_mpc8544ds.c | 21 +++-- pc-bios/mpc8544ds.dtb | Bin 2257 -> 2277 bytes pc-bios/mpc8544ds.dts |3 +++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c index 3ba8e75..073de3c 100644 --- a/hw/ppce500_mpc8544ds.c +++ b/hw/ppce500_mpc8544ds.c @@ -82,11 +82,12 @@ out: } #endif -static int mpc8544_load_device_tree(target_phys_addr_t addr, - uint32_t ramsize, - target_phys_addr_t initrd_base, - target_phys_addr_t initrd_size, - const char *kernel_cmdline) +static int mpc8544_load_device_tree(CPUState *env, +target_phys_addr_t addr, +uint32_t ramsize, +target_phys_addr_t initrd_base, +target_phys_addr_t initrd_size, +const char *kernel_cmdline) { int ret = -1; #ifdef CONFIG_FDT @@ -94,6 +95,7 @@ static int mpc8544_load_device_tree(target_phys_addr_t addr, char *filename; int fdt_size; void *fdt; +uint8_t hypercall[16]; filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE); if (!filename) { @@ -157,6 +159,13 @@ static int mpc8544_load_device_tree(target_phys_addr_t addr, mpc8544_copy_soc_cell(fdt, buf, "clock-frequency"); mpc8544_copy_soc_cell(fdt, buf, "timebase-frequency"); + +/* indicate KVM hypercall interface */ +qemu_devtree_setprop_string(fdt, "/hypervisor", "compatible", +"linux,kvm"); +kvmppc_get_hypercall(env, hypercall, sizeof(hypercall)); +qemu_devtree_setprop(fdt, "/hypervisor", "hcall-instructions", + hypercall, sizeof(hypercall)); } else { const uint32_t freq = 4; @@ -330,7 +339,7 @@ static void mpc8544ds_init(ram_addr_t ram_size, cpu_abort(env, "Compiled without FDT support - can't load kernel\n"); #endif dt_base = (kernel_size + DTC_LOAD_PAD) & ~DTC_PAD_MASK; -if (mpc8544_load_device_tree(dt_base, ram_size, +if (mpc8544_load_device_tree(env, dt_base, ram_size, initrd_base, initrd_size, kernel_cmdline) < 0) { fprintf(stderr, "couldn't load device tree\n"); exit(1); diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb index 189224e5875e9dd0d9195c22624b85bfb29dd820..ae318b1fe83846cc2e133951a3666fcfcdf87f79 100644 GIT binary patch delta 47 zcmca8_*78f0`I@K3=AAk85kHW7#P?qCJKl%I&9Q1XXj?js4Pe=D$6X+FWMZ&Ud;#q DUc3%i delta 35 rcmaDVcu`Q`0`I@K3=A9>85kHW7#P@7CJKl%>TJ|7XWv}RUc?9h$W95# diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts index fd792d5..a88b47c 100644 --- a/pc-bios/mpc8544ds.dts +++ b/pc-bios/mpc8544ds.dts @@ -125,4 +125,7 @@ chosen { linux,stdout-path = "/soc8544@e000/serial@4500"; }; + + hypervisor { + }; }; -- 1.6.0.2
[Qemu-devel] [PATCH 9/9] PPC: move TLBs to their own arrays
Until now, we've created a union over multiple different TLB types and allocated that union. While it's a waste of memory (and cache) to allocate TLB information for a TLB type with much information when you only need little, it also inflicts another issue. With the new KVM API, we can now share the TLB between KVM and qemu, but for that to work we need to have both be in the same layout. We can't just stretch it over to fit some internal different TLB representation. Hence this patch moves all TLB types to their own array, allowing us to only address and allocate exactly the boundaries required for the specific TLB type at hand. Signed-off-by: Alexander Graf --- hw/virtex_ml507.c |4 ++-- target-ppc/cpu.h| 21 ++--- target-ppc/helper.c | 26 +- target-ppc/machine.c| 16 target-ppc/op_helper.c | 12 ++-- target-ppc/translate_init.c | 27 ++- 6 files changed, 69 insertions(+), 37 deletions(-) diff --git a/hw/virtex_ml507.c b/hw/virtex_ml507.c index fa60515..7bde8c7 100644 --- a/hw/virtex_ml507.c +++ b/hw/virtex_ml507.c @@ -60,7 +60,7 @@ static void mmubooke_create_initial_mapping(CPUState *env, target_ulong va, target_phys_addr_t pa) { -ppcemb_tlb_t *tlb = &env->tlb[0].tlbe; +ppcemb_tlb_t *tlb = &env->tlb.tlbe[0]; tlb->attr = 0; tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4); @@ -69,7 +69,7 @@ static void mmubooke_create_initial_mapping(CPUState *env, tlb->RPN = pa & TARGET_PAGE_MASK; tlb->PID = 0; -tlb = &env->tlb[1].tlbe; +tlb = &env->tlb.tlbe[1]; tlb->attr = 0; tlb->prot = PAGE_VALID | ((PAGE_READ | PAGE_WRITE | PAGE_EXEC) << 4); tlb->size = 1 << 31; /* up to 0x */ diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 758c554..46d86be 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -368,10 +368,16 @@ typedef struct ppcmas_tlb_t { } ppcmas_tlb_t; union ppc_tlb_t { -ppc6xx_tlb_t tlb6; -ppcemb_tlb_t tlbe; -ppcmas_tlb_t tlbm; +ppc6xx_tlb_t *tlb6; +ppcemb_tlb_t *tlbe; +ppcmas_tlb_t *tlbm; }; + +/* possible TLB variants */ +#define TLB_NONE 0 +#define TLB_6XX1 +#define TLB_EMB2 +#define TLB_MAS3 #endif #define SDR_32_HTABORG 0xUL @@ -911,7 +917,8 @@ struct CPUPPCState { int last_way;/* Last used way used to allocate TLB in a LRU way */ int id_tlbs; /* If 1, MMU has separated TLBs for instructions & data */ int nb_pids; /* Number of available PID registers*/ -ppc_tlb_t *tlb; /* TLB is optional. Allocate them only if needed*/ +int tlb_type;/* Type of TLB we're dealing with */ +ppc_tlb_t tlb; /* TLB is optional. Allocate them only if needed*/ /* 403 dedicated access protection registers */ target_ulong pb[4]; #endif @@ -1942,9 +1949,9 @@ static inline void cpu_set_tls(CPUState *env, target_ulong newtls) static inline int booke206_tlbm_id(CPUState *env, ppcmas_tlb_t *tlbm) { uintptr_t tlbml = (uintptr_t)tlbm; -uintptr_t tlbl = (uintptr_t)env->tlb; +uintptr_t tlbl = (uintptr_t)env->tlb.tlbm; -return (tlbml - tlbl) / sizeof(env->tlb[0]); +return (tlbml - tlbl) / sizeof(env->tlb.tlbm[0]); } static inline int booke206_tlb_size(CPUState *env, int tlbn) @@ -2004,7 +2011,7 @@ static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int tlbn, r += booke206_tlb_size(env, i); } -return &env->tlb[r].tlbm; +return &env->tlb.tlbm[r]; } #endif diff --git a/target-ppc/helper.c b/target-ppc/helper.c index 8cf9ee1..3884976 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -323,7 +323,7 @@ static inline void ppc6xx_tlb_invalidate_all(CPUState *env) if (env->id_tlbs == 1) max *= 2; for (nr = 0; nr < max; nr++) { -tlb = &env->tlb[nr].tlb6; +tlb = &env->tlb.tlb6[nr]; pte_invalidate(&tlb->pte0); } tlb_flush(env, 1); @@ -340,7 +340,7 @@ static inline void __ppc6xx_tlb_invalidate_virt(CPUState *env, /* Invalidate ITLB + DTLB, all ways */ for (way = 0; way < env->nb_ways; way++) { nr = ppc6xx_tlb_getnum(env, eaddr, way, is_code); -tlb = &env->tlb[nr].tlb6; +tlb = &env->tlb.tlb6[nr]; if (pte_is_valid(tlb->pte0) && (match_epn == 0 || eaddr == tlb->EPN)) { LOG_SWTLB("TLB invalidate %d/%d " TARGET_FMT_lx "\n", nr, env->nb_tlb, eaddr); @@ -367,7 +367,7 @@ void ppc6xx_tlb_store (CPUState *env, target_ulong EPN, int way, int is_code, int nr; nr = ppc6xx_tlb_getnum(env, EPN, way, is_code); -tlb = &env->tlb[nr].tlb6; +tlb = &env->tlb.tlb6[nr]; LOG_SWTLB("Set TLB %d/
[Qemu-devel] [PATCH v4 2/2] Add support for Zipit Z2 machine
Zipit Z2 is small PXA270 based handheld. Signed-off-by: Vasily Khoruzhick --- v2: codestyle fixes, added VMStateDescription for LCD device and AER915, traces clean up. v3: no changes v4: no changes Makefile.target |1 + hw/z2.c | 352 +++ 2 files changed, 353 insertions(+), 0 deletions(-) create mode 100644 hw/z2.c diff --git a/Makefile.target b/Makefile.target index 602d50d..5750499 100644 --- a/Makefile.target +++ b/Makefile.target @@ -358,6 +358,7 @@ obj-arm-y += omap2.o omap_dss.o soc_dma.o omap_gptimer.o omap_synctimer.o \ obj-arm-y += omap_sx1.o palm.o tsc210x.o obj-arm-y += nseries.o blizzard.o onenand.o vga.o cbus.o tusb6010.o usb-musb.o obj-arm-y += mst_fpga.o mainstone.o +obj-arm-y += z2.o obj-arm-y += musicpal.o bitbang_i2c.o marvell_88w8618_audio.o obj-arm-y += framebuffer.o obj-arm-y += syborg.o syborg_fb.o syborg_interrupt.o syborg_keyboard.o diff --git a/hw/z2.c b/hw/z2.c new file mode 100644 index 000..3e3591a --- /dev/null +++ b/hw/z2.c @@ -0,0 +1,352 @@ +/* + * PXA270-based Zipit Z2 device + * + * Copyright (c) 2011 by Vasily Khoruzhick + * + * Code is based on mainstone platform. + * + * This code is licensed under the GNU GPL v2. + */ + +#include "hw.h" +#include "pxa.h" +#include "arm-misc.h" +#include "devices.h" +#include "i2c.h" +#include "ssi.h" +#include "boards.h" +#include "sysemu.h" +#include "flash.h" +#include "blockdev.h" +#include "console.h" +#include "audio/audio.h" + +#if 0 +#define DPRINTF(fmt, ...) \ +printf(fmt, ## __VA_ARGS__) +#else +#define DPRINTF(fmt, ...) +#endif + +static struct keymap map[0x100] = { +[0 ... 0xff] = { -1, -1 }, +[0x3b] = {0, 0}, /* Option = F1 */ +[0xc8] = {0, 1}, /* Up */ +[0xd0] = {0, 2}, /* Down */ +[0xcb] = {0, 3}, /* Left */ +[0xcd] = {0, 4}, /* Right */ +[0xcf] = {0, 5}, /* End */ +[0x0d] = {0, 6}, /* KPPLUS */ +[0xc7] = {1, 0}, /* Home */ +[0x10] = {1, 1}, /* Q */ +[0x17] = {1, 2}, /* I */ +[0x22] = {1, 3}, /* G */ +[0x2d] = {1, 4}, /* X */ +[0x1c] = {1, 5}, /* Enter */ +[0x0c] = {1, 6}, /* KPMINUS */ +[0xc9] = {2, 0}, /* PageUp */ +[0x11] = {2, 1}, /* W */ +[0x18] = {2, 2}, /* O */ +[0x23] = {2, 3}, /* H */ +[0x2e] = {2, 4}, /* C */ +[0x38] = {2, 5}, /* LeftAlt */ +[0xd1] = {3, 0}, /* PageDown */ +[0x12] = {3, 1}, /* E */ +[0x19] = {3, 2}, /* P */ +[0x24] = {3, 3}, /* J */ +[0x2f] = {3, 4}, /* V */ +[0x2a] = {3, 5}, /* LeftShift */ +[0x01] = {4, 0}, /* Esc */ +[0x13] = {4, 1}, /* R */ +[0x1e] = {4, 2}, /* A */ +[0x25] = {4, 3}, /* K */ +[0x30] = {4, 4}, /* B */ +[0x1d] = {4, 5}, /* LeftCtrl */ +[0x0f] = {5, 0}, /* Tab */ +[0x14] = {5, 1}, /* T */ +[0x1f] = {5, 2}, /* S */ +[0x26] = {5, 3}, /* L */ +[0x31] = {5, 4}, /* N */ +[0x39] = {5, 5}, /* Space */ +[0x3c] = {6, 0}, /* Stop = F2 */ +[0x15] = {6, 1}, /* Y */ +[0x20] = {6, 2}, /* D */ +[0x0e] = {6, 3}, /* Backspace */ +[0x32] = {6, 4}, /* M */ +[0x33] = {6, 5}, /* Comma */ +[0x3d] = {7, 0}, /* Play = F3 */ +[0x16] = {7, 1}, /* U */ +[0x21] = {7, 2}, /* F */ +[0x2c] = {7, 3}, /* Z */ +[0x27] = {7, 4}, /* Semicolon */ +[0x34] = {7, 5}, /* Dot */ +}; + +#define Z2_RAM_SIZE 0x0200 +#define Z2_FLASH_BASE 0x +#define Z2_FLASH_SIZE 0x0080 + +static struct arm_boot_info z2_binfo = { +.loader_start = PXA2XX_SDRAM_BASE, +.ram_size = Z2_RAM_SIZE, +}; + +#define Z2_GPIO_SD_DETECT 96 +#define Z2_GPIO_AC_IN 0 +#define Z2_GPIO_KEY_ON 1 +#define Z2_GPIO_LCD_CS 88 + +typedef struct { +SSISlave ssidev; +int32_t selected; +int32_t enabled; +uint8_t buf[3]; +uint32_t cur_reg; +int pos; +} ZipitLCD; + +static uint32_t zipit_lcd_transfer(SSISlave *dev, uint32_t value) +{ +ZipitLCD *z = FROM_SSI_SLAVE(ZipitLCD, dev); +uint16_t val; +if (z->selected) { +z->buf[z->pos] = value & 0xff; +z->pos++; +} +if (z->pos == 3) { +switch (z->buf[0]) { +case 0x74: +DPRINTF("%s: reg: 0x%.2x\n", __func__, z->buf[2]); +z->cur_reg = z->buf[2]; +break; +case 0x76: +val = z->buf[1] << 8 | z->buf[2]; +DPRINTF("%s: value: 0x%.4x\n", __func__, val); +if (z->cur_reg == 0x22 && val == 0x) { +z->enabled = 1; +printf("%s: LCD enabled\n", __func__); +} else if (z->cur_reg == 0x10 && val == 0x) { +z->enabled = 0; +printf("%s: LCD disabled\n", __func__); +} +break; +default: +fprintf(stderr, "%s: unknown command!\n", __func__); +break; +} +z->pos = 0; +} +return 0; +} + +static void z2_lcd_cs(void *opaque, int line, int level) +{ +ZipitLCD *z2_lcd = opaque; +z2_lcd->
[Qemu-devel] [PATCH 2/9] target-ppc: Handle memory-forced I/O controller access
From: Hervé Poussineau On at least the PowerPC 601, a direct-store (T=1) with bus unit ID 0x07F is special-cased as memory-forced I/O controller access. It is supposed to be checked immediately if T=1, bypassing all protection mechanisms and acting cache-inhibited and global. Signed-off-by: Hervé Poussineau Simplified by avoiding reindentation. Added explanatory comments. Cc: Alexander Graf Signed-off-by: Andreas Färber Signed-off-by: Alexander Graf --- target-ppc/helper.c | 16 1 files changed, 16 insertions(+), 0 deletions(-) diff --git a/target-ppc/helper.c b/target-ppc/helper.c index cf2a368..2944b06 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -949,8 +949,24 @@ static inline int get_segment(CPUState *env, mmu_ctx_t *ctx, ret = -3; } } else { +target_ulong sr; LOG_MMU("direct store...\n"); /* Direct-store segment : absolutely *BUGGY* for now */ + +/* Direct-store implies a 32-bit MMU. + * Check the Segment Register's bus unit ID (BUID). + */ +sr = env->sr[eaddr >> 28]; +if ((sr & 0x1FF0) >> 20 == 0x07f) { +/* Memory-forced I/O controller interface access */ +/* If T=1 and BUID=x'07F', the 601 performs a memory access + * to SR[28-31] LA[4-31], bypassing all protection mechanisms. + */ +ctx->raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFF); +ctx->prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; +return 0; +} + switch (type) { case ACCESS_INT: /* Integer load/store : only access allowed */ -- 1.6.0.2
[Qemu-devel] [PATCH 0/9] PPC patch queue 2011-06-17
The following changes since commit eb47d7c5d96060040931c42773ee07e61e547af9: Peter Maydell (1): hw/9118.c: Implement active-low interrupt support are available in the git repository at: git://repo.or.cz/qemu/agraf.git ppc-next Alexander Graf (8): PPC: E500: Implement reboot controller PPC: calculate kernel,initrd,cmdline locations dynamically PPC: mpc8544ds: Add hypervisor node PPC: update openbios firmware PPC: Only set lower 32bits with mtmsr PPC: E500: Use MAS registers instead of internal TLB representation PPC: 440: Use 440 style MMU as default, so Qemu knows the MMU type PPC: move TLBs to their own arrays Hervé Poussineau (1): target-ppc: Handle memory-forced I/O controller access Makefile.target |2 +- hw/mpc8544_guts.c | 135 ++ hw/ppc440.c |5 +- hw/ppc_mac.h|3 +- hw/ppc_newworld.c | 15 +++- hw/ppc_oldworld.c | 15 +++- hw/ppce500_mpc8544ds.c | 46 +++--- hw/virtex_ml507.c |4 +- pc-bios/README |6 +- pc-bios/mpc8544ds.dtb | Bin 12288 -> 2277 bytes pc-bios/mpc8544ds.dts |9 ++ pc-bios/openbios-ppc| Bin 729876 -> 750392 bytes target-ppc/cpu.h| 47 --- target-ppc/helper.c | 193 ++- target-ppc/machine.c| 16 ++-- target-ppc/op_helper.c | 160 +--- target-ppc/translate.c | 17 ++--- target-ppc/translate_init.c | 27 ++- 18 files changed, 493 insertions(+), 207 deletions(-) create mode 100644 hw/mpc8544_guts.c
[Qemu-devel] [PATCH 6/9] PPC: Only set lower 32bits with mtmsr
As Nathan pointed out correctly, the mtmsr instruction does not modify the high 32 bits of MSR. It also doesn't matter if SF is set or not, the instruction always behaves the same. This patch moves it a bit closer to the spec. Reported-by: Nathan Whitehorn Signed-off-by: Alexander Graf --- target-ppc/translate.c | 17 ++--- 1 files changed, 6 insertions(+), 11 deletions(-) diff --git a/target-ppc/translate.c b/target-ppc/translate.c index 59aef85..7e318e3 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -3878,24 +3878,19 @@ static void gen_mtmsr(DisasContext *ctx) tcg_gen_or_tl(cpu_msr, cpu_msr, t0); tcg_temp_free(t0); } else { +TCGv msr = tcg_temp_new(); + /* XXX: we need to update nip before the store * if we enter power saving mode, we will exit the loop * directly from ppc_store_msr */ gen_update_nip(ctx, ctx->nip); #if defined(TARGET_PPC64) -if (!ctx->sf_mode) { -TCGv t0 = tcg_temp_new(); -TCGv t1 = tcg_temp_new(); -tcg_gen_andi_tl(t0, cpu_msr, 0xULL); -tcg_gen_ext32u_tl(t1, cpu_gpr[rS(ctx->opcode)]); -tcg_gen_or_tl(t0, t0, t1); -tcg_temp_free(t1); -gen_helper_store_msr(t0); -tcg_temp_free(t0); -} else +tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32); +#else +tcg_gen_mov_tl(msr, cpu_gpr[rS(ctx->opcode)]); #endif -gen_helper_store_msr(cpu_gpr[rS(ctx->opcode)]); +gen_helper_store_msr(msr); /* Must stop the translation as machine state (may have) changed */ /* Note that mtmsr is not always defined as context-synchronizing */ gen_stop_exception(ctx); -- 1.6.0.2
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On Fri, 17 Jun 2011 10:16:44 -0300 Luiz Capitulino wrote: > On Fri, 17 Jun 2011 12:17:36 +0530 > Amit Shah wrote: > > > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: > > > If I start qemu with: > > > > > > # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ > > > -device virtio-serial \ > > > -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ > > > -device virtserialport,chardev=foo,name=org.qemu.guest_agent > > > > > > I get a segfault when booting a Fedora 14 guest. The backtrace says: > > > > > > Program terminated with signal 11, Segmentation fault. > > > #0 0x00420850 in handle_control_message (vser=0x3732bd0, > > > buf=0x2c173e0, len=8) at > > > /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 > > > 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > > Strange, I've not seen it so far in my testing (neither in the daily > > test runs of the virtio-serial testsuite). > > > > > I've also bisected this and git points out to commit: > > > > > > commit a15bb0d6a981de749452a5180fc8084d625671da > > > Author: Markus Armbruster > > > Date: Wed May 25 14:21:13 2011 +0200 > > > > > > virtio-serial: Drop redundant VirtIOSerialPort member info > > > > > > I think what's happening is that the device is not initialized on a > > > VIRTIO_CONSOLE_DEVICE_READY event. Moving the DO_UPCAST() call to > > > the other events fixes the problem to me. > > > > > > Signed-off-by: Luiz Capitulino > > > --- > > > hw/virtio-serial-bus.c |4 ++-- > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > > index 9a12104..579f676 100644 > > > --- a/hw/virtio-serial-bus.c > > > +++ b/hw/virtio-serial-bus.c > > > @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial > > > *vser, void *buf, size_t len) > > > if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > > > return; > > > > > > -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > - > > > > Ah - this missed the !port check. It should be possible to do this in > > a 'if (port)' block instead of replicating in the individual case > > statements. > > > > Thanks for the debugging and patch; please update with the above and > > I'll apply it to the virtio-serial tree. > > What about moving the VIRTIO_CONSOLE_DEVICE_READY handling out of the > switch, like the patch below? This way the function is divided in a way > that related events are handled together. > > I'll implement your first suggestion if you don't like this... > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > index 579f676..5f96245 100644 > --- a/hw/virtio-serial-bus.c > +++ b/hw/virtio-serial-bus.c > @@ -325,19 +325,12 @@ static void handle_control_message(VirtIOSerial *vser, > void *buf, size_t len) > return; > } > > -cpkt.event = lduw_p(&gcpkt->event); > cpkt.value = lduw_p(&gcpkt->value); > - > -port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > -if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > -return; > - > -switch(cpkt.event) { > -case VIRTIO_CONSOLE_DEVICE_READY: > +cpkt.event = lduw_p(&gcpkt->event); > +if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) { > if (!cpkt.value) { > -error_report("virtio-serial-bus: Guest failure in adding device > %s\n", > - vser->bus.qbus.name); > -break; > +error_report("virtio-serial-bus: Guest failure in adding device > %s\n", vser->bus.qbus.name); > +return; > } > /* > * The device is up, we can now tell the device about all the > @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial *vser, > void *buf, size_t len) > QTAILQ_FOREACH(port, &vser->ports, next) { > send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); > } > -break; > +return; > +} > > +port = find_port_by_id(vser, ldl_p(&gcpkt->id)); > +assert(port != NULL); I did this on top of my first patch and forgot to move the DO_UPCAST() call here... But you got the idea :) > + > +switch(cpkt.event) { > case VIRTIO_CONSOLE_PORT_READY: > if (!cpkt.value) { > error_report("virtio-serial-bus: Guest failure in adding port %u > for device %s\n",
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On Fri, 17 Jun 2011 12:17:36 +0530 Amit Shah wrote: > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: > > If I start qemu with: > > > > # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ > > -device virtio-serial \ > > -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ > > -device virtserialport,chardev=foo,name=org.qemu.guest_agent > > > > I get a segfault when booting a Fedora 14 guest. The backtrace says: > > > > Program terminated with signal 11, Segmentation fault. > > #0 0x00420850 in handle_control_message (vser=0x3732bd0, > > buf=0x2c173e0, len=8) at > > /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 > > 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > Strange, I've not seen it so far in my testing (neither in the daily > test runs of the virtio-serial testsuite). > > > I've also bisected this and git points out to commit: > > > > commit a15bb0d6a981de749452a5180fc8084d625671da > > Author: Markus Armbruster > > Date: Wed May 25 14:21:13 2011 +0200 > > > > virtio-serial: Drop redundant VirtIOSerialPort member info > > > > I think what's happening is that the device is not initialized on a > > VIRTIO_CONSOLE_DEVICE_READY event. Moving the DO_UPCAST() call to > > the other events fixes the problem to me. > > > > Signed-off-by: Luiz Capitulino > > --- > > hw/virtio-serial-bus.c |4 ++-- > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > > index 9a12104..579f676 100644 > > --- a/hw/virtio-serial-bus.c > > +++ b/hw/virtio-serial-bus.c > > @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial *vser, > > void *buf, size_t len) > > if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > > return; > > > > -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > - > > Ah - this missed the !port check. It should be possible to do this in > a 'if (port)' block instead of replicating in the individual case > statements. > > Thanks for the debugging and patch; please update with the above and > I'll apply it to the virtio-serial tree. What about moving the VIRTIO_CONSOLE_DEVICE_READY handling out of the switch, like the patch below? This way the function is divided in a way that related events are handled together. I'll implement your first suggestion if you don't like this... diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c index 579f676..5f96245 100644 --- a/hw/virtio-serial-bus.c +++ b/hw/virtio-serial-bus.c @@ -325,19 +325,12 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len) return; } -cpkt.event = lduw_p(&gcpkt->event); cpkt.value = lduw_p(&gcpkt->value); - -port = find_port_by_id(vser, ldl_p(&gcpkt->id)); -if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) -return; - -switch(cpkt.event) { -case VIRTIO_CONSOLE_DEVICE_READY: +cpkt.event = lduw_p(&gcpkt->event); +if (cpkt.event == VIRTIO_CONSOLE_DEVICE_READY) { if (!cpkt.value) { -error_report("virtio-serial-bus: Guest failure in adding device %s\n", - vser->bus.qbus.name); -break; +error_report("virtio-serial-bus: Guest failure in adding device %s\n", vser->bus.qbus.name); +return; } /* * The device is up, we can now tell the device about all the @@ -346,8 +339,13 @@ static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len) QTAILQ_FOREACH(port, &vser->ports, next) { send_control_event(port, VIRTIO_CONSOLE_PORT_ADD, 1); } -break; +return; +} +port = find_port_by_id(vser, ldl_p(&gcpkt->id)); +assert(port != NULL); + +switch(cpkt.event) { case VIRTIO_CONSOLE_PORT_READY: if (!cpkt.value) { error_report("virtio-serial-bus: Guest failure in adding port %u for device %s\n",
[Qemu-devel] [PATCH 1/9] PPC: E500: Implement reboot controller
When Linux reboots an e500 VM, it writes to a magic register in the "global-utilities" device indicated by the device tree. We were not emulating that device so far, rendering the VM reboot-less. This patch implements that device with only the reboot functionality implemented and adds it to the device tree. With this patch applied, I can successfully reboot a -M mpc8544ds VM. Signed-off-by: Alexander Graf Reviewed-by: Andreas Färber --- Makefile.target|2 +- hw/mpc8544_guts.c | 135 hw/ppce500_mpc8544ds.c |4 ++ pc-bios/mpc8544ds.dtb | Bin 12288 -> 2257 bytes pc-bios/mpc8544ds.dts |6 ++ 5 files changed, 146 insertions(+), 1 deletions(-) create mode 100644 hw/mpc8544_guts.c diff --git a/Makefile.target b/Makefile.target index b1a0f6d..d3ebe57 100644 --- a/Makefile.target +++ b/Makefile.target @@ -256,7 +256,7 @@ endif obj-ppc-y += ppc4xx_devs.o ppc4xx_pci.o ppc405_uc.o ppc405_boards.o obj-ppc-y += ppc440.o ppc440_bamboo.o # PowerPC E500 boards -obj-ppc-y += ppce500_mpc8544ds.o +obj-ppc-y += ppce500_mpc8544ds.o mpc8544_guts.o # PowerPC 440 Xilinx ML507 reference board. obj-ppc-y += virtex_ml507.o obj-ppc-$(CONFIG_KVM) += kvm_ppc.o diff --git a/hw/mpc8544_guts.c b/hw/mpc8544_guts.c new file mode 100644 index 000..c685f3e --- /dev/null +++ b/hw/mpc8544_guts.c @@ -0,0 +1,135 @@ +/* + * QEMU PowerPC MPC8544 global util pseudo-device + * + * Copyright (C) 2011 Freescale Semiconductor, Inc. All rights reserved. + * + * Author: Alexander Graf, + * + * This is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * * + * + * The documentation for this device is noted in the MPC8544 documentation, + * file name "MPC8544ERM.pdf". You can easily find it on the web. + * + */ + +#include "hw.h" +#include "sysemu.h" +#include "sysbus.h" + +#define MPC8544_GUTS_MMIO_SIZE0x1000 +#define MPC8544_GUTS_RSTCR_RESET 0x02 + +#define MPC8544_GUTS_ADDR_PORPLLSR0x00 +#define MPC8544_GUTS_ADDR_PORBMSR 0x04 +#define MPC8544_GUTS_ADDR_PORIMPSCR 0x08 +#define MPC8544_GUTS_ADDR_PORDEVSR0x0C +#define MPC8544_GUTS_ADDR_PORDBGMSR 0x10 +#define MPC8544_GUTS_ADDR_PORDEVSR2 0x14 +#define MPC8544_GUTS_ADDR_GPPORCR 0x20 +#define MPC8544_GUTS_ADDR_GPIOCR 0x30 +#define MPC8544_GUTS_ADDR_GPOUTDR 0x40 +#define MPC8544_GUTS_ADDR_GPINDR 0x50 +#define MPC8544_GUTS_ADDR_PMUXCR 0x60 +#define MPC8544_GUTS_ADDR_DEVDISR 0x70 +#define MPC8544_GUTS_ADDR_POWMGTCSR 0x80 +#define MPC8544_GUTS_ADDR_MCPSUMR 0x90 +#define MPC8544_GUTS_ADDR_RSTRSCR 0x94 +#define MPC8544_GUTS_ADDR_PVR 0xA0 +#define MPC8544_GUTS_ADDR_SVR 0xA4 +#define MPC8544_GUTS_ADDR_RSTCR 0xB0 +#define MPC8544_GUTS_ADDR_IOVSELSR0xC0 +#define MPC8544_GUTS_ADDR_DDRCSR 0xB20 +#define MPC8544_GUTS_ADDR_DDRCDR 0xB24 +#define MPC8544_GUTS_ADDR_DDRCLKDR0xB28 +#define MPC8544_GUTS_ADDR_CLKOCR 0xE00 +#define MPC8544_GUTS_ADDR_SRDS1CR10xF04 +#define MPC8544_GUTS_ADDR_SRDS2CR10xF10 +#define MPC8544_GUTS_ADDR_SRDS2CR30xF18 + +struct GutsState { +SysBusDevice busdev; +}; + +typedef struct GutsState GutsState; + +static uint32_t mpc8544_guts_read32(void *opaque, target_phys_addr_t addr) +{ +uint32_t value = 0; +CPUState *env = cpu_single_env; + +addr &= MPC8544_GUTS_MMIO_SIZE - 1; +switch (addr) { +case MPC8544_GUTS_ADDR_PVR: +value = env->spr[SPR_PVR]; +break; +case MPC8544_GUTS_ADDR_SVR: +value = env->spr[SPR_E500_SVR]; +break; +default: +fprintf(stderr, "guts: Unknown register read: %x\n", (int)addr); +break; +} + +return value; +} + +static CPUReadMemoryFunc * const mpc8544_guts_read[] = { +NULL, +NULL, +&mpc8544_guts_read32, +}; + +static void mpc8544_guts_write32(void *opaque, target_phys_addr_t addr, + uint32_t value) +{ +addr &= MPC8544_GUTS_MMIO_SIZE - 1; + +switch (addr) { +case MPC8544_GUTS_ADDR_RSTCR: +if (value & MPC8544_GUTS_RSTCR_RESET) { +qemu_system_reset_request(); +} +break; +default: +fprintf(stderr, "guts: Unknown register write: %x = %x\n", +(int)addr, value); +break; +} +} + +static CPUWriteMemoryFunc * const mpc8544_guts_write[] = { +NULL, +NULL, +&mpc8544_guts_write32, +}; + +static int mpc8544_guts_initfn(SysBusDevice *dev) +{ +GutsState *s; +int iomem; + +s = FROM_SYSBUS(GutsState, sysbus_from_qdev(dev)); + +iomem = cpu_register_io_memory(mpc8544_guts_read, mpc8544_guts_write, s, + DEVICE_BIG_ENDIAN); +sysbus_init_mmio(dev, MPC8544_GUTS_M
[Qemu-devel] [PATCH 0/2] Initial TSC-Scaling support for uq/master
Hi, these two patches implement initial tsc-scaling support for qemu. The TSC frequency for the guest can be configured with -cpu ,tsc_khz=x. If supported by the kernel module qemu will pass the frequency to KVM. The patches apply against current uq/master branch. Regards, Joerg Diffstat: target-i386/cpu.h |1 + target-i386/cpuid.c | 10 ++ target-i386/kvm.c | 11 +++ 3 files changed, 22 insertions(+), 0 deletions(-) Shortlog: Joerg Roedel (2): qemu-x86: Add tsc_khz option to -cpu qemu-x86: Set tsc_khz in kvm when supported
Re: [Qemu-devel] [PATCH] multiboot: Support commas in module parameters
Am 17.06.2011 08:51, schrieb Adam Lackorzynski: > Support commas in the parameter list of multiboot modules, by using double > commas (via get_opt_value()). > > Signed-off-by: Adam Lackorzynski The one thing the commit message doesn't mention is that this change requires that commas in -kernel must be escaped by doubling, too. Anyway, I think this behaviour is acceptable. Reviewed-by: Kevin Wolf
Re: [Qemu-devel] how to verify virtio is being used?
Am 16.06.2011 20:57, schrieb al pat: > I have posted this on kvm alias, but have not heard back. seeing some inputs. > > seeking some pointers/guidance as to how to determine virtio is being used... > > I configured a VM to use block device with if=virtio (create a 1GB > disk using dd I exported this disk to the VM and am now doing scp from > host to the > guest after creating partition/mkfs. > > I created another 1GB disk and export it as a IDE disk. I use the same > scp command from host to guest after creating partition/mkfs. > > I am trying to determine if my block IO is indeed using virtio in the > first case. > > Empirically, I observe that with if=virtio, the throughput is about > 30% more (in terms of mbps) and time taken is about 40% less than > for the case where I passed the disk as a IDE disk. > > My scp happens over virbr0 interface (and currently I am not concerned > if networking is using virtio) > > How do I confirm that virtio is being used? Are there any debugs that > I can enable to do that. Have a look at the guest kernel logs, lspci output or just at the device name: IDE disks are called /dev/sda etc. whereas virtio-blk disks are called /dev/vda etc. Kevin
Re: [Qemu-devel] [PATCH] target-i386: fix cmpxchg
On Fri, 17 Jun 2011, Christoph Egger wrote: > Correct emulation of i386 cmpxchg instruction in the case where the > comparison outcome is unequal and the memory write causes a page > fault. > > From: Andreas Gustafsson > Signed-off-by: Christoph Egger We've been through this a couple of times already: http://www.mail-archive.com/qemu-devel@nongnu.org/msg50159.html [..snip..] -- mailto:av1...@comtv.ru
[Qemu-devel] [PATCH 2/2] qemu-x86: Set tsc_khz in kvm when supported
Make use of the KVM_TSC_CONTROL feature if available. Signed-off-by: Joerg Roedel --- target-i386/kvm.c | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 1ae2d61..2d89544 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -528,6 +528,17 @@ int kvm_arch_init_vcpu(CPUState *env) } #endif +#ifdef KVM_CAP_TSC_CONTROL +r = kvm_check_extension(env->kvm_state, KVM_CAP_TSC_CONTROL); +if (r && env->tsc_khz) { +r = kvm_vcpu_ioctl(env, KVM_SET_TSC_KHZ, env->tsc_khz); +if (r < 0) { +fprintf(stderr, "KVM_SET_TSC_KHZ failed\n"); +return r; +} +} +#endif + qemu_add_vm_change_state_handler(cpu_update_state, env); return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data); -- 1.7.4.1
Re: [Qemu-devel] how to verify virtio is being used?
Hi Kevin, Thanks! Yes, one disk is visible in guest as sdb (partitioned to sdb1), mounted and I write to it. The virtio disk is visible as /dev/vda, (partitioned to vda1), mounted and I write to it. Kernel log on guest - do you mean dmesg? I was trying to trace through the virt io calls to confirm. and determine the invocation sequence. My lspci output: 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02) 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II] 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II] 00:01.2 USB Controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] (rev 01) 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03) 00:02.0 VGA compatible controller: Cirrus Logic GD 5446 00:03.0 Ethernet controller: Qumranet, Inc. Unknown device 1000 00:04.0 RAM memory: Qumranet, Inc. Unknown device 1002 00:06.0 SCSI storage controller: Qumranet, Inc. Unknown device 1001 lspci -k --- to show kernel drivers associated with the device does not work in the guest. Thanks a On Fri, Jun 17, 2011 at 7:15 AM, Kevin Wolf wrote: > Am 16.06.2011 20:57, schrieb al pat: >> I have posted this on kvm alias, but have not heard back. seeing some inputs. >> >> seeking some pointers/guidance as to how to determine virtio is being used... >> >> I configured a VM to use block device with if=virtio (create a 1GB >> disk using dd I exported this disk to the VM and am now doing scp from >> host to the >> guest after creating partition/mkfs. >> >> I created another 1GB disk and export it as a IDE disk. I use the same >> scp command from host to guest after creating partition/mkfs. >> >> I am trying to determine if my block IO is indeed using virtio in the >> first case. >> >> Empirically, I observe that with if=virtio, the throughput is about >> 30% more (in terms of mbps) and time taken is about 40% less than >> for the case where I passed the disk as a IDE disk. >> >> My scp happens over virbr0 interface (and currently I am not concerned >> if networking is using virtio) >> >> How do I confirm that virtio is being used? Are there any debugs that >> I can enable to do that. > > Have a look at the guest kernel logs, lspci output or just at the device > name: IDE disks are called /dev/sda etc. whereas virtio-blk disks are > called /dev/vda etc. > > Kevin >
[Qemu-devel] [PATCH] use mmap to allocate execute memory
Use mmap to allocate executable memory on NetBSD as well. From: Tobias Nygren Signed-off-by: Christoph Egger diff --git a/exec.c b/exec.c index 09928a3..1954a1c 100644 --- a/exec.c +++ b/exec.c @@ -520,7 +520,8 @@ static void code_gen_alloc(unsigned long tb_size) } } #elif defined(__FreeBSD__) || defined(__FreeBSD_kernel__) \ -|| defined(__DragonFly__) || defined(__OpenBSD__) +|| defined(__DragonFly__) || defined(__OpenBSD__) \ +|| defined(__NetBSD__) { int flags; void *addr = NULL; -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
[Qemu-devel] [PATCH] target-i386: fix cmpxchg
Correct emulation of i386 cmpxchg instruction in the case where the comparison outcome is unequal and the memory write causes a page fault. From: Andreas Gustafsson Signed-off-by: Christoph Egger diff --git a/target-i386/translate.c b/target-i386/translate.c index 10bd72a..69a878f 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -4857,20 +4857,23 @@ static target_ulong disas_insn(DisasContext *s, target_ulong pc_start) tcg_gen_sub_tl(t2, cpu_regs[R_EAX], t0); gen_extu(ot, t2); tcg_gen_brcondi_tl(TCG_COND_EQ, t2, 0, label1); +label2 = gen_new_label(); if (mod == 3) { -label2 = gen_new_label(); gen_op_mov_reg_v(ot, R_EAX, t0); tcg_gen_br(label2); gen_set_label(label1); gen_op_mov_reg_v(ot, rm, t1); -gen_set_label(label2); } else { -tcg_gen_mov_tl(t1, t0); +/* perform no-op store cycle like physical cpu; must be + * before changing accumulator to ensure idempotency if + * the store faults and the instruction is restarted */ +gen_op_st_v(ot + s->mem_index, t0, a0); gen_op_mov_reg_v(ot, R_EAX, t0); +tcg_gen_br(label2); gen_set_label(label1); -/* always store */ gen_op_st_v(ot + s->mem_index, t1, a0); } +gen_set_label(label2); tcg_gen_mov_tl(cpu_cc_src, t0); tcg_gen_mov_tl(cpu_cc_dst, t2); s->cc_op = CC_OP_SUBB + ot; -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
Re: [Qemu-devel] Image streaming and live block copy
On Fri, Jun 17, 2011 at 10:22 AM, Kevin Wolf wrote: > Am 17.06.2011 10:57, schrieb Stefan Hajnoczi: >> On Fri, Jun 17, 2011 at 9:36 AM, Kevin Wolf wrote: >>> By the way, we'll get problems with the colon syntax. Without -blockdev >>> we'll have to invent a new syntax, maybe with brackets: >>> >>> blkstream:[nbd:localhost]:out.qcow2 >> >> Embedding block driver options in filenames is getting worse as time >> goes on. > > Well, yes. We need -blockdev for a sane way to express complex relations > between BlockDriverStates. But then, we'll also want to have convenient > shortcuts for manual use, and that may be something like the existing > colon syntax. I really don't feel like typing three full -blockdev > parameters for qcow2 on blockdbg on raw. > >> I recently tried to refactor and eliminate >> QEMUOptionParameter so that we only use QemuOpts instead of two >> different option APIs. Part of that involves keeping separate >> per-block driver (i.e. -blockdev) options lists, which would allow us >> to pass proper options to block drivers instead of embedding them in >> the filename. > > Aren't these completely independent things? QEMUOptionParameter is used > for image creation, whereas filenames are used for opening images. I > think you can change one without changing the other. Yeah but I'd rather not spread two different APIs to do the same thing. Most of QEMU uses QemuOpts but image creation uses QEMUOptionParameter. The longer we leave that the more quirks like the concatenation behavior will fragment the two. Stefan
[Qemu-devel] [PATCH 1/2] qemu-x86: Add tsc_khz option to -cpu
To let the user configure the desired tsc frequency for the guest if running in KVM. Signed-off-by: Joerg Roedel --- target-i386/cpu.h |1 + target-i386/cpuid.c | 10 ++ 2 files changed, 11 insertions(+), 0 deletions(-) diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 0b039d4..a381c34 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -753,6 +753,7 @@ typedef struct CPUX86State { uint32_t cpuid_kvm_features; uint32_t cpuid_svm_features; bool tsc_valid; +int tsc_khz; /* in order to simplify APIC support, we leave this pointer to the user */ diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c index 79e7580..2b59e66 100644 --- a/target-i386/cpuid.c +++ b/target-i386/cpuid.c @@ -224,6 +224,7 @@ typedef struct x86_def_t { int family; int model; int stepping; +int tsc_khz; uint32_t features, ext_features, ext2_features, ext3_features; uint32_t kvm_features, svm_features; uint32_t xlevel; @@ -704,6 +705,14 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model) } else if (!strcmp(featurestr, "model_id")) { pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id), val); +} else if (!strcmp(featurestr, "tsc_khz")) { +char *err; +numvalue = strtoul(val, &err, 0); +if (!*val || *err) { +fprintf(stderr, "bad numerical value %s\n", val); +goto error; +} +x86_cpu_def->tsc_khz = numvalue; } else { fprintf(stderr, "unrecognized feature %s\n", featurestr); goto error; @@ -872,6 +881,7 @@ int cpu_x86_register (CPUX86State *env, const char *cpu_model) env->cpuid_svm_features = def->svm_features; env->cpuid_ext4_features = def->ext4_features; env->cpuid_xlevel2 = def->xlevel2; +env->tsc_khz = def->tsc_khz; if (!kvm_enabled()) { env->cpuid_features &= TCG_FEATURES; env->cpuid_ext_features &= TCG_EXT_FEATURES; -- 1.7.4.1
Re: [Qemu-devel] Image streaming and live block copy
On Fri, Jun 17, 2011 at 10:36:21AM +0200, Kevin Wolf wrote: > Am 16.06.2011 16:52, schrieb Marcelo Tosatti: > > On Thu, Jun 16, 2011 at 03:08:30PM +0200, Kevin Wolf wrote: > >> Am 16.06.2011 14:49, schrieb Avi Kivity: > >>> On 06/16/2011 03:35 PM, Kevin Wolf wrote: > * Image streaming is a normal image file plus copy-on-read plus a > background task that copies data from the source image > >>> > >>> Or a block-mirror started in degraded mode. > >> > >> At least not in the same configuration as with live block copy: You > >> don't want to write to the source, you only want to read from it when > >> the destination doesn't have the data yet. > >> > * Live block copy is a block-mirror of two normal image files plus a > background task that copies data from the source image > >>> > >>> = block-mirror started in degraded mode > >> > The right solution is probably to implement COR and the background task > in generic block layer code (no reason to restrict it to QED) and use it > for both image streaming and live block copy. (This is a bit more > complicated than it may sound here because guest writes must always take > precedence over a copy - but doing complicated things is an even better > reason to do it in a common place instead of duplicating) > >>> > >>> Or in a block-mirror block format driver - generic code need not be > >>> involved. > >> > >> Might be an option. In this case generic code is only involved with the > >> stacking of BlockDriverStates, which is already implemented (but > >> requires -blockdev for a sane way to configure things). > >> > >> Kevin > > > > What are the disadvantages of such an approach for image streaming, > > versus the current QED approach? > > > > blkstream block driver: > > > > - Maintain in memory whether given block is allocated in local image, > > if not, read from remote, write to local. Set block as local. > > Local and remote simply two block drivers from image streaming driver > > POV. > > Why maintain it in memory? We already have mechanisms to track this in > COW image formats, so that you can even continue after a crash. Which mechanism is that? You'd need separate metadata for image streaming purposes, since you might want the destination-image to use a backing image itself. > We can still add a raw-cow driver that maintains the COW data in memory > for allowing raw copies, if this is needed.
Re: [Qemu-devel] Image streaming and live block copy
On Thu, Jun 16, 2011 at 04:30:18PM +0100, Stefan Hajnoczi wrote: > On Thu, Jun 16, 2011 at 11:52:43AM -0300, Marcelo Tosatti wrote: > This approach does not use the backing file feature? Not directly. For block copy, the destination (or source) images might share the same backing file (this is a requirement for live snapshot merge).
[Qemu-devel] [PATCH v4 1/2] pxa2xx_lcd: add proper rotation support
Until now, pxa2xx_lcd only supported 90deg rotation, but some machines (for example Zipit Z2) needs 270deg rotation. Signed-off-by: Vasily Khoruzhick --- v2: codestyle fixes v3: fix dpy_update calls for 180 and 360 deg. rotation. v4: codestyle fixes; replace atoi with strtol hw/framebuffer.c |3 + hw/pxa2xx_lcd.c | 109 -- input.c | 34 qemu-options.hx |9 vl.c | 11 +- 5 files changed, 144 insertions(+), 22 deletions(-) diff --git a/hw/framebuffer.c b/hw/framebuffer.c index 24cdf25..56cf16e 100644 --- a/hw/framebuffer.c +++ b/hw/framebuffer.c @@ -78,6 +78,9 @@ void framebuffer_update_display( dest = ds_get_data(ds); if (dest_col_pitch < 0) dest -= dest_col_pitch * (cols - 1); +if (dest_row_pitch < 0) { +dest -= dest_row_pitch * (rows - 1); +} first = -1; addr = pd; diff --git a/hw/pxa2xx_lcd.c b/hw/pxa2xx_lcd.c index e524802..a5f8c51 100644 --- a/hw/pxa2xx_lcd.c +++ b/hw/pxa2xx_lcd.c @@ -665,7 +665,7 @@ static void pxa2xx_palette_parse(PXA2xxLCDState *s, int ch, int bpp) } } -static void pxa2xx_lcdc_dma0_redraw_horiz(PXA2xxLCDState *s, +static void pxa2xx_lcdc_dma0_redraw_rot0(PXA2xxLCDState *s, target_phys_addr_t addr, int *miny, int *maxy) { int src_width, dest_width; @@ -692,7 +692,7 @@ static void pxa2xx_lcdc_dma0_redraw_horiz(PXA2xxLCDState *s, fn, s->dma_ch[0].palette, miny, maxy); } -static void pxa2xx_lcdc_dma0_redraw_vert(PXA2xxLCDState *s, +static void pxa2xx_lcdc_dma0_redraw_rot90(PXA2xxLCDState *s, target_phys_addr_t addr, int *miny, int *maxy) { int src_width, dest_width; @@ -720,6 +720,67 @@ static void pxa2xx_lcdc_dma0_redraw_vert(PXA2xxLCDState *s, miny, maxy); } +static void pxa2xx_lcdc_dma0_redraw_rot180(PXA2xxLCDState *s, +target_phys_addr_t addr, int *miny, int *maxy) +{ +int src_width, dest_width; +drawfn fn = NULL; +if (s->dest_width) { +fn = s->line_fn[s->transp][s->bpp]; +} +if (!fn) { +return; +} + +src_width = (s->xres + 3) & ~3; /* Pad to a 4 pixels multiple */ +if (s->bpp == pxa_lcdc_19pbpp || s->bpp == pxa_lcdc_18pbpp) { +src_width *= 3; +} else if (s->bpp > pxa_lcdc_16bpp) { +src_width *= 4; +} else if (s->bpp > pxa_lcdc_8bpp) { +src_width *= 2; +} + +dest_width = s->xres * s->dest_width; +*miny = 0; +framebuffer_update_display(s->ds, + addr, s->xres, s->yres, + src_width, -dest_width, -s->dest_width, + s->invalidated, + fn, s->dma_ch[0].palette, miny, maxy); +} + +static void pxa2xx_lcdc_dma0_redraw_rot270(PXA2xxLCDState *s, + target_phys_addr_t addr, int *miny, int *maxy) +{ +int src_width, dest_width; +drawfn fn = NULL; +if (s->dest_width) { +fn = s->line_fn[s->transp][s->bpp]; +} +if (!fn) { +return; +} + +src_width = (s->xres + 3) & ~3; /* Pad to a 4 pixels multiple */ +if (s->bpp == pxa_lcdc_19pbpp || s->bpp == pxa_lcdc_18pbpp) { +src_width *= 3; +} else if (s->bpp > pxa_lcdc_16bpp) { +src_width *= 4; +} else if (s->bpp > pxa_lcdc_8bpp) { +src_width *= 2; +} + +dest_width = s->yres * s->dest_width; +*miny = 0; +framebuffer_update_display(s->ds, + addr, s->xres, s->yres, + src_width, -s->dest_width, dest_width, + s->invalidated, + fn, s->dma_ch[0].palette, + miny, maxy); +} + static void pxa2xx_lcdc_resize(PXA2xxLCDState *s) { int width, height; @@ -730,10 +791,11 @@ static void pxa2xx_lcdc_resize(PXA2xxLCDState *s) height = LCCR2_LPP(s->control[2]) + 1; if (width != s->xres || height != s->yres) { -if (s->orientation) +if (s->orientation == 90 || s->orientation == 270) { qemu_console_resize(s->ds, height, width); -else +} else { qemu_console_resize(s->ds, width, height); +} s->invalidated = 1; s->xres = width; s->yres = height; @@ -797,10 +859,24 @@ static void pxa2xx_update_display(void *opaque) } if (miny >= 0) { -if (s->orientation) -dpy_update(s->ds, miny, 0, maxy - miny, s->xres); -else -dpy_update(s->ds, 0, miny, s->xres, maxy - miny); +switch (s->orientation) { +case 0: +dpy_update(s->ds, 0, miny, s->xres, maxy - miny + 1); +break; +case 90: +dpy_update(s->ds, miny, 0, maxy - miny + 1, s->xres); +break; +case 180: +maxy = s->yres - maxy -
Re: [Qemu-devel] Aboriginal Linux 1.0.2: linux-2.6.39 system images for a dozen targets.
On 06/16/2011 12:45 PM, Mulyadi Santosa wrote: > On Thu, Jun 16, 2011 at 19:34, Rob Landley wrote: >> Aboriginal Linux's motto is "we cross compile so you don't have to". > > Hi Rob > > Thanks for your kind work I began to learn about building cross > compiler and found your workit's like I found a good hint. > Glad I could help. If you have any questions that aren't specifically about QEMU, the other people trying to figure this stuff out (myself included) are on the project's mailing list: http://lists.landley.net/listinfo.cgi/aboriginal-landley.net It's not very high traffic, but we try to answer questions (albeit sometimes with "I have no idea"). Thanks, Rob
Re: [Qemu-devel] [Xen-devel] [PATCH 3/3] xen: implement unplug protocol in xen_platform
On Thu, Jun 16, 2011 at 17:05, wrote: > From: Stefano Stabellini > > The unplug protocol is necessary to support PV drivers in the guest: the > drivers expect to be able to "unplug" emulated disks and nics before > initializing the Xen PV interfaces. > It is responsibility of the guest to make sure that the unplug is done > before the emulated devices or the PV interface start to be used. > > We use pci_for_each_device to walk the PCI bus, identify the devices and > disks that we want to disable and dynamically unplug them. > > Signed-off-by: Stefano Stabellini > --- > hw/xen_platform.c | 63 > - > 1 files changed, 62 insertions(+), 1 deletions(-) > > diff --git a/hw/xen_platform.c b/hw/xen_platform.c > index b167eee..9f8c843 100644 > --- a/hw/xen_platform.c > +++ b/hw/xen_platform.c > @@ -34,6 +34,9 @@ > #include "xen_backend.h" > #include "rwhandler.h" > #include "trace.h" > +#include "hw/ide/internal.h" > +#include "hw/ide/pci.h" > +#include "hw/pci_ids.h" > > #include > > @@ -76,6 +79,54 @@ static void log_writeb(PCIXenPlatformState *s, char val) > } > > /* Xen Platform, Fixed IOPort */ > +#define UNPLUG_ALL_IDE_DISKS 1 > +#define UNPLUG_ALL_NICS 2 > +#define UNPLUG_AUX_IDE_DISKS 4 > + > +static int unplug_param; > + > +static void unplug_nic(PCIBus *b, PCIDevice *d) > +{ > + if (d->config[0xa] == 0 && d->config[0xb] == 2) { You should use: pci_get_word(d->config+PCI_CLASS_DEVICE) == PCI_CLASS_NETWORK_ETHERNET It'll be clearer. > + pci_unplug_device(&(d->qdev)); > + } > +} > + > +static void pci_unplug_nics(PCIBus *bus) > +{ > + pci_for_each_device(bus, 0, unplug_nic); > +} > + > +static void unplug_disks(PCIBus *b, PCIDevice *d) > +{ > + if (d->config[0xa] == 1 && d->config[0xb] == 1) { Same here with PCI_CLASS_STORAGE_IDE. Regards, -- Anthony PERARD
Re: [Qemu-devel] Image streaming and live block copy
On 06/17/2011 03:36 AM, Kevin Wolf wrote: Am 16.06.2011 16:52, schrieb Marcelo Tosatti: On Thu, Jun 16, 2011 at 03:08:30PM +0200, Kevin Wolf wrote: Over this scheme, you'd have: 1) Block copy. Reopen image to be copied with blkstream:/path/to/current-image:/path/to/destination-image, background read sectors 0...N. 2) Image stream: blkstream:remote-image:/path/to/local-image, background read sectors 0...N. Where remote-image is remote accessible image such as NBD. I think that should work. By the way, we'll get problems with the colon syntax. Without -blockdev we'll have to invent a new syntax, maybe with brackets: blkstream:[nbd:localhost]:out.qcow2 So what's the main issue with -blockdev today? Just need someone to spend some time implementing it? Also, it would be a useful exercise to try and capture some of this in the wiki. That makes it a bit easier to reference as we move forward. Regards, Anthony Liguori Kevin
[Qemu-devel] [PATCH] fix network interface tap backend
Fix network interface tap backend work on NetBSD. It uses an ioctl to get the tap name. From Manuel Bouyer Signed-off-by: Christoph Egger diff --git a/net/tap-bsd.c b/net/tap-bsd.c index 2f3efde..577aafe 100644 --- a/net/tap-bsd.c +++ b/net/tap-bsd.c @@ -28,6 +28,8 @@ #include "qemu-error.h" #ifdef __NetBSD__ +#include +#include #include #endif @@ -40,8 +42,12 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required) { int fd; +#ifdef TAPGIFNAME +struct ifreq ifr; +#else char *dev; struct stat s; +#endif #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__OpenBSD__) /* if no ifname is given, always start the search from tap0/tun0. */ @@ -77,14 +83,30 @@ int tap_open(char *ifname, int ifname_size, int *vnet_hdr, int vnet_hdr_required #else TFR(fd = open("/dev/tap", O_RDWR)); if (fd < 0) { -fprintf(stderr, "warning: could not open /dev/tap: no virtual network emulation\n"); +fprintf(stderr, +"warning: could not open /dev/tap: no virtual network emulation: %s\n", +strerror(errno)); return -1; } #endif -fstat(fd, &s); +#ifdef TAPGIFNAME +if (ioctl(fd, TAPGIFNAME, (void *)&ifr) < 0) { +fprintf(stderr, "warning: could not get tap name: %s\n", +strerror(errno)); +return -1; +} +pstrcpy(ifname, ifname_size, ifr.ifr_name); +#else +if (fstat(fd, &s) < 0) { +fprintf(stderr, +"warning: could not stat /dev/tap: no virtual network emulation: %s\n", +strerror(errno)); +return -1; +} dev = devname(s.st_rdev, S_IFCHR); pstrcpy(ifname, ifname_size, dev); +#endif if (*vnet_hdr) { /* BSD doesn't have IFF_VNET_HDR */ -- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632
Re: [Qemu-devel] [RFC][PATCH] Register Linux dyntick timer as per-thread signal
On 2011-06-16 17:24, Alexandre Raymond wrote: > Hi Jan, > > On Thu, Jun 16, 2011 at 5:31 AM, Jan Kiszka wrote: >> Ingo Molnar pointed out that sending the timer signal to the whole >> process, just blocking it everywhere, is suboptimal with an increasing >> number of threads. QEMU is using this pattern so far. > > I am not familiar with this code, but don't you already need to block > SIGALRM properly in all threads for OSes != Linux ? If so, isn't this > patch redundant? Yes, we still need to block for the sake of non-Linux UNIX or pre-signalfd Linux. That blocking becomes in fact redundant in the signalfd case, but that's both harmless and not worth optimizing. The key is that per-thread signals do not care about other threads having them blocked or not, they only deal with the target thread. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH v2] Register Linux dyntick timer as per-thread signal
Derived from kvm-tool patch http://thread.gmane.org/gmane.comp.emulators.kvm.devel/74309 Ingo Molnar pointed out that sending the timer signal to the whole process, just blocking it everywhere, is suboptimal with an increasing number of threads. QEMU is also using this pattern so far. Linux provides a (non-portable) way to restrict the signal to a single thread: We can use SIGEV_THREAD_ID unless we are forced to emulate signalfd via an additional thread. That case could theoretically be optimized as well, but it doesn't look worth bothering. Signed-off-by: Jan Kiszka --- Changes in v2: - refactored dynticks_start_timer changes as suggested by Richard Henderson - added reference to original kvm-tool patch compatfd.c | 11 +++ compatfd.h |1 + qemu-timer.c |8 3 files changed, 20 insertions(+), 0 deletions(-) diff --git a/compatfd.c b/compatfd.c index 41586ce..31654c6 100644 --- a/compatfd.c +++ b/compatfd.c @@ -115,3 +115,14 @@ int qemu_signalfd(const sigset_t *mask) return qemu_signalfd_compat(mask); } + +bool qemu_signalfd_available(void) +{ +#ifdef CONFIG_SIGNALFD +errno = 0; +syscall(SYS_signalfd, -1, NULL, _NSIG / 8); +return errno != ENOSYS; +#else +return false; +#endif +} diff --git a/compatfd.h b/compatfd.h index fc37915..6b04877 100644 --- a/compatfd.h +++ b/compatfd.h @@ -39,5 +39,6 @@ struct qemu_signalfd_siginfo { }; int qemu_signalfd(const sigset_t *mask); +bool qemu_signalfd_available(void); #endif diff --git a/qemu-timer.c b/qemu-timer.c index 72066c7..743cf96 100644 --- a/qemu-timer.c +++ b/qemu-timer.c @@ -803,6 +803,8 @@ static int64_t qemu_next_alarm_deadline(void) #if defined(__linux__) +#include "compatfd.h" + static int dynticks_start_timer(struct qemu_alarm_timer *t) { struct sigevent ev; @@ -822,6 +824,12 @@ static int dynticks_start_timer(struct qemu_alarm_timer *t) memset(&ev, 0, sizeof(ev)); ev.sigev_value.sival_int = 0; ev.sigev_notify = SIGEV_SIGNAL; +#ifdef SIGEV_THREAD_ID +if (qemu_signalfd_available()) { +ev.sigev_notify = SIGEV_THREAD_ID; +ev._sigev_un._tid = qemu_get_thread_id(); +} +#endif /* SIGEV_THREAD_ID */ ev.sigev_signo = SIGALRM; if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) { -- 1.7.1
Re: [Qemu-devel] Image streaming and live block copy
Am 17.06.2011 10:57, schrieb Stefan Hajnoczi: > On Fri, Jun 17, 2011 at 9:36 AM, Kevin Wolf wrote: >> By the way, we'll get problems with the colon syntax. Without -blockdev >> we'll have to invent a new syntax, maybe with brackets: >> >> blkstream:[nbd:localhost]:out.qcow2 > > Embedding block driver options in filenames is getting worse as time > goes on. Well, yes. We need -blockdev for a sane way to express complex relations between BlockDriverStates. But then, we'll also want to have convenient shortcuts for manual use, and that may be something like the existing colon syntax. I really don't feel like typing three full -blockdev parameters for qcow2 on blockdbg on raw. > I recently tried to refactor and eliminate > QEMUOptionParameter so that we only use QemuOpts instead of two > different option APIs. Part of that involves keeping separate > per-block driver (i.e. -blockdev) options lists, which would allow us > to pass proper options to block drivers instead of embedding them in > the filename. Aren't these completely independent things? QEMUOptionParameter is used for image creation, whereas filenames are used for opening images. I think you can change one without changing the other. Kevin
Re: [Qemu-devel] Image streaming and live block copy
On Fri, Jun 17, 2011 at 9:36 AM, Kevin Wolf wrote: > By the way, we'll get problems with the colon syntax. Without -blockdev > we'll have to invent a new syntax, maybe with brackets: > > blkstream:[nbd:localhost]:out.qcow2 Embedding block driver options in filenames is getting worse as time goes on. I recently tried to refactor and eliminate QEMUOptionParameter so that we only use QemuOpts instead of two different option APIs. Part of that involves keeping separate per-block driver (i.e. -blockdev) options lists, which would allow us to pass proper options to block drivers instead of embedding them in the filename. I got stuck because today the protocol and format QEMUOptionParameters get concatenated in some cases. Concatenation is not really supported by QemuOpts :). Anyway, here's the current state if anyone is interested: http://repo.or.cz/w/qemu/stefanha.git/commitdiff/b49babb2c8b476a36357cfd7276ca45a11039ca5 Stefan
[Qemu-devel] [PATCH] usb-storage: Turn drive serial into a qdev property usb-storage.serial
It needs to be a qdev property, because it belongs to the drive's guest part. Precedence: commit a0fef654 and 6ced55a5. Bonus: info qtree now shows the serial number. Signed-off-by: Markus Armbruster --- hw/usb-msd.c | 14 +++--- 1 files changed, 11 insertions(+), 3 deletions(-) diff --git a/hw/usb-msd.c b/hw/usb-msd.c index c59797b..9033843 100644 --- a/hw/usb-msd.c +++ b/hw/usb-msd.c @@ -51,6 +51,7 @@ typedef struct { SCSIRequest *req; SCSIBus bus; BlockConf conf; +char *serial; SCSIDevice *scsi_dev; uint32_t removable; int result; @@ -531,9 +532,15 @@ static int usb_msd_initfn(USBDevice *dev) bdrv_detach(bs, &s->dev.qdev); s->conf.bs = NULL; -dinfo = drive_get_by_blockdev(bs); -if (dinfo && dinfo->serial) { -usb_desc_set_string(dev, STR_SERIALNUMBER, dinfo->serial); +if (!s->serial) { +/* try to fall back to value set with legacy -drive serial=... */ +dinfo = drive_get_by_blockdev(bs); +if (*dinfo->serial) { +s->serial = strdup(dinfo->serial); +} +} +if (s->serial) { +usb_desc_set_string(dev, STR_SERIALNUMBER, s->serial); } usb_desc_init(dev); @@ -632,6 +639,7 @@ static struct USBDeviceInfo msd_info = { .usbdevice_init = usb_msd_init, .qdev.props = (Property[]) { DEFINE_BLOCK_PROPERTIES(MSDState, conf), +DEFINE_PROP_STRING("serial", MSDState, serial), DEFINE_PROP_BIT("removable", MSDState, removable, 0, false), DEFINE_PROP_END_OF_LIST(), }, -- 1.7.2.3
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
On (Fri) 17 Jun 2011 [09:47:31], Markus Armbruster wrote: > Amit Shah writes: > > > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: > >> If I start qemu with: > >> > >> # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ > >> -device virtio-serial \ > >> -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ > >> -device virtserialport,chardev=foo,name=org.qemu.guest_agent > >> > >> I get a segfault when booting a Fedora 14 guest. The backtrace says: > >> > >> Program terminated with signal 11, Segmentation fault. > >> #0 0x00420850 in handle_control_message (vser=0x3732bd0, > >> buf=0x2c173e0, len=8) at > >> /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 > >> 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > > > Strange, I've not seen it so far in my testing (neither in the daily > > test runs of the virtio-serial testsuite). > > > >> I've also bisected this and git points out to commit: > >> > >> commit a15bb0d6a981de749452a5180fc8084d625671da > >> Author: Markus Armbruster > >> Date: Wed May 25 14:21:13 2011 +0200 > >> > >> virtio-serial: Drop redundant VirtIOSerialPort member info > >> > >> I think what's happening is that the device is not initialized on a > >> VIRTIO_CONSOLE_DEVICE_READY event. > > Really? > > I believe the device is initialized just fine, but the message refers to > a nonexistant port. The check after find_port_by_id() suggests that's > expected for VIRTIO_CONSOLE_DEVICE_READY. What's happening is VIRTIO_CONSOLE_DEVICE_READY is a message for the whole device, not for an individual port. So port is NULL. All other messages (currently) are port-specific, so they have 'port' set to something. The commit message needs to be tweaked for this. > > >>Moving the DO_UPCAST() call to > >> the other events fixes the problem to me. > >> > >> Signed-off-by: Luiz Capitulino > >> --- > >> hw/virtio-serial-bus.c |4 ++-- > >> 1 files changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c > >> index 9a12104..579f676 100644 > >> --- a/hw/virtio-serial-bus.c > >> +++ b/hw/virtio-serial-bus.c > >> @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial *vser, > >> void *buf, size_t len) > >> if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) > >> return; > >> > >> -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > >> - > > > > Ah - this missed the !port check. It should be possible to do this in > > a 'if (port)' block instead of replicating in the individual case > > statements. > > You might have to do something like info = port ? DO_UPCAST(...) : NULL > to avoid warnings about info used uninitialized. That won't happen -- in the case where port is NULL, we shouldn't try to use info. If we do get such a warning, it means there's a bug :-) Amit
[Qemu-devel] [Bug 754591] Re: NIC doesn't work when it had been used before
It was fixed in kvm.git. This bug is fixed in the kvm upstream. It was fixed in kvm.git f8fcfd7 by alex.william...@redhat.com. commit f8fcfd775523347afe460dc3a0f45d0479e784a2 Author: Alex Williamson Date: Tue May 10 10:02:39 2011 -0600 KVM: Use pci_store/load_saved_state() around VM device usage Store the device saved state so that we can reload the device back to the original state when it's unassigned. This has the benefit that the state survives across pci_reset_function() calls via the PCI sysfs reset interface while the VM is using the device. Signed-off-by: Alex Williamson Acked-by: Avi Kivity Signed-off-by: Jesse Barnes ** Changed in: qemu Status: New => Fix Committed ** Changed in: qemu Status: Fix Committed => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/754591 Title: NIC doesn't work when it had been used before Status in QEMU: Fix Released Bug description: Environment: Host OS (ia32/ia32e/IA64): All Guest OS (ia32/ia32e/IA64): All Guest OS Type (Linux/Windows):All kvm.git Commit:3b840cc27e5b831a26e3303096b88e112d1cf59f qemu-kvm Commit:df85c051d780bca0ee2462cfeb8ef6d9552a19b0 Host Kernel Version:2.6.38+ Hardware:Westmere-EP / Sandy Bridge Bug detailed description: -- NIC doesn't work when it had been used before. Statically assign a NIC to guest, it works well. Then shutdown the guest, and assign the same NIC to a guest again. The NIC doesn't work. The dmesg of the guest is as following. -- Intel(R) Gigabit Ethernet Network Driver - version 2.1.0-k2 Copyright (c) 2007-2009 Intel Corporation. ACPI: PCI Interrupt Link [LNKC] enabled at IRQ 11 igb :00:03.0: PCI INT A -> Link[LNKC] -> GSI 11 (level, high) -> IRQ 11 igb :00:03.0: setting latency timer to 64 alloc irq_desc for 24 on node -1 alloc kstat_irqs on node -1 igb :00:03.0: irq 24 for MSI/MSI-X alloc irq_desc for 25 on node -1 alloc kstat_irqs on node -1 igb :00:03.0: irq 25 for MSI/MSI-X alloc irq_desc for 26 on node -1 alloc kstat_irqs on node -1 igb :00:03.0: irq 26 for MSI/MSI-X igb :00:03.0: PHY reset is blocked due to SOL/IDER session. igb :00:03.0: The NVM Checksum Is Not Valid igb :00:03.0: PCI INT A disabled igb: probe of :00:03.0 failed with error -5 Reproduce steps: 1.statically assign a NIC to a guest, check if it works well 2.shutdown the guest. 3.statically assign the same NIC to a guest (NIC doesn't work now). To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/754591/+subscriptions
Re: [Qemu-devel] Image streaming and live block copy
Am 16.06.2011 16:52, schrieb Marcelo Tosatti: > On Thu, Jun 16, 2011 at 03:08:30PM +0200, Kevin Wolf wrote: >> Am 16.06.2011 14:49, schrieb Avi Kivity: >>> On 06/16/2011 03:35 PM, Kevin Wolf wrote: * Image streaming is a normal image file plus copy-on-read plus a background task that copies data from the source image >>> >>> Or a block-mirror started in degraded mode. >> >> At least not in the same configuration as with live block copy: You >> don't want to write to the source, you only want to read from it when >> the destination doesn't have the data yet. >> * Live block copy is a block-mirror of two normal image files plus a background task that copies data from the source image >>> >>> = block-mirror started in degraded mode >> The right solution is probably to implement COR and the background task in generic block layer code (no reason to restrict it to QED) and use it for both image streaming and live block copy. (This is a bit more complicated than it may sound here because guest writes must always take precedence over a copy - but doing complicated things is an even better reason to do it in a common place instead of duplicating) >>> >>> Or in a block-mirror block format driver - generic code need not be >>> involved. >> >> Might be an option. In this case generic code is only involved with the >> stacking of BlockDriverStates, which is already implemented (but >> requires -blockdev for a sane way to configure things). >> >> Kevin > > What are the disadvantages of such an approach for image streaming, > versus the current QED approach? > > blkstream block driver: > > - Maintain in memory whether given block is allocated in local image, > if not, read from remote, write to local. Set block as local. > Local and remote simply two block drivers from image streaming driver > POV. Why maintain it in memory? We already have mechanisms to track this in COW image formats, so that you can even continue after a crash. We can still add a raw-cow driver that maintains the COW data in memory for allowing raw copies, if this is needed. > - Once all blocks are local, notify mgmt so it can switch to local > copy. > - Writes are mirrored to source and destination, minding guest writes > over copy writes. Image streaming shouldn't write to the source. But adding a flag for this isn't a major problem. > Over this scheme, you'd have: > > 1) Block copy. > Reopen image to be copied with > blkstream:/path/to/current-image:/path/to/destination-image, > background read sectors 0...N. > > 2) Image stream: > blkstream:remote-image:/path/to/local-image, > background read sectors 0...N. > > Where remote-image is remote accessible image such as NBD. I think that should work. By the way, we'll get problems with the colon syntax. Without -blockdev we'll have to invent a new syntax, maybe with brackets: blkstream:[nbd:localhost]:out.qcow2 Kevin
Re: [Qemu-devel] Image streaming and live block copy
Am 16.06.2011 16:38, schrieb Marcelo Tosatti: > On Thu, Jun 16, 2011 at 02:35:37PM +0200, Kevin Wolf wrote: >> Am 14.06.2011 20:18, schrieb Stefan Hajnoczi: >>> Overview >>> >>> >>> This patch series adds image streaming support for QED image files. Other >>> image formats can also be supported in the future. >>> >>> Image streaming populates the file in the background while the guest is >>> running. This makes it possible to start the guest before its image file >>> has >>> been fully provisioned. >>> >>> Example use cases include: >>> * Providing small virtual appliances for download that can be launched >>>immediately but provision themselves in the background. >>> * Reducing guest provisioning time by creating local image files but >>> backing >>>them with shared master images which will be streamed. >>> >>> When image streaming is enabled, the unallocated regions of the image file >>> are >>> populated with the data from the backing file. This occurs in the >>> background >>> and the guest can perform regular I/O in the meantime. Once the entire >>> backing >>> file has been streamed, the image no longer requires a backing file and will >>> drop its reference >> >> Long CC list and Kevin wearing his upstream hat - this might be an >> unpleasant email. :-) >> >> So yesterday I had separate discussions with Stefan about image >> streaming and Marcelo, Avi and some other folks about live block copy. >> The conclusion was in both cases that, yes, both things are pretty >> similar and, yes, the current implementation don't reflect that but >> duplicate everything. >> >> To summarise what both things are about: >> >> * Image streaming is a normal image file plus copy-on-read plus a >> background task that copies data from the source image >> * Live block copy is a block-mirror of two normal image files plus a >> background task that copies data from the source image >> >> The right solution is probably to implement COR and the background task >> in generic block layer code (no reason to restrict it to QED) and use it >> for both image streaming and live block copy. (This is a bit more >> complicated than it may sound here because guest writes must always take >> precedence over a copy - but doing complicated things is an even better >> reason to do it in a common place instead of duplicating) >> >> People seem to agree on this, and the reason that I've heard why we >> should merge the existing code instead is downstream time pressure. That >> may be a valid reason for downstreams to add such code, but is taking >> such code really the best option for upstream (and therefore long-term >> for downstreams)? >> >> If we take these patches as they are, I doubt that we'll ever get a >> rewrite to implement the code as it should have been done in the first >> place. > > That (a newer, unified mechanism) is just a matter of allocating > resources to the implementation. > > At least in block copy's case the interface can be reused, so it can be > seen as an incremental approach (read: advocating in favour of merging > live block copy patchset). Right. However, my point was that I'm afraid that this resource allocation and therefore the incremental improvement won't happen because for most people (who don't care about the code) the result will be "good enough" and the problem will only be visible for block layer people who must work with the code. Kevin
Re: [Qemu-devel] [PATCH] virtio-serial: Fix segfault on guest boot
Amit Shah writes: > On (Thu) 16 Jun 2011 [13:38:49], Luiz Capitulino wrote: >> If I start qemu with: >> >> # qemu -hda disks/test.img -enable-kvm -m 1G -snapshot \ >> -device virtio-serial \ >> -chardev socket,host=localhost,port=1234,server,nowait,id=foo \ >> -device virtserialport,chardev=foo,name=org.qemu.guest_agent >> >> I get a segfault when booting a Fedora 14 guest. The backtrace says: >> >> Program terminated with signal 11, Segmentation fault. >> #0 0x00420850 in handle_control_message (vser=0x3732bd0, >> buf=0x2c173e0, len=8) at >> /home/lcapitulino/src/qmp-unstable/hw/virtio-serial-bus.c:335 >> 335 info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); > > Strange, I've not seen it so far in my testing (neither in the daily > test runs of the virtio-serial testsuite). > >> I've also bisected this and git points out to commit: >> >> commit a15bb0d6a981de749452a5180fc8084d625671da >> Author: Markus Armbruster >> Date: Wed May 25 14:21:13 2011 +0200 >> >> virtio-serial: Drop redundant VirtIOSerialPort member info >> >> I think what's happening is that the device is not initialized on a >> VIRTIO_CONSOLE_DEVICE_READY event. Really? I believe the device is initialized just fine, but the message refers to a nonexistant port. The check after find_port_by_id() suggests that's expected for VIRTIO_CONSOLE_DEVICE_READY. >>Moving the DO_UPCAST() call to >> the other events fixes the problem to me. >> >> Signed-off-by: Luiz Capitulino >> --- >> hw/virtio-serial-bus.c |4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c >> index 9a12104..579f676 100644 >> --- a/hw/virtio-serial-bus.c >> +++ b/hw/virtio-serial-bus.c >> @@ -332,8 +332,6 @@ static void handle_control_message(VirtIOSerial *vser, >> void *buf, size_t len) >> if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY) >> return; >> >> -info = DO_UPCAST(VirtIOSerialPortInfo, qdev, port->dev.info); >> - > > Ah - this missed the !port check. It should be possible to do this in > a 'if (port)' block instead of replicating in the individual case > statements. You might have to do something like info = port ? DO_UPCAST(...) : NULL to avoid warnings about info used uninitialized. [...]
Re: [Qemu-devel] [PATCH] Fix ioapic vmstate
On 2011-06-17 09:30, Pavel Dovgaluk wrote: > This patch fixes save/restore vmstate of IOAPIC. > When irr member of IOAPICState is not saved and loaded, > restoring becomes non-deterministic, because irr is kept from > state of VM that was before loading. > > Signed-off-by: Pavel Dovgalyuk > --- > hw/ioapic.c |1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/hw/ioapic.c b/hw/ioapic.c > index 2109568..e583284 100644 > --- a/hw/ioapic.c > +++ b/hw/ioapic.c > @@ -207,6 +207,7 @@ static const VMStateDescription vmstate_ioapic = { > .fields = (VMStateField []) { > VMSTATE_UINT8(id, IOAPICState), > VMSTATE_UINT8(ioregsel, IOAPICState), > +VMSTATE_UINT32(irr, IOAPICState), > VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS), > VMSTATE_END_OF_LIST() > } > You are using an old qemu version. This was fixed via commit 35a74c5c59. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH] Fix ioapic vmstate
This patch fixes save/restore vmstate of IOAPIC. When irr member of IOAPICState is not saved and loaded, restoring becomes non-deterministic, because irr is kept from state of VM that was before loading. Signed-off-by: Pavel Dovgalyuk --- hw/ioapic.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/ioapic.c b/hw/ioapic.c index 2109568..e583284 100644 --- a/hw/ioapic.c +++ b/hw/ioapic.c @@ -207,6 +207,7 @@ static const VMStateDescription vmstate_ioapic = { .fields = (VMStateField []) { VMSTATE_UINT8(id, IOAPICState), VMSTATE_UINT8(ioregsel, IOAPICState), +VMSTATE_UINT32(irr, IOAPICState), VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS), VMSTATE_END_OF_LIST() }
Re: [Qemu-devel] [PATCH] CPU consumption optimization of 'qemu-img convert' using bdrv_is_allocated()
On Thu, Jun 16, 2011 at 9:10 AM, Dmitry Konishchev wrote: > On Wed, Jun 15, 2011 at 5:57 PM, Stefan Hajnoczi wrote: >> Anyway, bdrv_getlength() will return the total_sectors value instead >> of calling into raw-posix.c .bdrv_getlength(). That's why it should >> be cheap. > > Yeah, I see it now after a closer look in the drivers code. It looks > like I get this 9% slowdown just because for my particular test > bdrv_get_geometry() is called 37,348,536 times, which is a big number > even for function which calls another function which checks a few IFs > and returns a multiplication of two numbers. Ultimately the QEMU block layer should make total_sectors always up-to-date so it can be fetched cheaply. This requires looking at all the block drivers and considering when device size may change at runtime. Your patch improves backing file convert cases without regressing other cases too much. If you continue optimizing qemu-img, consider profiling the block operations and improving the core block driver performance instead of the outer loop of the qemu-img command. That way running VMs will benefit from your optimizations too. And the simple outer loop may be acceptable if the operation inside it is fast enough. Kevin: Are you happy with this patch? Stefan