Re: [Qemu-devel] Some uncertain about implementing stream optimized writing
No error report with writing (dd of=/dev/sda ...), but can't change data, (dd if=/dev/sda) dump unchanged. On Wed, Jun 29, 2011 at 1:47 PM, Stefan Hajnoczi wrote: > On Wed, Jun 29, 2011 at 5:47 AM, Fam Zheng wrote: >> Stream optimized VMDK image allocates minimized space for a compressed >> cluster, which means if there is high compress ratio, a cluster >> possibly only takes one physical sector in the file. It makes >> overwriting hard, especially when new data needs more sectors than >> previously allocated. >> >> Attach the image to VMware and boot the VM to test this format, it >> seems that VMware wouldn’t do write to allocated clusters, but can >> allocate new cluster to save data. > > What happens when the VM writes to allocated clusters? EIO? > > Stefan > -- Best regards! Fam Zheng
Re: [Qemu-devel] Some uncertain about implementing stream optimized writing
On Wed, Jun 29, 2011 at 5:47 AM, Fam Zheng wrote: > Stream optimized VMDK image allocates minimized space for a compressed > cluster, which means if there is high compress ratio, a cluster > possibly only takes one physical sector in the file. It makes > overwriting hard, especially when new data needs more sectors than > previously allocated. > > Attach the image to VMware and boot the VM to test this format, it > seems that VMware wouldn’t do write to allocated clusters, but can > allocate new cluster to save data. What happens when the VM writes to allocated clusters? EIO? Stefan
Re: [Qemu-devel] KVM call agenda for June 28
On Tue, Jun 28, 2011 at 8:41 PM, Marcelo Tosatti wrote: > On Tue, Jun 28, 2011 at 02:38:15PM +0100, Stefan Hajnoczi wrote: >> On Mon, Jun 27, 2011 at 3:32 PM, Juan Quintela wrote: >> > Please send in any agenda items you are interested in covering. >> >> Live block copy and image streaming: >> * The differences between Marcelo and Kevin's approaches >> * Which approach to choose and who can help implement it > > After more thinking, i dislike the image metadata approach. Management > must carry the information anyway, so its pointless to duplicate it > inside an image format. I agree with you. It would be a significant change for QEMU users to deal with block state files just in case they want to use live block copy/image streaming. Not only would existing management layers need to be updated but also custom management or provisioning scripts. > After the discussion today, i think the internal mechanism and interface > should be different for copy and stream: > > block copy > -- > > With backing files: > > 1) base <- sn1 <- sn2 > 2) base <- copy > > Without: > > 1) source > 2) destination > > Copy is only valid after switch has been performed. Same interface and > crash recovery characteristics for all image formats. > > If management wants to support continuation, it must specify > blkcopy:sn2:copy on startup. > > stream > -- > > 1) base <- remote > 2) base <- remote <- local > 3) base <- local > > "local" image is always valid. Requires backing file support. I agree that the modes of operation are different and we should provide different HMP/QMP APIs for them. Internally I still think they can share code for the source -> destination copy operation. Stefan
Re: [Qemu-devel] SPARC64 support on FreeBSD, has it improved as of yet?
Super Bisquit wrote: > ... > > It builds, doesn't run. More like it runs and hangs. > > $ qemu-system-sparc -cpu LEON3 -hda test.img -cdrom > Downloads/debian-6.0.2.1-sparc-businesscard.iso -m 256 -boot d > That command line won't work. OpenBIOS doesn't support LEON, and the last version of Debian for sparc32 was 4.0. Try instead: "qemu-system-sparc -cdrom debian-40r9-sparc-netinst.iso -boot d" You can get a cd image from http://cdimage.debian.org/cdimage/archive/4.0_r9/sparc/iso-cd/ but the installer may not be able to load packages from the internet because the packages have been moved to archive.debian.org. Bob
[Qemu-devel] Some uncertain about implementing stream optimized writing
Hi, I have implemented reading for sparse optimized and come to implement writing. It is a little complicated and I am not sure what is the best approach. Could you give me some advice? Here is the details: (pasted from http://warm.la/soc/?p=98) Stream optimized VMDK image allocates minimized space for a compressed cluster, which means if there is high compress ratio, a cluster possibly only takes one physical sector in the file. It makes overwriting hard, especially when new data needs more sectors than previously allocated. Attach the image to VMware and boot the VM to test this format, it seems that VMware wouldn’t do write to allocated clusters, but can allocate new cluster to save data. Overwriting existing cluster requires measuring new data size and deciding whether in-place overwrite is OK, if not we must look for other free space. Metadata in image has no such information for sector allocation algorithm, so if we want to fully enable writing to stream optimized, sector allocation bitmap will be introduced into block state. This should significantly increase memory usage and somehow complicate the driver. Another approach I think of is to allocate each non-inplace at the end of image and leave the old allocation unreferenced, which wastes disk space. -- Best regards! Fam Zheng
Re: [Qemu-devel] [PATCH 0/2] iothread improvements for Mac OS X
Hi Paolo, > Ping? 1/2 is probably somehow working around the sigmask problem fixed by > Alexandre (Mac people, can you check?), but it is way more readable than the > fair_mutex IMNSHO. I would be surprised if 2/2 also turned out to be a > workaround, but even if this were the case, it makes CPU usage lower. Those patches (I tested them together) do indeed appear to work around the freezes I was encountering with io-thread enabled on OS X. I get dma timeout errors when trying to boot Linux, but they don't seem related to this patch as I got the same errors with the patches I submitted a couple of weeks ago. Alexandre -- Tested-by: Alexandre Raymond
Re: [Qemu-devel] [PATCH v2 1/2] coreaudio: Fix OSStatus format specifier
Sorry for the delay. Commit 744d3644181ddb16ef5944a0f9217e46961c8c84 works fine on OSX 10.6. Alexandre On Thu, Jun 23, 2011 at 10:58 AM, malc wrote: > On Thu, 23 Jun 2011, Andreas F?rber wrote: > >> OSStatus type is defined as SInt32. That's signed int on __LP64__ and >> signed long otherwise. >> Since it is an explicit 32-bit-width type, cast to corresponsing POSIX type >> and use PRId32 format specifier. This avoids a warning on ppc64. >> > > [..snip..] > > Applied thanks (ditto UInt32 patch). > > -- > mailto:av1...@comtv.ru >
[Qemu-devel] [PATCH v4] showing a splash picture when start
Made an option to let qemu pass a picture to bios, let the bios show it as a
logo. By default it is off, enable it as following
-boot splash=P<,splash-time=T>
P is jpg/bmp file name or an absolute path, specifying it would enable log.
T have a max value of 0x, unit is ms, and its predefined value is 2500ms.
Signed-off-by: Wayne Xia
---
hw/fw_cfg.c | 141 -
qemu-config.c | 27 +++
sysemu.h |3 +
vl.c | 17 +++-
4 files changed, 186 insertions(+), 2 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index 85c8c3c..3ef5f29 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -26,6 +26,7 @@
#include "isa.h"
#include "fw_cfg.h"
#include "sysbus.h"
+#include "qemu-error.h"
/* debug firmware config */
//#define DEBUG_FW_CFG
@@ -56,6 +57,144 @@ struct FWCfgState {
Notifier machine_ready;
};
+#define JPG_FILE 0
+#define BMP_FILE 1
+
+static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
+{
+FILE *fp = NULL;
+int fop_ret;
+int file_size;
+int file_type = -1;
+unsigned char buf[2] = {0, 0};
+unsigned int filehead_value = 0;
+int bmp_bpp;
+
+fp = fopen(filename, "rb");
+if (fp == NULL) {
+error_report("failed to open file '%s'.", filename);
+return fp;
+}
+/* check file size */
+fseek(fp, 0L, SEEK_END);
+file_size = ftell(fp);
+if (file_size < 2) {
+error_report("file size is less than 2 bytes '%s'.", filename);
+fclose(fp);
+fp = NULL;
+return fp;
+}
+/* check magic ID */
+fseek(fp, 0L, SEEK_SET);
+fop_ret = fread(buf, 1, 2, fp);
+filehead_value = (buf[0] + (buf[1] << 8)) & 0x;
+if (filehead_value == 0xd8ff) {
+file_type = JPG_FILE;
+} else {
+if (filehead_value == 0x4d42) {
+file_type = BMP_FILE;
+}
+}
+if (file_type < 0) {
+error_report("'%s' not jpg/bmp file,head:0x%x.",
+ filename, filehead_value);
+fclose(fp);
+fp = NULL;
+return fp;
+}
+/* check BMP bpp */
+if (file_type == BMP_FILE) {
+fseek(fp, 28, SEEK_SET);
+fop_ret = fread(buf, 1, 2, fp);
+bmp_bpp = (buf[0] + (buf[1] << 8)) & 0x;
+if (bmp_bpp != 24) {
+error_report("only 24bpp bmp file is supported.");
+fclose(fp);
+fp = NULL;
+return fp;
+}
+}
+/* return values */
+*file_sizep = file_size;
+*file_typep = file_type;
+return fp;
+}
+
+static void fw_cfg_bootsplash(FWCfgState *s)
+{
+int boot_splash_time = 2500; /* predifined time */
+const char *boot_splash_filename = NULL; /*default is off */
+char *p;
+char *filename;
+FILE *fp;
+int fop_ret;
+int file_size;
+int file_type = -1;
+const char *temp;
+
+/* get user configuration */
+QemuOptsList *plist = qemu_find_opts("boot-opts");
+QemuOpts *opts = QTAILQ_FIRST(&plist->head);
+if (opts != NULL) {
+temp = qemu_opt_get(opts, "splash");
+if (temp != NULL) {
+boot_splash_filename = temp;
+}
+temp = qemu_opt_get(opts, "splash-time");
+if (temp != NULL) {
+p = (char *)temp;
+boot_splash_time = strtol(p, (char **)&p, 10);
+}
+}
+
+/* check user configuration */
+if (boot_splash_filename == NULL) {
+/* do nothing, directly return */
+return;
+}
+if (boot_splash_time > 0x) {
+error_report("splash time is big than 65535, force it to 65535.");
+boot_splash_time = 65535;
+}
+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, boot_splash_filename);
+if (filename == NULL) {
+error_report("failed to find file '%s'.", boot_splash_filename);
+return;
+}
+
+/* probing the file */
+fp = probe_splashfile(filename, &file_size, &file_type);
+if (fp == NULL) {
+qemu_free(filename);
+return;
+}
+
+/* loading file data */
+if (boot_splash_filedata != NULL) {
+qemu_free(boot_splash_filedata);
+}
+boot_splash_filedata = qemu_malloc(file_size);
+boot_splash_filedata_size = file_size;
+fseek(fp, 0L, SEEK_SET);
+fop_ret = fread(boot_splash_filedata, 1, file_size, fp);
+fclose(fp);
+
+/* insert data */
+if (file_type == JPG_FILE) {
+fw_cfg_add_file(s, "bootsplash.jpg",
+boot_splash_filedata, boot_splash_filedata_size);
+} else {
+fw_cfg_add_file(s, "bootsplash.bmp",
+boot_splash_filedata, boot_splash_filedata_size);
+}
+/* use little endian format */
+qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff);
+qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff);
+fw_cfg_add_file(s, "qemu_extra_params_fw.cfg", qemu_extra_params_fw, 4);
+qemu_free(filename);
+}
Re: [Qemu-devel] SPARC64 support on FreeBSD, has it improved as of yet?
On Sun, Jun 26, 2011 at 1:58 PM, Blue Swirl wrote: > On Fri, Jun 24, 2011 at 3:52 AM, Super Bisquit > wrote: > > The last time I asked, Blue Swirl was somewhat working on the port. > > Has anything been improved since? > > I'm somewhat working on OpenBSD host support, not FreeBSD, but there > shouldn't be great differences. What's the status on FreeBSD, does > QEMU build and run? > http://lists.gnu.org/archive/html/qemu-devel/2011-04/msg02277.html Our last conversation on the subject. It builds, doesn't run. More like it runs and hangs. The core is ~428M in size. $ qemu-system-sparc -cpu LEON3 -hda test.img -cdrom Downloads/debian-6.0.2.1-sparc-businesscard.iso -m 256 -boot d qemu: fatal: Trap 0x02 while interrupts disabled, Error state pc: npc: 0004 General Registers: %g0-7: Current Register Window: %o0-7: %l0-7: %i0-7: Floating Point Registers: %f00: 0.00 0.00 0.00 0.00 %f04: 0.00 0.00 0.00 0.00 %f08: 0.00 0.00 0.00 0.00 %f12: 0.00 0.00 0.00 0.00 %f16: 0.00 0.00 0.00 0.00 %f20: 0.00 0.00 0.00 0.00 %f24: 0.00 0.00 0.00 0.00 %f28: 0.00 0.00 0.00 0.00 psr: f3c0 (icc: SPE: SP-) wim: 0001 fsr: 0008 y: Abort trap (core dumped) $ gdb qemu-system-sparc qemu-system-sparc.core GNU gdb 6.1.1 [FreeBSD] Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "sparc64-marcel-freebsd"...(no debugging symbols found)... warning: core file may not match specified executable file. Core was generated by `qemu-system-sparc'. Program terminated with signal 6, Aborted. Reading symbols from /lib/libthr.so.3...(no debugging symbols found)...done. Loaded symbols for /lib/libthr.so.3 Reading symbols from /lib/libutil.so.9...(no debugging symbols found)...done. Loaded symbols for /lib/libutil.so.9 Reading symbols from /usr/local/lib/libcurl.so.6...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libcurl.so.6 Reading symbols from /lib/libncurses.so.8...(no debugging symbols found)...done. Loaded symbols for /lib/libncurses.so.8 Reading symbols from /usr/local/lib/libgnutls.so.40...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libgnutls.so.40 Reading symbols from /lib/libpcap.so.7...(no debugging symbols found)...done. Loaded symbols for /lib/libpcap.so.7 Reading symbols from /usr/local/lib/libSDL-1.2.so.11...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libSDL-1.2.so.11 Reading symbols from /usr/local/lib/libX11.so.6...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libX11.so.6 Reading symbols from /lib/libm.so.5...(no debugging symbols found)...done. Loaded symbols for /lib/libm.so.5 Reading symbols from /lib/libz.so.6...(no debugging symbols found)...done. Loaded symbols for /lib/libz.so.6 Reading symbols from /lib/libc.so.7...(no debugging symbols found)...done. Loaded symbols for /lib/libc.so.7 Reading symbols from /usr/lib/libssl.so.6...(no debugging symbols found)...done. Loaded symbols for /usr/lib/libssl.so.6 Reading symbols from /lib/libcrypto.so.6...(no debugging symbols found)...done. Loaded symbols for /lib/libcrypto.so.6 Reading symbols from /usr/local/lib/libgcrypt.so.17...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libgcrypt.so.17 Reading symbols from /usr/local/lib/libgpg-error.so.0...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libgpg-error.so.0 Reading symbols from /usr/local/lib/libintl.so.9...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libintl.so.9 Reading symbols from /usr/local/lib/libiconv.so.3...(no debugging symbols found)...done. Loaded symbols for /usr/local/lib/libiconv.so.3 Reading symbols from /usr/local/lib/libggi.so.2...done. Loaded symbols for /usr/local/lib/libggi.so.2 Reading symbols from /usr/local/lib/libXxf86vm.so.1...done. Loaded symbols for /usr/local/lib/libXxf86vm.so.1 Reading symbols from /usr/local/lib/libgii.so.1...done. Loaded symbols for /usr/local/lib/libgii.so.1 Reading symbols from /usr/local/lib/libXxf86dga
[Qemu-devel] [Bug 568614] Re: x86_64 host curses interface: spacing/garbling
any news on that bug? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/568614 Title: x86_64 host curses interface: spacing/garbling Status in QEMU: In Progress Bug description: Environment: Arch Linux x86_64, kernel 2.6.33, qemu 0.12.3 Steps to reproduce: 1. Have a host system running 64-bit Linux. 2. Start a qemu VM with the -curses flag. Expected results: Text displayed looks as it would on a real text-mode display, and VM is therefore usable. Actual results: Text displayed contains an extra space between characters, causing text to flow off the right and bottom sides of the screen. This makes the curses interface unintelligible. The attached patch fixes this problem on 0.12.3 on my installation without changing behavior on a 32-bit machine. I don't know enough of the semantics of console_ch_t to know if this is the "correct" fix or if there should be, say, an extra cast somewhere instead. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/568614/+subscriptions
Re: [Qemu-devel] KVM call agenda for June 28
On Tue, Jun 28, 2011 at 02:38:15PM +0100, Stefan Hajnoczi wrote: > On Mon, Jun 27, 2011 at 3:32 PM, Juan Quintela wrote: > > Please send in any agenda items you are interested in covering. > > Live block copy and image streaming: > * The differences between Marcelo and Kevin's approaches > * Which approach to choose and who can help implement it After more thinking, i dislike the image metadata approach. Management must carry the information anyway, so its pointless to duplicate it inside an image format. After the discussion today, i think the internal mechanism and interface should be different for copy and stream: block copy -- With backing files: 1) base <- sn1 <- sn2 2) base <- copy Without: 1) source 2) destination Copy is only valid after switch has been performed. Same interface and crash recovery characteristics for all image formats. If management wants to support continuation, it must specify blkcopy:sn2:copy on startup. stream -- 1) base <- remote 2) base <- remote <- local 3) base <- local "local" image is always valid. Requires backing file support.
Re: [Qemu-devel] [PATCH] Command line support for altering the log file location
On Wed, Jun 08, 2011 at 12:32:40PM +1000, Matthew Fernandez wrote:
> Add command line support for logging to a location other than /tmp/qemu.log.
>
> With logging enabled (command line option -d), the log is written to
> the hard-coded path /tmp/qemu.log. This patch adds support for writing
> the log to a different location by passing the -D option.
>
> Signed-off-by: Matthew Fernandez
>
Hi,
I've applied the following, only tested on linux-user.
Cheers
commit 1dfdcaa83f9ce34aded8bc0669e81753d94f1b7d
Author: Edgar E. Iglesias
Date: Tue Jun 28 20:57:09 2011 +0200
user: Fix -d debug logging for usermode emulation
Signed-off-by: Edgar E. Iglesias
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 5f790b2..6018a41 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -866,7 +866,7 @@ int main(int argc, char **argv)
int mask;
const CPULogItem *item;
-mask = cpu_str_to_log_mask(r);
+mask = cpu_str_to_log_mask(log_mask);
if (!mask) {
printf("Log items (comma separated):\n");
for (item = cpu_log_items; item->mask != 0; item++) {
diff --git a/darwin-user/main.c b/darwin-user/main.c
index a6dc859..35196a1 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -819,7 +819,7 @@ int main(int argc, char **argv)
int mask;
CPULogItem *item;
-mask = cpu_str_to_log_mask(r);
+mask = cpu_str_to_log_mask(log_mask);
if (!mask) {
printf("Log items (comma separated):\n");
for (item = cpu_log_items; item->mask != 0; item++) {
diff --git a/linux-user/main.c b/linux-user/main.c
index db5577b..289054b 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3030,7 +3030,7 @@ int main(int argc, char **argv, char **envp)
int mask;
const CPULogItem *item;
-mask = cpu_str_to_log_mask(r);
+mask = cpu_str_to_log_mask(log_mask);
if (!mask) {
printf("Log items (comma separated):\n");
for (item = cpu_log_items; item->mask != 0; item++) {
Re: [Qemu-devel] [PATCH] Command line support for altering the log file location
On Wed, Jun 08, 2011 at 12:32:40PM +1000, Matthew Fernandez wrote:
> Add command line support for logging to a location other than /tmp/qemu.log.
>
> With logging enabled (command line option -d), the log is written to
> the hard-coded path /tmp/qemu.log. This patch adds support for writing
> the log to a different location by passing the -D option.
>
> Signed-off-by: Matthew Fernandez
Hi,
This patch broke -d for all *-user targets AFAICT.
r is passed to cpu_str_to_log_mask(r) instead of log_mask.
Cheers
> } else if (!strcmp(r, "d")) {
> -int mask;
> -const CPULogItem *item;
> -
> -if (optind >= argc)
> +if (optind >= argc) {
> break;
> -
> -r = argv[optind++];
> -mask = cpu_str_to_log_mask(r);
> -if (!mask) {
> -printf("Log items (comma separated):\n");
> -for(item = cpu_log_items; item->mask != 0; item++) {
> -printf("%-10s %s\n", item->name, item->help);
> -}
> -exit(1);
> }
> -cpu_set_log(mask);
> +log_mask = argv[optind++];
> +} else if (!strcmp(r, "D")) {
> +if (optind >= argc) {
> +break;
> +}
> +log_file = argv[optind++];
> } else if (!strcmp(r, "E")) {
> r = argv[optind++];
> if (envlist_setenv(envlist, r) != 0)
> @@ -867,6 +860,23 @@ int main(int argc, char **argv)
> usage();
> filename = argv[optind];
>
> +/* init debug */
> +cpu_set_log_filename(log_file);
> +if (log_mask) {
> +int mask;
> +const CPULogItem *item;
> +
> +mask = cpu_str_to_log_mask(r);
Re: [Qemu-devel] [PATCH] [PowerPC][RFC] booke timers
On Tue, 28 Jun 2011 15:35:00 +0200
Fabien Chouteau wrote:
> Subject: Re: [Qemu-devel] [PATCH] [PowerPC][RFC] booke timers
> To:Scott Wood
> Cc:qemu-devel@nongnu.org
> Bcc:
> -=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-w
> On 27/06/2011 22:28, Scott Wood wrote:
> > On Mon, 27 Jun 2011 18:14:06 +0200
> > Fabien Chouteau wrote:
> >
> >> While working on the emulation of the freescale p2010 (e500v2) I realized
> >> that
> >> there's no implementation of booke's timers features. Currently mpc8544
> >> uses
> >> ppc_emb (ppc_emb_timers_init) which is close but not exactly like booke
> >> (for
> >> example booke uses different SPR).
> >
> > ppc_emb_timers_init should be renamed something less generic, then.
>
> Agreed, can you help me to find a better name?
What chips are covered by it? 40x?
> Maybe I can do something like:
>
> static uint64_t booke_get_fit_target(CPUState *env)
> {
> uint32_t fp = (env->spr[SPR_BOOKE_TCR] & TCR_FP_MASK) >> TCR_FP_SHIFT;
>
> /* Only for e500 */
> if (/* CPU is e500 */) {
> uint32_t fpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_FPEXT_MASK)
> >> TCR_E500_FPEXT_SHIFT;
> fp |= fpext << 2;
> } else {
> fp = env->fit_period[fp];
> }
>
> return 1 << fp;
> }
Looks good -- or just have a CPU-specific function pointer that extracts
the period from TCR.
> > I think some changes in the decrementer code are needed to provide booke
> > semantics -- no raising the interrupt based on the high bit of decr, and
> > stop counting when reach zero.
>
> Can you please explain, I don't see where I'm not compliant with booke's
> decrementer.
It's not an issue with this code specifically, but existing behavior in the
decrementer code that isn't appropriate for booke.
On classic/server powerpc, when decrementer hits zero, it wraps around, and
the upper bit of DECR is used to signal the interrupt. On booke, when
decrementer hits zero, it stops, and the upper bit of DECR is not special.
> >> +void store_booke_tsr (CPUState *env, target_ulong val)
> >> +{
> >> +ppc_tb_t *tb_env = env->tb_env;
> >> +booke_timer_t *booke_timer;
> >> +
> >> +booke_timer = tb_env->opaque;
> >> +
> >> +env->spr[SPR_BOOKE_TSR] &= ~(val & 0xFC00);
> >
> > Do we really need the "& 0xFC00"? Likewise in TCR.
>
> It's just a mask to keep only the defined bits.
Just seems unnecessary, and potentially harmful if CPU-specific code wants
to interpret implementation-defined bits, or if new bits are architected
in the future.
> >> +if (val & TSR_DIS) {
> >> +ppc_set_irq(env, booke_timer->decr_excp, 0);
> >> +}
> >> +
> >> +if (val & TSR_FIS) {
> >> +ppc_set_irq(env, booke_timer->fit_excp, 0);
> >> +}
> >> +
> >> +if (val & TSR_WIS) {
> >> +ppc_set_irq(env, booke_timer->wdt_excp, 0);
> >> +}
> >> +}
> >
> > It looks like ppc_hw_interrupt() is currently treating these as
> > edge-triggered, deasserting upon delivery. It should be level for booke.
>
> This is beyond the scope of this patch.
It's part of correctly implementing booke timers.
> >> +void store_booke_tcr (CPUState *env, target_ulong val)
> >> +{
> >> +ppc_tb_t *tb_env = env->tb_env;
> >> +booke_timer_t *booke_timer = tb_env->opaque;
> >> +
> >> +tb_env = env->tb_env;
> >> +env->spr[SPR_BOOKE_TCR] = val & 0xFFC0;
> >> +
> >> +booke_update_fixed_timer(env,
> >> + booke_get_fit_target(env),
> >> + &booke_timer->fit_next,
> >> + booke_timer->fit_timer);
> >> +
> >> +booke_update_fixed_timer(env,
> >> + booke_get_wdt_target(env),
> >> + &booke_timer->wdt_next,
> >> + booke_timer->wdt_timer);
> >> +}
> >
> > Check for FIS/DIS/WIS -- the corresponding enable bit might have just been
> > set.
>
> Can you explain, I don't see the problem.
If a decrementer fires with TCR[DIE] unset, it won't be delivered, but
TSR[DIS] will be set.
If a guest subsequenly sets TCR[DIE], without having first cleared TSR[DIS],
the interrupt should fire immediately -- but that will only happen if you
check for it here.
-Scott
Re: [Qemu-devel] [PATCH] arm-semi: Provide access to CLI arguments passed through the "-append" option
2011/6/16 Cédric VINCENT :
> This patch basically adapts the new semi-hosting command-line support
> -- introduced by Wolfgang Schildbach in the commit 2e8785ac -- for use
> in system-mode.
Generally looks OK. Some nits:
> -/* Build a commandline from the original argv. */
> +/* Build a command-line from the original argv.
> + *
If you're going to expand this into a multiline comment I'd prefer
it to be inside the brace rather than outside.
> + if (!input_size || output_size > input_size) {
> + /* Not enough space to store command-line arguments. */
> + return -1;
You can drop the check for !input_size here, because you've eliminated
the case where output_size == 0, and so a zero input_size will be
caught by the other half of the conditional.
> + /* Separate arguments by white spaces. */
> + for (i = 0; i < output_size; i++) {
> + if (output_buffer[i] == 0) {
> + output_buffer[i] = ' ';
> + }
> + }
This will turn the final trailing NUL into a space -- should
be "i < output_size - 1".
> + out:
> +#endif
> + /* Unlock the buffer on the ARM side. */
> + unlock_user(output_buffer, ARG(0), output_size);
>
> - /* Return success if we could return a commandline. */
> - return (arm_cmdline_buffer && host_cmdline_buffer) ? 0 : -1;
> + return status;
> }
> -#else
> - return -1;
> -#endif
> + /* Never reached. */
This is kind of obvious so I think the comment is unnecessary.
-- PMM
[Qemu-devel] [PATCH v5 08/10] trace-state: [simple] disable all trace points by default
Note that this refers to the backend-specific state (whether the output must be
generated), not the event "disabled" property (which always uses the "nop"
backend).
Signed-off-by: Lluís Vilanova
---
scripts/tracetool |9 ++---
trace-events |3 ---
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/scripts/tracetool b/scripts/tracetool
index e2cf117..c740080 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -221,15 +221,10 @@ EOF
linetoc_simple()
{
-local name state
+local name
name=$(get_name "$1")
-if has_property "$1" "disable"; then
-state="0"
-else
-state="1"
-fi
cat < must be a valid as a C function name.
#
Re: [Qemu-devel] [PATCH] Clean up virtio-9p error handling code
On 28 June 2011 15:22, Venkateswararao Jujjuri wrote:
> **
> On 06/28/2011 05:25 AM, Sassan Panahinejad wrote:
>
> Hi JV,
>
> Any progress regarding merging this patch (and the fsync patch I
> submitted)?
> Is there anything I can do to assist/speed the process?
>
> Sussan, Thanks for the ping. :)
>
> Everything is on hold waiting for 0.15 tag; Including threading/coroutine
> patches.
> As soon as we go in with the coroutines, you can just rebase your patch and
> we should be
> ready to go. I am waiting too. :-D
>
Thanks for the info :)
>
> Thanks,
> JV
>
>
>
> Thanks
> Sassan
>
>
> On 8 June 2011 17:21, Sassan Panahinejad wrote:
>
>> In a lot of cases, the handling of errors was quite ugly.
>> This patch moves reading of errno to immediately after the system calls
>> and passes it up through the system more cleanly.
>> Also, in the case of the xattr functions (and possibly others), completely
>> the wrong error was being returned.
>>
>>
>> This patch is created against your 9p-coroutine-bh branch, as requested.
>> Sorry for the delay, I was unexpectedly required to work abroad for 2 weeks.
>>
>> Signed-off-by: Sassan Panahinejad
>>
>> ---
>> fsdev/file-op-9p.h|4 +-
>> hw/9pfs/codir.c | 14 +
>> hw/9pfs/virtio-9p-local.c | 123
>> +
>> hw/9pfs/virtio-9p-xattr.c | 21 +++-
>> 4 files changed, 90 insertions(+), 72 deletions(-)
>>
>> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
>> index af9daf7..3d9575b 100644
>> --- a/fsdev/file-op-9p.h
>> +++ b/fsdev/file-op-9p.h
>> @@ -73,12 +73,12 @@ typedef struct FileOperations
>> int (*setuid)(FsContext *, uid_t);
>> int (*close)(FsContext *, int);
>> int (*closedir)(FsContext *, DIR *);
>> -DIR *(*opendir)(FsContext *, const char *);
>> +int (*opendir)(FsContext *, const char *, DIR **);
>> int (*open)(FsContext *, const char *, int);
>> int (*open2)(FsContext *, const char *, int, FsCred *);
>> void (*rewinddir)(FsContext *, DIR *);
>> off_t (*telldir)(FsContext *, DIR *);
>> -struct dirent *(*readdir)(FsContext *, DIR *);
>> +int (*readdir)(FsContext *, DIR *, struct dirent **);
>> void (*seekdir)(FsContext *, DIR *, off_t);
>> ssize_t (*preadv)(FsContext *, int, const struct iovec *, int, off_t);
>> ssize_t (*pwritev)(FsContext *, int, const struct iovec *, int,
>> off_t);
>> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
>> index 110289f..acbbb39 100644
>> --- a/hw/9pfs/codir.c
>> +++ b/hw/9pfs/codir.c
>> @@ -25,12 +25,7 @@ int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
>> struct dirent **dent)
>> {
>> errno = 0;
>> /*FIXME!! need to switch to readdir_r */
>> -*dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
>> -if (!*dent && errno) {
>> -err = -errno;
>> -} else {
>> -err = 0;
>> -}
>> +err = s->ops->readdir(&s->ctx, fidp->fs.dir, dent);
>> });
>> return err;
>> }
>> @@ -93,12 +88,7 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
>>
>> v9fs_co_run_in_worker(
>> {
>> -dir = s->ops->opendir(&s->ctx, fidp->path.data);
>> -if (!dir) {
>> -err = -errno;
>> -} else {
>> -err = 0;
>> -}
>> +err = s->ops->opendir(&s->ctx, fidp->path.data, &dir);
>> });
>> fidp->fs.dir = dir;
>> return err;
>> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
>> index 77904c3..65f35eb 100644
>> --- a/hw/9pfs/virtio-9p-local.c
>> +++ b/hw/9pfs/virtio-9p-local.c
>> @@ -28,7 +28,7 @@ static int local_lstat(FsContext *fs_ctx, const char
>> *path, struct stat *stbuf)
>> char buffer[PATH_MAX];
>> err = lstat(rpath(fs_ctx, path, buffer), stbuf);
>> if (err) {
>> -return err;
>> +return -errno;
>> }
>> if (fs_ctx->fs_sm == SM_MAPPED) {
>> /* Actual credentials are part of extended attrs */
>> @@ -53,7 +53,7 @@ static int local_lstat(FsContext *fs_ctx, const char
>> *path, struct stat *stbuf)
>> stbuf->st_rdev = tmp_dev;
>> }
>> }
>> -return err;
>> +return 0;
>> }
>>
>> static int local_set_xattr(const char *path, FsCred *credp)
>> @@ -63,28 +63,28 @@ static int local_set_xattr(const char *path, FsCred
>> *credp)
>> err = setxattr(path, "user.virtfs.uid", &credp->fc_uid,
>> sizeof(uid_t),
>> 0);
>> if (err) {
>> -return err;
>> +return -errno;
>> }
>> }
>> if (credp->fc_gid != -1) {
>> err = setxattr(path, "user.virtfs.gid", &credp->fc_gid,
>> sizeof(gid_t),
>> 0);
>> if (err) {
>> -return err;
>> +return -errno;
>> }
>> }
>> if (credp->fc_mode != -1) {
>> err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
>>
[Qemu-devel] [PATCH v5 06/10] trace-state: add "-trace events" argument to control initial state
The "-trace events" argument can be used to provide a file with a list of trace
event names that will be enabled prior to starting execution, thus providing
early tracing.
This saves the user from manually toggling event states through the monitor
interface or whichever backend-specific interface.
Signed-off-by: Lluís Vilanova
---
docs/tracing.txt |3 +++
qemu-config.c|3 +++
qemu-options.hx | 24 ++--
trace/control.c | 30 ++
trace/control.h |2 ++
vl.c |2 ++
6 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 017ff59..8f6e5c9 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -129,6 +129,9 @@ This functionality is also provided through monitor
commands:
* trace-event NAME on|off
Enable/disable a given trace event.
+The "-trace events=" command line argument can be used to enable the
+events listed in from the very beginning of the program.
+
== Trace backends ==
The "tracetool" script automates tedious trace event code generation and also
diff --git a/qemu-config.c b/qemu-config.c
index d187dc5..3fae102 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -302,6 +302,9 @@ static QemuOptsList qemu_trace_opts = {
.head = QTAILQ_HEAD_INITIALIZER(qemu_trace_opts.head),
.desc = {
{
+.name = "events",
+.type = QEMU_OPT_STRING,
+},{
.name = "file",
.type = QEMU_OPT_STRING,
},
diff --git a/qemu-options.hx b/qemu-options.hx
index b691e13..7e7b1f7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2394,17 +2394,29 @@ Normally QEMU loads a configuration file from
@var{sysconfdir}/qemu.conf and
option will prevent QEMU from loading these configuration files at startup.
ETEXI
DEF("trace", HAS_ARG, QEMU_OPTION_trace,
-"-trace\n"
-"Specify a trace file to log traces to\n",
+"-trace [events=][,file=]\n"
+"specify tracing options\n",
QEMU_ARCH_ALL)
STEXI
-HXCOMM This line is not accurate, as the option is backend-specific but HX does
-HXCOMM not support conditional compilation of text.
-@item -trace
+HXCOMM This line is not accurate, as some sub-options are backend-specific but
+HXCOMM HX does not support conditional compilation of text.
+@item -trace [events=@var{file}][,file=@var{file}]
@findex -trace
-Specify a trace file to log output traces to.
+
+Specify tracing options.
+
+@table @option
+@item events=@var{file}
+Immediately enable events listed in @var{file}.
+The file must contain one event name (as listed in the @var{trace-events} file)
+per line.
+
+This option is not available when using the @var{nop} tracing backend.
+@item file=@var{file}
+Log output traces to @var{file}.
This option is available only when using the @var{simple} tracing backend.
+@end table
ETEXI
HXCOMM This is the last statement. Insert new options before this line!
diff --git a/trace/control.c b/trace/control.c
index 6138eab..0086f1f 100644
--- a/trace/control.c
+++ b/trace/control.c
@@ -34,3 +34,33 @@ bool trace_event_set_state (const char *name, bool state)
#endif
return false;
}
+
+void trace_config_event_set_state (const char *fname)
+{
+if (fname == NULL) {
+return;
+}
+
+FILE *fp = fopen(fname, "r");
+if (!fp) {
+fprintf(stderr, "could not open trace events file '%s': %s\n",
+fname, strerror(errno));
+exit(1);
+}
+char line_buf[1024];
+while (fgets(line_buf, sizeof(line_buf), fp)) {
+size_t len = strlen(line_buf);
+if (len > 1) { /* skip empty lines */
+line_buf[len - 1] = '\0';
+if (!trace_event_set_state(line_buf, true)) {
+fprintf(stderr, "qemu: error: trace event '%s' does not
exist\n", line_buf);
+exit(1);
+}
+}
+}
+if (fclose(fp) != 0) {
+fprintf(stderr, "qemu: error: closing file '%s': %s\n",
+fname, strerror(errno));
+exit(1);
+}
+}
diff --git a/trace/control.h b/trace/control.h
index 37d3aa0..48f4b01 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -11,4 +11,6 @@
void trace_print_events (FILE *stream, fprintf_function stream_printf);
bool trace_event_set_state (const char *name, bool state);
+void trace_config_event_set_state (const char *fname);
+
#endif /* TRACE_CONTROL_H */
diff --git a/vl.c b/vl.c
index b766dc7..c95b186 100644
--- a/vl.c
+++ b/vl.c
@@ -156,6 +156,7 @@ int main(int argc, char **argv)
#include "slirp/libslirp.h"
#include "trace.h"
+#include "trace/control.h"
#include "trace/simple.h"
#include "qemu-queue.h"
#include "cpus.h"
@@ -2870,6 +2871,7 @@ int main(int argc, char **argv, char **envp)
fprintf(stderr, "qemu: option \"-%s\" is not supported by this
tracing backend\n", popt->name);
exit(1);
#endif
+trac
[Qemu-devel] [PATCH v5 07/10] trace-state: always use the "nop" backend on events with the "disable" keyword
Any event with the keyword/property "disable" generates an empty trace event
using the "nop" backend, regardless of the current backend.
Signed-off-by: Lluís Vilanova
---
docs/tracing.txt | 25 +++--
scripts/tracetool | 15 ++-
2 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 8f6e5c9..c443171 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -12,15 +12,11 @@ for debugging, profiling, and observing execution.
./configure --trace-backend=simple
make
-2. Enable trace events you are interested in:
-
-$EDITOR trace-events # remove "disable" from events you want
-
-3. Run the virtual machine to produce a trace file:
+2. Run the virtual machine to produce a trace file:
qemu ... # your normal QEMU invocation
-4. Pretty-print the binary trace file:
+3. Pretty-print the binary trace file:
./simpletrace.py trace-events trace-*
@@ -103,10 +99,11 @@ portability macros, ensure they are preceded and followed
by double quotes:
4. Name trace events after their function. If there are multiple trace events
in one function, append a unique distinguisher at the end of the name.
-5. Declare trace events with the "disable" property. Some trace events can
- produce a lot of output and users are typically only interested in a subset
- of trace events. Marking trace events disabled by default saves the user
- from having to manually disable noisy trace events.
+5. If specific trace events are going to be called a huge number of times, this
+ might have a noticeable performance impact even when the trace events are
+ programmatically disabled. In this case you should declare the trace event
+ with the "disable" property, which will effectively disable it at compile
+ time (using the "nop" backend).
== Generic interface and monitor commands ==
@@ -155,6 +152,9 @@ The "nop" backend generates empty trace event functions so
that the compiler
can optimize out trace events completely. This is the default and imposes no
performance penalty.
+Note that regardless of the selected trace backend, events with the "disable"
+property will be generated with the "nop" backend.
+
=== Stderr ===
The "stderr" backend sends trace events directly to standard error. This
@@ -163,6 +163,11 @@ effectively turns trace events into debug printfs.
This is the simplest backend and can be used together with existing code that
uses DPRINTF().
+Note that with this backend trace events cannot be programmatically
+enabled/disabled. Thus, in order to trim down the amount of output and the
+performance impact of tracing, you might want to add the "disable" property in
+the "trace-events" file for those events you are not interested in.
+
=== Simpletrace ===
The "simple" backend supports common use cases and comes as part of the QEMU
diff --git a/scripts/tracetool b/scripts/tracetool
index e649a5b..e2cf117 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -506,21 +506,10 @@ convert()
# Skip comments and empty lines
test -z "${str%%#*}" && continue
+echo
# Process the line. The nop backend handles disabled lines.
-disable="0"
if has_property "$str" "disable"; then
-disable="1"
-fi
-echo
-if [ "$disable" = "1" ]; then
-# Pass the disabled state as an arg for the simple
-# or DTrace backends which handle it dynamically.
-# For all other backends, call lineto$1_nop()
-if [ $backend = "simple" -o "$backend" = "dtrace" ]; then
-"$process_line" "$str"
-else
-"lineto$1_nop" "${str##disable }"
-fi
+"lineto$1_nop" "$str"
else
"$process_line" "$str"
fi
[Qemu-devel] [PATCH v5 04/10] trace-state: separate trace event control and query routines from the simple backend
Move the 'st_print_trace_events' and 'st_change_trace_event_state' into
backend-agnostic 'trace_print_events' and 'trace_event_set_state' (respectively)
in the "trace/control.c" file.
By moving them, other backends will later be able to provide their own
implementation.
Signed-off-by: Lluís Vilanova
---
Makefile.objs |1 +
hmp-commands.hx |2 +-
monitor.c | 13 +++--
trace/control.c | 36
trace/control.h | 14 ++
trace/simple.c | 23 ---
trace/simple.h |2 --
7 files changed, 59 insertions(+), 32 deletions(-)
create mode 100644 trace/control.c
create mode 100644 trace/control.h
diff --git a/Makefile.objs b/Makefile.objs
index 9a67374..fe22e8a 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -366,6 +366,7 @@ trace-obj-y += trace/simple.o
user-obj-y += qemu-timer-common.o
endif
endif
+trace-obj-y += trace/control.o
##
# smartcard
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 6ad8806..623e012 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -186,7 +186,7 @@ ETEXI
.args_type = "name:s,option:b",
.params = "name on|off",
.help = "changes status of a specific trace event",
-.mhandler.cmd = do_change_trace_event_state,
+.mhandler.cmd = do_trace_event_set_state,
},
STEXI
diff --git a/monitor.c b/monitor.c
index 67ceb46..f8954fa 100644
--- a/monitor.c
+++ b/monitor.c
@@ -58,7 +58,8 @@
#include "osdep.h"
#include "cpu.h"
#ifdef CONFIG_SIMPLE_TRACE
-#include "trace.h"
+#include "trace/simple.h"
+#include "trace/control.h"
#endif
#include "ui/qemu-spice.h"
@@ -593,11 +594,11 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
}
#ifdef CONFIG_SIMPLE_TRACE
-static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
+static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
{
const char *tp_name = qdict_get_str(qdict, "name");
bool new_state = qdict_get_bool(qdict, "option");
-int ret = st_change_trace_event_state(tp_name, new_state);
+int ret = trace_event_set_state(tp_name, new_state);
if (!ret) {
monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
@@ -1002,9 +1003,9 @@ static void do_info_trace(Monitor *mon)
st_print_trace((FILE *)mon, &monitor_fprintf);
}
-static void do_info_trace_events(Monitor *mon)
+static void do_trace_print_events(Monitor *mon)
{
-st_print_trace_events((FILE *)mon, &monitor_fprintf);
+trace_print_events((FILE *)mon, &monitor_fprintf);
}
#endif
@@ -3102,7 +3103,7 @@ static const mon_cmd_t info_cmds[] = {
.args_type = "",
.params = "",
.help = "show available trace-events & their state",
-.mhandler.info = do_info_trace_events,
+.mhandler.info = do_trace_print_events,
},
#endif
{
diff --git a/trace/control.c b/trace/control.c
new file mode 100644
index 000..6138eab
--- /dev/null
+++ b/trace/control.c
@@ -0,0 +1,36 @@
+#include "trace/control.h"
+
+#include "trace.h"
+
+
+void trace_print_events(FILE *stream, fprintf_function stream_printf)
+{
+#if defined(CONFIG_SIMPLE_TRACE)
+unsigned int i;
+
+for (i = 0; i < NR_TRACE_EVENTS; i++) {
+stream_printf(stream, "%s [Event ID %u] : state %u\n",
+ trace_list[i].tp_name, i, trace_list[i].state);
+}
+#else
+fprintf(stderr, "qemu: warning: cannot print the trace events with the
current backend\n");
+stream_printf(stream, "error: operation not supported with the current
backend\n");
+#endif
+}
+
+bool trace_event_set_state (const char *name, bool state)
+{
+#if defined(CONFIG_SIMPLE_TRACE)
+unsigned int i;
+
+for (i = 0; i < NR_TRACE_EVENTS; i++) {
+if (!strcmp(trace_list[i].tp_name, name)) {
+trace_list[i].state = state;
+return true;
+}
+}
+#else
+fprintf(stderr, "qemu: warning: cannot set the state of a trace event with
the current backend\n");
+#endif
+return false;
+}
diff --git a/trace/control.h b/trace/control.h
new file mode 100644
index 000..37d3aa0
--- /dev/null
+++ b/trace/control.h
@@ -0,0 +1,14 @@
+#ifndef TRACE_CONTROL_H
+#define TRACE_CONTROL_H
+
+#include
+#include
+#include
+
+#include "qemu-common.h"
+
+
+void trace_print_events (FILE *stream, fprintf_function stream_printf);
+bool trace_event_set_state (const char *name, bool state);
+
+#endif /* TRACE_CONTROL_H */
diff --git a/trace/simple.c b/trace/simple.c
index f1dbb5e..8aa3376 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -302,29 +302,6 @@ void st_print_trace(FILE *stream, int
(*stream_printf)(FILE *stream, const char
}
}
-void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream,
const char *fmt, ...))
-{
-unsigned int i;
-
-for (i = 0; i < NR_TRACE_EVENTS; i++) {
-stream_printf(
[Qemu-devel] [PATCH v5 05/10] trace-state: always compile support for controlling and querying trace event states
The current interface is generic for this small set of operations, and thus
other backends can easily modify the "trace/control.c" file to add their own
implementation.
Signed-off-by: Lluís Vilanova
---
docs/tracing.txt | 39 +--
hmp-commands.hx |7 +--
monitor.c|8
3 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 1ad106a..017ff59 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -108,6 +108,27 @@ portability macros, ensure they are preceded and followed
by double quotes:
of trace events. Marking trace events disabled by default saves the user
from having to manually disable noisy trace events.
+== Generic interface and monitor commands ==
+
+You can programmatically query and control the dynamic state of trace events
+through a backend-agnostic interface:
+
+* trace_print_events
+
+* trace_event_set_state
+
+Note that some of the backends do not provide and implementation for this
+interface, in which case QEMU will just print a warning.
+
+This functionality is also provided through monitor commands:
+
+* info trace-events
+ View available trace events and their state. State 1 means enabled, state 0
+ means disabled.
+
+* trace-event NAME on|off
+ Enable/disable a given trace event.
+
== Trace backends ==
The "tracetool" script automates tedious trace event code generation and also
@@ -157,27 +178,9 @@ unless you have specific needs for more advanced backends.
flushed and emptied. This means the 'info trace' will display few or no
entries if the buffer has just been flushed.
-* info trace-events
- View available trace events and their state. State 1 means enabled, state 0
- means disabled.
-
-* trace-event NAME on|off
- Enable/disable a given trace event.
-
* trace-file on|off|flush|set
Enable/disable/flush the trace file or set the trace file name.
- Enabling/disabling trace events programmatically
-
-The st_change_trace_event_state() function can be used to enable or disable
trace
-events at runtime inside QEMU:
-
-#include "trace.h"
-
-st_change_trace_event_state("virtio_irq", true); /* enable */
-[...]
-st_change_trace_event_state("virtio_irq", false); /* disable */
-
Analyzing trace files
The "simple" backend produces binary trace files that can be formatted with the
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 623e012..90804b1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -180,7 +180,6 @@ STEXI
Output logs to @var{filename}.
ETEXI
-#ifdef CONFIG_SIMPLE_TRACE
{
.name = "trace-event",
.args_type = "name:s,option:b",
@@ -195,6 +194,7 @@ STEXI
changes status of a trace event
ETEXI
+#if defined(CONFIG_SIMPLE_TRACE)
{
.name = "trace-file",
.args_type = "op:s?,arg:F?",
@@ -1360,10 +1360,13 @@ ETEXI
STEXI
@item info trace
show contents of trace buffer
+ETEXI
+#endif
+
+STEXI
@item info trace-events
show available trace events and their state
ETEXI
-#endif
STEXI
@end table
diff --git a/monitor.c b/monitor.c
index f8954fa..c28a43d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -57,9 +57,9 @@
#include "json-parser.h"
#include "osdep.h"
#include "cpu.h"
+#include "trace/control.h"
#ifdef CONFIG_SIMPLE_TRACE
#include "trace/simple.h"
-#include "trace/control.h"
#endif
#include "ui/qemu-spice.h"
@@ -593,7 +593,6 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
help_cmd(mon, qdict_get_try_str(qdict, "name"));
}
-#ifdef CONFIG_SIMPLE_TRACE
static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
{
const char *tp_name = qdict_get_str(qdict, "name");
@@ -605,6 +604,7 @@ static void do_trace_event_set_state(Monitor *mon, const
QDict *qdict)
}
}
+#ifdef CONFIG_SIMPLE_TRACE
static void do_trace_file(Monitor *mon, const QDict *qdict)
{
const char *op = qdict_get_try_str(qdict, "op");
@@ -1002,12 +1002,12 @@ static void do_info_trace(Monitor *mon)
{
st_print_trace((FILE *)mon, &monitor_fprintf);
}
+#endif
static void do_trace_print_events(Monitor *mon)
{
trace_print_events((FILE *)mon, &monitor_fprintf);
}
-#endif
/**
* do_quit(): Quit QEMU execution
@@ -3098,6 +3098,7 @@ static const mon_cmd_t info_cmds[] = {
.help = "show current contents of trace buffer",
.mhandler.info = do_info_trace,
},
+#endif
{
.name = "trace-events",
.args_type = "",
@@ -3105,7 +3106,6 @@ static const mon_cmd_t info_cmds[] = {
.help = "show available trace-events & their state",
.mhandler.info = do_trace_print_events,
},
-#endif
{
.name = NULL,
},
[Qemu-devel] [PATCH v5 10/10] trace: enable all events
Given that all events with programmatically-controlled state are disabled by default, we can delete the "disable" property from all events. Signed-off-by: Lluís Vilanova --- trace-events | 566 +- 1 files changed, 283 insertions(+), 283 deletions(-) diff --git a/trace-events b/trace-events index 7551105..66a2567 100644 --- a/trace-events +++ b/trace-events @@ -26,384 +26,384 @@ # The should be a sprintf()-compatible format string. # qemu-malloc.c -disable qemu_malloc(size_t size, void *ptr) "size %zu ptr %p" -disable qemu_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p" -disable qemu_free(void *ptr) "ptr %p" +qemu_malloc(size_t size, void *ptr) "size %zu ptr %p" +qemu_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu newptr %p" +qemu_free(void *ptr) "ptr %p" # osdep.c -disable qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p" -disable qemu_vmalloc(size_t size, void *ptr) "size %zu ptr %p" -disable qemu_vfree(void *ptr) "ptr %p" +qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p" +qemu_vmalloc(size_t size, void *ptr) "size %zu ptr %p" +qemu_vfree(void *ptr) "ptr %p" # hw/virtio.c -disable virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u" -disable virtqueue_flush(void *vq, unsigned int count) "vq %p count %u" -disable virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int out_num) "vq %p elem %p in_num %u out_num %u" -disable virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p" -disable virtio_irq(void *vq) "vq %p" -disable virtio_notify(void *vdev, void *vq) "vdev %p vq %p" +virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u" +virtqueue_flush(void *vq, unsigned int count) "vq %p count %u" +virtqueue_pop(void *vq, void *elem, unsigned int in_num, unsigned int out_num) "vq %p elem %p in_num %u out_num %u" +virtio_queue_notify(void *vdev, int n, void *vq) "vdev %p n %d vq %p" +virtio_irq(void *vq) "vq %p" +virtio_notify(void *vdev, void *vq) "vdev %p vq %p" # block.c -disable multiwrite_cb(void *mcb, int ret) "mcb %p ret %d" -disable bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d" -disable bdrv_aio_multiwrite_earlyfail(void *mcb) "mcb %p" -disable bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d" -disable bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p" -disable bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" -disable bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" -disable bdrv_set_locked(void *bs, int locked) "bs %p locked %d" +multiwrite_cb(void *mcb, int ret) "mcb %p ret %d" +bdrv_aio_multiwrite(void *mcb, int num_callbacks, int num_reqs) "mcb %p num_callbacks %d num_reqs %d" +bdrv_aio_multiwrite_earlyfail(void *mcb) "mcb %p" +bdrv_aio_multiwrite_latefail(void *mcb, int i) "mcb %p i %d" +bdrv_aio_flush(void *bs, void *opaque) "bs %p opaque %p" +bdrv_aio_readv(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" +bdrv_aio_writev(void *bs, int64_t sector_num, int nb_sectors, void *opaque) "bs %p sector_num %"PRId64" nb_sectors %d opaque %p" +bdrv_set_locked(void *bs, int locked) "bs %p locked %d" # hw/virtio-blk.c -disable virtio_blk_req_complete(void *req, int status) "req %p status %d" -disable virtio_blk_rw_complete(void *req, int ret) "req %p ret %d" -disable virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu" +virtio_blk_req_complete(void *req, int status) "req %p status %d" +virtio_blk_rw_complete(void *req, int ret) "req %p ret %d" +virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p sector %"PRIu64" nsectors %zu" # posix-aio-compat.c -disable paio_submit(void *acb, void *opaque, int64_t sector_num, int nb_sectors, int type) "acb %p opaque %p sector_num %"PRId64" nb_sectors %d type %d" -disable paio_complete(void *acb, void *opaque, int ret) "acb %p opaque %p ret %d" -disable paio_cancel(void *acb, void *opaque) "acb %p opaque %p" +paio_submit(void *acb, void *opaque, int64_t sector_num, int nb_sectors, int type) "acb %p opaque %p sector_num %"PRId64" nb_sectors %d type %d" +paio_complete(void *acb, void *opaque, int ret) "acb %p opaque %p ret %d" +paio_cancel(void *acb, void *opaque) "acb %p opaque %p" # ioport.c -disable cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u" -disable cpu_out(unsigned int addr, unsigned int val) "addr %#x value %u" +cpu_in(unsigned int addr, unsigned int val) "addr %#x value %u" +cpu_out(unsigned int addr, unsigned int val) "addr %#x
[Qemu-devel] [PATCH v5 02/10] trace: avoid conditional code compilation during option parsing
Instead of conditionally compiling option support, perform checks and issue the
corresponding error messages.
Signed-off-by: Lluís Vilanova
---
configure |4 +++-
qemu-config.c |4
qemu-options.hx |6 --
vl.c| 17 +
4 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/configure b/configure
index 88159ac..e41dcca 100755
--- a/configure
+++ b/configure
@@ -2942,7 +2942,9 @@ bsd)
esac
echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
-if test "$trace_backend" = "simple"; then
+if test "$trace_backend" = "nop"; then
+ echo "CONFIG_TRACE_NOP=y" >> $config_host_mak
+elif test "$trace_backend" = "simple"; then
echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
fi
# Set the appropriate trace file.
diff --git a/qemu-config.c b/qemu-config.c
index c63741c..d187dc5 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -296,7 +296,6 @@ static QemuOptsList qemu_mon_opts = {
},
};
-#ifdef CONFIG_SIMPLE_TRACE
static QemuOptsList qemu_trace_opts = {
.name = "trace",
.implied_opt_name = "trace",
@@ -309,7 +308,6 @@ static QemuOptsList qemu_trace_opts = {
{ /* end of list */ }
},
};
-#endif
static QemuOptsList qemu_cpudef_opts = {
.name = "cpudef",
@@ -479,9 +477,7 @@ static QemuOptsList *vm_config_groups[32] = {
&qemu_global_opts,
&qemu_mon_opts,
&qemu_cpudef_opts,
-#ifdef CONFIG_SIMPLE_TRACE
&qemu_trace_opts,
-#endif
&qemu_option_rom_opts,
&qemu_machine_opts,
NULL,
diff --git a/qemu-options.hx b/qemu-options.hx
index 37e54ee..b691e13 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2393,17 +2393,19 @@ Normally QEMU loads a configuration file from
@var{sysconfdir}/qemu.conf and
@var{sysconfdir}/target-@var{ARCH}.conf on startup. The @code{-nodefconfig}
option will prevent QEMU from loading these configuration files at startup.
ETEXI
-#ifdef CONFIG_SIMPLE_TRACE
DEF("trace", HAS_ARG, QEMU_OPTION_trace,
"-trace\n"
"Specify a trace file to log traces to\n",
QEMU_ARCH_ALL)
STEXI
+HXCOMM This line is not accurate, as the option is backend-specific but HX does
+HXCOMM not support conditional compilation of text.
@item -trace
@findex -trace
Specify a trace file to log output traces to.
+
+This option is available only when using the @var{simple} tracing backend.
ETEXI
-#endif
HXCOMM This is the last statement. Insert new options before this line!
STEXI
diff --git a/vl.c b/vl.c
index b2f41fd..b766dc7 100644
--- a/vl.c
+++ b/vl.c
@@ -2861,14 +2861,23 @@ int main(int argc, char **argv, char **envp)
}
xen_mode = XEN_ATTACH;
break;
-#ifdef CONFIG_SIMPLE_TRACE
case QEMU_OPTION_trace:
opts = qemu_opts_parse(qemu_find_opts("trace"), optarg, 0);
-if (opts) {
-trace_file = qemu_opt_get(opts, "file");
+if (!opts) {
+exit(1);
}
-break;
+#if defined(CONFIG_TRACE_NOP)
+fprintf(stderr, "qemu: option \"-%s\" is not supported by this
tracing backend\n", popt->name);
+exit(1);
#endif
+trace_file = qemu_opt_get(opts, "file");
+#if !defined(CONFIG_SIMPLE_TRACE)
+if (trace_file) {
+fprintf(stderr, "qemu: suboption \"-%s file\" is not
supported by this tracing backend\n", popt->name);
+exit(1);
+}
+#endif
+break;
case QEMU_OPTION_readconfig:
{
int ret = qemu_read_config_file(optarg);
[Qemu-devel] [PATCH v5 03/10] trace: generalize the "property" concept in the trace-events file
This adds/modifies the following functions:
* get_name: Get _only_ the event name
* has_property: Return whether an event has a property (keyword before the event
name)
Signed-off-by: Lluís Vilanova
---
docs/tracing.txt |4 +--
scripts/tracetool | 73 -
2 files changed, 35 insertions(+), 42 deletions(-)
diff --git a/docs/tracing.txt b/docs/tracing.txt
index c99a0f2..1ad106a 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -38,7 +38,7 @@ generate code for the trace events. Trace events are invoked
directly from
source code like this:
#include "trace.h" /* needed for trace event prototype */
-
+
void *qemu_malloc(size_t size)
{
void *ptr;
@@ -103,7 +103,7 @@ portability macros, ensure they are preceded and followed
by double quotes:
4. Name trace events after their function. If there are multiple trace events
in one function, append a unique distinguisher at the end of the name.
-5. Declare trace events with the "disable" keyword. Some trace events can
+5. Declare trace events with the "disable" property. Some trace events can
produce a lot of output and users are typically only interested in a subset
of trace events. Marking trace events disabled by default saves the user
from having to manually disable noisy trace events.
diff --git a/scripts/tracetool b/scripts/tracetool
index 9ed4fae..e649a5b 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -43,7 +43,26 @@ EOF
# Get the name of a trace event
get_name()
{
-echo ${1%%\(*}
+local name
+name=${1%%\(*}
+echo "${name##* }"
+}
+
+# Get the given property of a trace event
+# 1: trace-events line
+# 2: property name
+# -> return 0 if property is present, or 1 otherwise
+has_property()
+{
+local props prop
+props=${1%%\(*}
+props=${props% *}
+for prop in $props; do
+if [ "$prop" = "$2" ]; then
+return 0
+fi
+done
+return 1
}
# Get the argument list of a trace event, including types and names
@@ -101,20 +120,6 @@ get_fmt()
echo "$fmt"
}
-# Get the state of a trace event
-get_state()
-{
-local str disable state
-str=$(get_name "$1")
-disable=${str##disable }
-if [ "$disable" = "$str" ] ; then
-state=1
-else
-state=0
-fi
-echo "$state"
-}
-
linetoh_begin_nop()
{
return
@@ -174,14 +179,10 @@ cast_args_to_uint64_t()
linetoh_simple()
{
-local name args argc trace_args state
+local name args argc trace_args
name=$(get_name "$1")
args=$(get_args "$1")
argc=$(get_argc "$1")
-state=$(get_state "$1")
-if [ "$state" = "0" ]; then
-name=${name##disable }
-fi
trace_args="$simple_event_num"
if [ "$argc" -gt 0 ]
@@ -222,9 +223,10 @@ linetoc_simple()
{
local name state
name=$(get_name "$1")
-state=$(get_state "$1")
-if [ "$state" = "0" ] ; then
-name=${name##disable }
+if has_property "$1" "disable"; then
+state="0"
+else
+state="1"
fi
cat <
[Qemu-devel] [PATCH v5 00/10] trace-state: make the behaviour of "disable" consistent across all backends
This patch defines the "disable" trace event state to always use the "nop" backend. As a side-effect, all events are now enabled (without "disable") by default, as all backends (except "stderr") have programmatic support for dynamically (de)activating each trace event. In order to make this true, the "simple" backend now has a "-trace events=" argument to let the user select which events must be enabled from the very beginning. NOTES: * Parsing of -trace arguments is not done in the OS-specific frontends. Signed-off-by: Lluís Vilanova --- Changes in v5: * Fix a variable name typo in configure. * Rebase on qemu.git/master (c24a9c6ef946ec1b5b280061d4f7b579aaac6707). Changes in v4: * Fix a couple of minor errors. Changes in v3: * Rebase on qemu.git/master (642cfd4d31241c0fc65c520cb1e703659af66236). * Remove already-merged patches. * Remove code styling patches. * Generalize programmatic interface for trace event state control. Changes in v2: * Documentation fixes. * Seggregate whitespace/indentation changes. * Minor code beautifications. * Make all -trace suboptions explicit. * Fix minor comments from Stefan. * Minor trace-events format fixes. * Integrate changes from Fabien. * Rebase on qemu.git/master (c8f930c0eeb696d638f4d4bf654e955fa44ff40f). Lluís Vilanova (10): trace: move backend-specific code into the trace/ directory trace: avoid conditional code compilation during option parsing trace: generalize the "property" concept in the trace-events file trace-state: separate trace event control and query routines from the simple backend trace-state: always compile support for controlling and querying trace event states trace-state: add "-trace events" argument to control initial state trace-state: always use the "nop" backend on events with the "disable" keyword trace-state: [simple] disable all trace points by default trace-state: [stderr] add support for dynamically enabling/disabling events trace: enable all events .gitignore|2 Makefile |1 Makefile.objs |5 configure |7 + docs/tracing.txt | 64 +++--- hmp-commands.hx |9 + monitor.c | 19 +- qemu-config.c |7 - qemu-options.hx | 26 ++ scripts/tracetool | 116 +-- simpletrace.c | 355 - simpletrace.h | 48 trace-events | 569 ++--- trace/control.c | 66 ++ trace/control.h | 16 + trace/simple.c| 332 +++ trace/simple.h| 46 trace/stderr.h| 11 + vl.c | 21 +- 19 files changed, 914 insertions(+), 806 deletions(-) delete mode 100644 simpletrace.c delete mode 100644 simpletrace.h create mode 100644 trace/control.c create mode 100644 trace/control.h create mode 100644 trace/simple.c create mode 100644 trace/simple.h create mode 100644 trace/stderr.h
[Qemu-devel] [PATCH v5 09/10] trace-state: [stderr] add support for dynamically enabling/disabling events
Uses the generic interface provided in "trace/control.h" in order to provide
a programmatic interface as well as command line and monitor controls.
Signed-off-by: Fabien Chouteau
Signed-off-by: Lluís Vilanova
---
configure |3 +++
docs/tracing.txt |5 -
scripts/tracetool | 33 -
trace/control.c |4 ++--
trace/stderr.h| 11 +++
5 files changed, 44 insertions(+), 12 deletions(-)
create mode 100644 trace/stderr.h
diff --git a/configure b/configure
index e41dcca..d3ab039 100755
--- a/configure
+++ b/configure
@@ -2947,6 +2947,9 @@ if test "$trace_backend" = "nop"; then
elif test "$trace_backend" = "simple"; then
echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
fi
+if test "$trace_backend" = "stderr"; then
+ echo "CONFIG_TRACE_STDERR=y" >> $config_host_mak
+fi
# Set the appropriate trace file.
if test "$trace_backend" = "simple"; then
trace_file="\"$trace_file-\" FMT_pid"
diff --git a/docs/tracing.txt b/docs/tracing.txt
index c443171..9c17497 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -163,11 +163,6 @@ effectively turns trace events into debug printfs.
This is the simplest backend and can be used together with existing code that
uses DPRINTF().
-Note that with this backend trace events cannot be programmatically
-enabled/disabled. Thus, in order to trim down the amount of output and the
-performance impact of tracing, you might want to add the "disable" property in
-the "trace-events" file for those events you are not interested in.
-
=== Simpletrace ===
The "simple" backend supports common use cases and comes as part of the QEMU
diff --git a/scripts/tracetool b/scripts/tracetool
index c740080..743d246 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -241,7 +241,12 @@ linetoh_begin_stderr()
{
cat <
+#include "trace/stderr.h"
+
+extern TraceEvent trace_list[];
EOF
+
+stderr_event_num=0
}
linetoh_stderr()
@@ -260,29 +265,47 @@ linetoh_stderr()
cat <
[Qemu-devel] [PATCH v5 01/10] trace: move backend-specific code into the trace/ directory
Signed-off-by: Lluís Vilanova
---
.gitignore|2
Makefile |1
Makefile.objs |4 -
scripts/tracetool |2
simpletrace.c | 355 -
simpletrace.h | 48 ---
trace/simple.c| 355 +
trace/simple.h| 48 +++
vl.c |2
9 files changed, 410 insertions(+), 407 deletions(-)
delete mode 100644 simpletrace.c
delete mode 100644 simpletrace.h
create mode 100644 trace/simple.c
create mode 100644 trace/simple.h
diff --git a/.gitignore b/.gitignore
index 08013fc..b1db525 100644
--- a/.gitignore
+++ b/.gitignore
@@ -2,6 +2,8 @@ config-devices.*
config-all-devices.*
config-host.*
config-target.*
+trace/*.d
+trace/*.o
trace.h
trace.c
trace-dtrace.h
diff --git a/Makefile b/Makefile
index b3ffbe2..fe01145 100644
--- a/Makefile
+++ b/Makefile
@@ -170,6 +170,7 @@ clean:
rm -Rf .libs
rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d
net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d
rm -f qemu-img-cmds.h
+ rm -f trace/*.o trace/*.d
rm -f trace.c trace.h trace.c-timestamp trace.h-timestamp
rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
rm -f trace-dtrace.h trace-dtrace.h-timestamp
diff --git a/Makefile.objs b/Makefile.objs
index cea15e4..9a67374 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -355,14 +355,14 @@ trace-dtrace.lo: trace-dtrace.dtrace
$(call quiet-command,libtool --mode=compile --tag=CC dtrace -o $@ -G -s
$<, " lt GEN trace-dtrace.o")
endif
-simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
ifeq ($(TRACE_BACKEND),dtrace)
trace-obj-y = trace-dtrace.o
else
trace-obj-y = trace.o
ifeq ($(TRACE_BACKEND),simple)
-trace-obj-y += simpletrace.o
+trace-obj-y += trace/simple.o
user-obj-y += qemu-timer-common.o
endif
endif
diff --git a/scripts/tracetool b/scripts/tracetool
index 2155a57..9ed4fae 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -158,7 +158,7 @@ linetoc_end_nop()
linetoh_begin_simple()
{
cat <
-#include
-#include
-#include
-#include
-#include
-#include "qemu-timer.h"
-#include "trace.h"
-
-/** Trace file header event ID */
-#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with
TraceEventIDs */
-
-/** Trace file magic number */
-#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
-
-/** Trace file version number, bump if format changes */
-#define HEADER_VERSION 0
-
-/** Records were dropped event ID */
-#define DROPPED_EVENT_ID (~(uint64_t)0 - 1)
-
-/** Trace record is valid */
-#define TRACE_RECORD_VALID ((uint64_t)1 << 63)
-
-/** Trace buffer entry */
-typedef struct {
-uint64_t event;
-uint64_t timestamp_ns;
-uint64_t x1;
-uint64_t x2;
-uint64_t x3;
-uint64_t x4;
-uint64_t x5;
-uint64_t x6;
-} TraceRecord;
-
-enum {
-TRACE_BUF_LEN = 4096,
-TRACE_BUF_FLUSH_THRESHOLD = TRACE_BUF_LEN / 4,
-};
-
-/*
- * Trace records are written out by a dedicated thread. The thread waits for
- * records to become available, writes them out, and then waits again.
- */
-static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER;
-static bool trace_available;
-static bool trace_writeout_enabled;
-
-static TraceRecord trace_buf[TRACE_BUF_LEN];
-static unsigned int trace_idx;
-static FILE *trace_fp;
-static char *trace_file_name = NULL;
-
-/**
- * Read a trace record from the trace buffer
- *
- * @idx Trace buffer index
- * @record Trace record to fill
- *
- * Returns false if the record is not valid.
- */
-static bool get_trace_record(unsigned int idx, TraceRecord *record)
-{
-if (!(trace_buf[idx].event & TRACE_RECORD_VALID)) {
-return false;
-}
-
-__sync_synchronize(); /* read memory barrier before accessing record */
-
-*record = trace_buf[idx];
-record->event &= ~TRACE_RECORD_VALID;
-return true;
-}
-
-/**
- * Kick writeout thread
- *
- * @waitWhether to wait for writeout thread to complete
- */
-static void flush_trace_file(bool wait)
-{
-pthread_mutex_lock(&trace_lock);
-trace_available = true;
-pthread_cond_signal(&trace_available_cond);
-
-if (wait) {
-pthread_cond_wait(&trace_empty_cond, &trace_lock);
-}
-
-pthread_mutex_unlock(&trace_lock);
-}
-
-static void wait_for_trace_records_available(void)
-{
-pthread_mutex_lock(&trace_lock);
-while (!(trace_available && trace_writeout_enabled)) {
-pthread_cond_signal(&trace_empty_cond);
-pthread_cond_wait(&trace_available_cond, &trace_lock);
-}
-trace_available = false;
-pthread_mutex_unlock(&trace_lock);
-}
-
-static void *writeout_thread(void *opaque)
-{
-TraceRecord record;
-unsigned int writeout_idx =
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On Tue, Jun 28, 2011 at 03:09:38PM +0300, Avi Kivity wrote: > On 06/28/2011 03:07 PM, Jan Kiszka wrote: > > > > > > The point is that different buses have different widths. > > > target_phys_addr_t matches just one bus in the system. It needs to be > > > the maximum size of all buses present to be useful. > > > > Then we need a type for that. Or we need to demand that > > target_phys_addr_t is defined large enough to support all buses that the > > particular arch wants to address. Hardcoding 64 bit or anything is not > > appropriate for a generic subsystem. > > Okay, let's make t_p_a_t max(bus size in system). Do we have 32-bit > targets that don't support pci (I guess, pc-isa with cpu < ppro?). Do > we want to support a 32-bit variant of pci? It certainly existed at > some point. PCI always had a mechanism for 64-bits addresses even on 32-bits wide bus, called Dual Address Cycle. I'm not sure which was rarer: devices which could handle it, or north bridges which could use it. Probably a tie. But in theory, it was there. OG.
Re: [Qemu-devel] [PATCH] Add e500 instructions dcblc, dcbtls and dcbtstl as no-op
On Tue, 28 Jun 2011 10:17:39 +0200
Fabien Chouteau wrote:
> On 27/06/2011 18:28, Scott Wood wrote:
> > On Mon, 27 Jun 2011 15:15:55 +0200
> > Fabien Chouteau wrote:
> >
> >> +/* dcbtls */
> >> +static void gen_dcbtls(DisasContext *ctx)
> >> +{
> >> +/* interpreted as no-op */
> >> +}
> >> +
> >> +/* dcbtstls */
> >> +static void gen_dcbtstls(DisasContext *ctx)
> >> +{
> >> +/* interpreted as no-op */
> >> +}
> >
> > Set L1CSR0[CUL] (unable to lock)?
>
> Why do you want to set this bit? Can't we consider that the instruction is
> always effective?
But it's not. Why claim it is, in the absence of some specific workload
that needs to be fooled (which could take many different forms, not all of
which are appropriate defaults)?
-Scott
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Enforce pselect6 sigset size restrictions
Acked-by: Mike Frysinger -mike signature.asc Description: This is a digitally signed message part.
[Qemu-devel] [PATCH] Add compat eventfd header
Support build on RHEL 5.X where we have syscall for eventfd but not
userspace wrapper.
(cherry-picked from commit 9e3269181e9bc56feb43bcd4e8ce0b82cd543e65
in qemu-kvm.git).
Signed-off-by: Michael S. Tsirkin
---
compat/sys/eventfd.h | 13 +
configure|4 +++
2 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 compat/sys/eventfd.h
diff --git a/compat/sys/eventfd.h b/compat/sys/eventfd.h
new file mode 100644
index 000..f55d96a
--- /dev/null
+++ b/compat/sys/eventfd.h
@@ -0,0 +1,13 @@
+#ifndef _COMPAT_SYS_EVENTFD
+#define _COMPAT_SYS_EVENTFD
+
+#include
+#include
+
+
+static inline int eventfd (int count, int flags)
+{
+return syscall(SYS_eventfd, count, flags);
+}
+
+#endif
diff --git a/configure b/configure
index 856b41e..d757a6a 100755
--- a/configure
+++ b/configure
@@ -891,6 +891,9 @@ sparc64-bsd-user \
"
fi
+#compat headers
+QEMU_CFLAGS="$QEMU_CFLAGS -idirafter $source_path/compat"
+
if test x"$show_help" = x"yes" ; then
cat << EOF
--
1.7.5.53.gc233e
Re: [Qemu-devel] [PATCH v2] xen_console: support the new extended xenstore protocol
On 28 June 2011 15:55, wrote:
> + xs = xs_daemon_open();
> + if (xs == NULL) {
> + fprintf(stderr, "Could not contact XenStore\n");
> + goto out;
> + }
> +out:
> + free(path);
> + xs_daemon_close(xs);
Google suggests xs_daemon_close(NULL) will crash...
-- PMM
Re: [Qemu-devel] [Xen-devel] Re: [PATCH v2] xen_console: support the new extended xenstore protocol
On Tue, 28 Jun 2011, Ian Campbell wrote:
> On Tue, 2011-06-28 at 16:02 +0100, Peter Maydell wrote:
> > On 28 June 2011 15:55, wrote:
> > > +xs = xs_daemon_open();
> > > +if (xs == NULL) {
> > > +fprintf(stderr, "Could not contact XenStore\n");
> > > +goto out;
> > > +}
> >
> > > +out:
> > > +free(path);
> > > +xs_daemon_close(xs);
> >
> > Google suggests xs_daemon_close(NULL) will crash...
>
> Also the preferred interface these days is just xs_open/close. The other
> variants are deprecated.
And xs_close doesn't crash if the parameter is NULL, so it will kill two
birds with one stone.
Re: [Qemu-devel] [PATCH] arm-semi: Provide access to CLI arguments passed through the "-append" option
Ping. On 06/16/2011, Cedric VINCENT wrote: > This patch basically adapts the new semi-hosting command-line support > -- introduced by Wolfgang Schildbach in the commit 2e8785ac -- for use > in system-mode.
Re: [Qemu-devel] [Xen-devel] Re: [PATCH v2] xen_console: support the new extended xenstore protocol
On Tue, 2011-06-28 at 16:02 +0100, Peter Maydell wrote:
> On 28 June 2011 15:55, wrote:
> > +xs = xs_daemon_open();
> > +if (xs == NULL) {
> > +fprintf(stderr, "Could not contact XenStore\n");
> > +goto out;
> > +}
>
> > +out:
> > +free(path);
> > +xs_daemon_close(xs);
>
> Google suggests xs_daemon_close(NULL) will crash...
Also the preferred interface these days is just xs_open/close. The other
variants are deprecated.
Ian.
[Qemu-devel] [PATCH v2] xen_console: support the new extended xenstore protocol
From: Stefano Stabellini
Since CS 21994 on xen-unstable.hg and CS
466608f3a32e1f9808acdf832a5843af37e5fcec on qemu-xen-unstable.git, few
changes have been introduced to the PV console xenstore protocol, as
described by the document docs/misc/console.txt under xen-unstable.hg.
>From the Qemu point of view, very few modifications are needed to
correctly support the protocol: read from xenstore the "output" node
that tell us what the output of the PV console is going to be.
In case the output is a tty, write to xenstore the device name.
Changes in v2:
- fix error paths: free malloc'ed strings and close the xenstore
connection before returning;
- remove useless snprintf in xenstore_store_pv_console_info if i == 0.
Signed-off-by: Stefano Stabellini
---
hw/xen.h |1 +
hw/xen_console.c | 16 +-
xen-all.c| 60 ++
3 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/hw/xen.h b/hw/xen.h
index d435ca0..dad0ca0 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -50,6 +50,7 @@ qemu_irq *xen_interrupt_controller_init(void);
int xen_init(void);
int xen_hvm_init(void);
void xen_vcpu_init(void);
+void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
#if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size);
diff --git a/hw/xen_console.c b/hw/xen_console.c
index 519d5f5..e81afcd 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -179,8 +179,9 @@ static void xencons_send(struct XenConsole *con)
static int con_init(struct XenDevice *xendev)
{
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
-char *type, *dom;
+char *type, *dom, label[32];
int ret = 0;
+const char *output;
/* setup */
dom = xs_get_domain_path(xenstore, con->xendev.dom);
@@ -194,11 +195,14 @@ static int con_init(struct XenDevice *xendev)
goto out;
}
-if (!serial_hds[con->xendev.dev])
- xen_be_printf(xendev, 1, "WARNING: serial line %d not configured\n",
- con->xendev.dev);
-else
-con->chr = serial_hds[con->xendev.dev];
+output = xenstore_read_str(con->console, "output");
+/* output is a pty by default */
+if (output == NULL) {
+output = "pty";
+}
+snprintf(label, sizeof(label), "xencons%d", con->xendev.dev);
+con->chr = qemu_chr_open(label, output, NULL);
+xenstore_store_pv_console_info(con->xendev.dev, con->chr);
out:
qemu_free(type);
diff --git a/xen-all.c b/xen-all.c
index 6099bff..3f8aadb 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -737,6 +737,66 @@ static void cpu_handle_ioreq(void *opaque)
}
}
+static int store_dev_info(int domid, CharDriverState *cs, const char *string)
+{
+struct xs_handle *xs = NULL;
+char *path = NULL;
+char *newpath = NULL;
+char *pts = NULL;
+int ret = -1;
+
+/* Only continue if we're talking to a pty. */
+if (strncmp(cs->filename, "pty:", 4)) {
+return 0;
+}
+pts = cs->filename + 4;
+
+/* We now have everything we need to set the xenstore entry. */
+xs = xs_daemon_open();
+if (xs == NULL) {
+fprintf(stderr, "Could not contact XenStore\n");
+goto out;
+}
+
+path = xs_get_domain_path(xs, domid);
+if (path == NULL) {
+fprintf(stderr, "xs_get_domain_path() error\n");
+goto out;
+}
+newpath = realloc(path, (strlen(path) + strlen(string) +
+strlen("/tty") + 1));
+if (newpath == NULL) {
+fprintf(stderr, "realloc error\n");
+goto out;
+}
+path = newpath;
+
+strcat(path, string);
+strcat(path, "/tty");
+if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
+fprintf(stderr, "xs_write for '%s' fail", string);
+goto out;
+}
+ret = 0;
+
+out:
+free(path);
+xs_daemon_close(xs);
+
+return ret;
+}
+
+void xenstore_store_pv_console_info(int i, CharDriverState *chr)
+{
+if (i == 0) {
+store_dev_info(xen_domid, chr, "/console");
+} else {
+char buf[32];
+snprintf(buf, sizeof(buf), "/device/console/%d", i);
+store_dev_info(xen_domid, chr, buf);
+}
+}
+
static void xenstore_record_dm_state(XenIOState *s, const char *state)
{
char path[50];
--
1.7.2.3
Re: [Qemu-devel] KVM call agenda for June 28
On 06/28/2011 04:43 PM, Anthony Liguori wrote: FYI, I'm in an all-day meeting so I can't attend. Did you do something really bad? -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] KVM call agenda for June 28
On 06/27/2011 09:32 AM, Juan Quintela wrote: Hi Please send in any agenda items you are interested in covering. FYI, I'm in an all-day meeting so I can't attend. Regards, Anthony Liguori Later, Juan.
Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol
On Tue, 28 Jun 2011, Peter Maydell wrote:
> On 28 June 2011 12:34, wrote:
> > + path = xs_get_domain_path(xs, domid);
> > + if (path == NULL) {
> > + fprintf(stderr, "xs_get_domain_path() error\n");
> > + return -1;
> > + }
>
> Don't we need to call xs_daemon_close() on these error-exit paths?
>
> > + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
> > + fprintf(stderr, "xs_write for '%s' fail", string);
> > + return -1;
> > + }
>
> ...and this one's leaking the path string too.
>
> > +void xenstore_store_pv_console_info(int i, CharDriverState *chr)
> > +{
> > + char buf[32];
> > +
> > + if (i == 0) {
> > + snprintf(buf, sizeof(buf), "/console");
> > + store_dev_info(xen_domid, chr, buf);
>
> Am I missing something, or could you just pass "/console" straight
> to store_dev_info() without bothering to copy it into buf[] here?
All very good points as usual.
I'll send another patch with the error paths fixed and the useless
snprintf removed.
Re: [Qemu-devel] KVM call agenda for June 28
On Mon, Jun 27, 2011 at 3:32 PM, Juan Quintela wrote: > Please send in any agenda items you are interested in covering. Live block copy and image streaming: * The differences between Marcelo and Kevin's approaches * Which approach to choose and who can help implement it
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 06/28/2011 04:25 PM, Peter Maydell wrote: On 28 June 2011 13:09, Avi Kivity wrote: > Okay, let's make t_p_a_t max(bus size in system). If you want a type for that, can't you give it a sensible (ie different) name? target_phys_addr_t is pretty clearly "the type of a physical address for this target" and having it actually be something else is just going to be confusing. "a physical address" is ambiguous. There are many physical addresses flowing around. Certainly it's most natural to think about the processor's physical address bus, but that's not always useful. Since all *devices* use target_phys_addr_t, I think we should just adopt that to avoid major and pointless churn. > Do we have 32-bit targets > that don't support pci (I guess, pc-isa with cpu< ppro?). Do we want to > support a 32-bit variant of pci? It certainly existed at some point. As a thought experiment, you could take an existing 32 bit target and define a new board model that happens to have eg a new pci controller on it. It doesn't seem right that that should cause the system's idea of this type width to change, it's just a new device model and board. So if you have this type I think it ought to be max(bus size of widest bus qemu supports). That indicates typedef uint64_t target_phys_addr_t; -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 28 June 2011 13:09, Avi Kivity wrote: > Okay, let's make t_p_a_t max(bus size in system). If you want a type for that, can't you give it a sensible (ie different) name? target_phys_addr_t is pretty clearly "the type of a physical address for this target" and having it actually be something else is just going to be confusing. > Do we have 32-bit targets > that don't support pci (I guess, pc-isa with cpu < ppro?). Do we want to > support a 32-bit variant of pci? It certainly existed at some point. As a thought experiment, you could take an existing 32 bit target and define a new board model that happens to have eg a new pci controller on it. It doesn't seem right that that should cause the system's idea of this type width to change, it's just a new device model and board. So if you have this type I think it ought to be max(bus size of widest bus qemu supports). -- PMM
Re: [Qemu-devel] [PATCH] [PowerPC][RFC] booke timers
Subject: Re: [Qemu-devel] [PATCH] [PowerPC][RFC] booke timers
To:Scott Wood
Cc:qemu-devel@nongnu.org
Bcc:
-=-=-=-=-=-=-=-=-=# Don't remove this line #=-=-=-=-=-=-=-=-=-w
On 27/06/2011 22:28, Scott Wood wrote:
> On Mon, 27 Jun 2011 18:14:06 +0200
> Fabien Chouteau wrote:
>
>> While working on the emulation of the freescale p2010 (e500v2) I realized
>> that
>> there's no implementation of booke's timers features. Currently mpc8544 uses
>> ppc_emb (ppc_emb_timers_init) which is close but not exactly like booke (for
>> example booke uses different SPR).
>
> ppc_emb_timers_init should be renamed something less generic, then.
Agreed, can you help me to find a better name?
>
>> +/* Timer Control Register */
>> +
>> +#define TCR_WP_MASK 0x3 /* Watchdog Timer Period */
>> +#define TCR_WP_SHIFT 30
>> +#define TCR_WRC_MASK 0x3 /* Watchdog Timer Reset Control */
>> +#define TCR_WRC_SHIFT 28
>
> Usually such MASK defines are before shifting right:
>
> #define TCR_WP_MASK 0xc000
> #define TCR_WP_SHIFT 30
> #define TCR_WRC_MASK 0x3000
> #define TCR_WRC_SHIFT 28
>
> That way you can do things like
>
> if (tcr & TCR_WRC_MASK) {
> ...
> }
OK, I will use this kind of declaration:
#define TCR_WP_SHIFT 30/* Watchdog Timer Period */
#define TCR_WP_MASK (0x3 << TCR_WP_SHIFT)
>
>> +/* Return the time base value at which the FIT will raise an interrupt */
>> +static uint64_t booke_get_fit_target(CPUState *env)
>> +{
>> +uint32_t fp = (env->spr[SPR_BOOKE_TCR] >> TCR_FP_SHIFT) & TCR_FP_MASK;
>> +
>> +/* Only for e500 */
>> +if (env->insns_flags2 & PPC2_BOOKE206) {
>> +uint32_t fpext = (env->spr[SPR_BOOKE_TCR] >> TCR_E500_FPEXT_SHIFT)
>> +& TCR_E500_FPEXT_MASK;
>> +fp |= fpext << 2;
>> +}
>
> BOOKE206 does not mean e500. FPEXT does not exist in Power ISA V2.06 Book
> III-E.
I will try to fix this for v2.
>
>> +
>> +return 1 << fp;
>> +}
>
> The particular bits selected by the possible values of FP are
> implementation-dependent. e500 uses fpext to make all values possible, but
> on 440 the four values of fp select from 2^13, 2^17, 2^21, and 2^25.
>
Maybe I can do something like:
static uint64_t booke_get_fit_target(CPUState *env)
{
uint32_t fp = (env->spr[SPR_BOOKE_TCR] & TCR_FP_MASK) >> TCR_FP_SHIFT;
/* Only for e500 */
if (/* CPU is e500 */) {
uint32_t fpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_FPEXT_MASK)
>> TCR_E500_FPEXT_SHIFT;
fp |= fpext << 2;
} else {
fp = env->fit_period[fp];
}
return 1 << fp;
}
>> +/* Return the time base value at which the WDT will raise an interrupt */
>> +static uint64_t booke_get_wdt_target(CPUState *env)
>> +{
>> +uint32_t fp = (env->spr[SPR_BOOKE_TCR] >> TCR_WP_SHIFT) & TCR_WP_MASK;
>> +
>> +/* Only for e500 */
>> +if (env->insns_flags2 & PPC2_BOOKE206) {
>> +uint32_t fpext = (env->spr[SPR_BOOKE_TCR] >> TCR_E500_WPEXT_SHIFT)
>> +& TCR_E500_WPEXT_MASK;
>> +fp |= fpext << 2;
>> +}
>> +
>> +return 1 << fp;
>> +}
>
> s/fp/wp/
>
> Avoiding the confusion is especially important on 440, since a different
> interval is selected by a given value in FP versus WP.
Fixed.
>
>> +static void booke_update_fixed_timer(CPUState *env,
>> + uint64_t tb_target,
>> + uint64_t *next,
>> + struct QEMUTimer *timer)
>> +{
>> +ppc_tb_t *tb_env = env->tb_env;
>> +uint64_t lapse;
>> +uint64_t tb;
>> +uint64_t now;
>> +
>> +now = qemu_get_clock_ns(vm_clock);
>> +tb = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
>> +
>> +if (tb_target < tb) {
>> +qemu_del_timer(timer);
>
> You're treating the target as the timebase value that has only the selected
> bit and nothing else -- you want to expire the next time that bit
> transitions from zero to one, regardless of the other bits.
>
> The timer should never be outright disabled.
That's right, I will fix this for v2.
>
>> +static void booke_decr_cb (void *opaque)
>> +{
>> +CPUState *env;
>> +ppc_tb_t *tb_env;
>> +booke_timer_t *booke_timer;
>> +
>> +env = opaque;
>> +tb_env = env->tb_env;
>> +booke_timer = tb_env->opaque;
>> +env->spr[SPR_BOOKE_TSR] |= TSR_DIS;
>> +if (env->spr[SPR_BOOKE_TCR] & TCR_DIE) {
>> +ppc_set_irq(env, booke_timer->decr_excp, 1);
>> +}
>> +
>> +if (env->spr[SPR_BOOKE_TCR] & TCR_ARE) {
>> +/* Auto Reload */
>> +cpu_ppc_store_decr(env, env->spr[SPR_BOOKE_DECAR]);
>> +}
>> +}
>
> I think some changes in the decrementer code are needed to provide booke
> semantics -- no raising the interrupt based on the high bit of decr, and
> stop counting when reach zero.
Can you please explain, I don't see where I'm not compliant with booke's
decrementer.
>
>> +static void booke_wdt_cb (void *opaque)
>>
Re: [Qemu-devel] [PATCH] Make SLIRP Ethernet packets size to 64 bytes minimuma
On Tue, Jun 28, 2011 at 10:08 AM, Fabien Chouteau wrote: > On 28/06/2011 10:34, Stefan Hajnoczi wrote: >> This patch doesn't hurt but we'd be just as well off without it. >> >> Did you do this to fix a bug? If so, then something else in QEMU >> needs to be fixed, not slirp. > > When Qemu generates bad Ethernet frames, I think it's a bug. > > And again, this is the extension of a previous patch. If this patch is not > valid then we should revert the first, it's also a question of consistency. IMO the previous patch is not necessary either. Since there is no net subsystem maintainer who can help decide which way to go, I'm going to back off from this issue. Please go ahead. Stefan
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 06/28/2011 03:46 PM, Jan Kiszka wrote: > Do we want to support a 32-bit variant of pci? It certainly existed at > some point. As long as making everything 64 bit in the implementation of the device models is not guest visible, I don't think that should be a problem. How would it become guest visible? AFAICT the only implication is a very minor slowdown in the cases where it is not actually required. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 2011-06-28 14:09, Avi Kivity wrote: > On 06/28/2011 03:07 PM, Jan Kiszka wrote: >>> >>> The point is that different buses have different widths. >>> target_phys_addr_t matches just one bus in the system. It needs to be >>> the maximum size of all buses present to be useful. >> >> Then we need a type for that. Or we need to demand that >> target_phys_addr_t is defined large enough to support all buses that the >> particular arch wants to address. Hardcoding 64 bit or anything is not >> appropriate for a generic subsystem. > > Okay, let's make t_p_a_t max(bus size in system). Do we have 32-bit > targets that don't support pci (I guess, pc-isa with cpu < ppro?). At least lm32 and microblaze appear to fall into that category. > Do we want to support a 32-bit variant of pci? It certainly existed at > some point. As long as making everything 64 bit in the implementation of the device models is not guest visible, I don't think that should be a problem. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH 0/4] A few cleanups of qdev users
On (Tue) 28 Jun 2011 [14:24:32], Markus Armbruster wrote: > Amit Shah writes: > > > On (Mon) 27 Jun 2011 [14:36:11], Markus Armbruster wrote: > >> Amit Shah writes: > >> > >> > On (Fri) 24 Jun 2011 [13:57:28], Markus Armbruster wrote: > >> >> Ping? > >> > > >> > There were a couple of things: > >> > > >> >> port 0, guest on, host off, throttle off > >> > > >> > guest on/off, host on/off doesn't convey much -- what's on/off? > >> > > >> > Also, 'throttle' could be 'thottled'? > >> > >> Discussion petered out with my message[*]: > >> > >> I chose on/off to stay consistent with how qdev shows bool > >> properties (print_bit() in qdev-properties.c). May be misguided. > >> Like you, I'm having difficulties coming up with a better version > >> that is still consise. > >> > >> But: should "info qtree" show such device state? It's about > >> configuration of the device tree, isn't it? Connection status is > >> useful to know, but it's not device configuration. Other > >> print_dev() methods may cross that line, too. For instance, > >> usb_bus_dev_print() prints attached, which looks suspicious (commit > >> 66a6593a). > >> > >> Should info qtree continue to show this information? If yes, care to > >> suggest a better format? > > > > Don't know. I'm fine with anything the qdev guys decide. I agree > > this isn't device state. > > Unfortunately, there's no qdev maintainer making descisions. I think Gerd and you can make those decisions? :-) > What shall we do now? > > 1. Commit as is. Need an ACK then. > > 2. Respin with virtser_bus_dev_print() printing the same stuff prettier. > Need ideas on a prettier format. > > 3. Respin with virtser_bus_dev_print() printing less stuff, but > prettier. Need ideas on what exactly to print, and how. Frankly, I've no clue. I can only suggest some better names, but since these values are debug values, and there's not much churn happening in the code, I could go with anything you people suggest. I'm open to slightly renamed strings going in, open to reworking things, etc.. How about: port 0, guest_con on, host_con off, throttled off ? Amit
Re: [Qemu-devel] [PATCH] Clean up virtio-9p error handling code
Hi JV,
Any progress regarding merging this patch (and the fsync patch I submitted)?
Is there anything I can do to assist/speed the process?
Thanks
Sassan
On 8 June 2011 17:21, Sassan Panahinejad wrote:
> In a lot of cases, the handling of errors was quite ugly.
> This patch moves reading of errno to immediately after the system calls and
> passes it up through the system more cleanly.
> Also, in the case of the xattr functions (and possibly others), completely
> the wrong error was being returned.
>
>
> This patch is created against your 9p-coroutine-bh branch, as requested.
> Sorry for the delay, I was unexpectedly required to work abroad for 2 weeks.
>
> Signed-off-by: Sassan Panahinejad
>
> ---
> fsdev/file-op-9p.h|4 +-
> hw/9pfs/codir.c | 14 +
> hw/9pfs/virtio-9p-local.c | 123
> +
> hw/9pfs/virtio-9p-xattr.c | 21 +++-
> 4 files changed, 90 insertions(+), 72 deletions(-)
>
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index af9daf7..3d9575b 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -73,12 +73,12 @@ typedef struct FileOperations
> int (*setuid)(FsContext *, uid_t);
> int (*close)(FsContext *, int);
> int (*closedir)(FsContext *, DIR *);
> -DIR *(*opendir)(FsContext *, const char *);
> +int (*opendir)(FsContext *, const char *, DIR **);
> int (*open)(FsContext *, const char *, int);
> int (*open2)(FsContext *, const char *, int, FsCred *);
> void (*rewinddir)(FsContext *, DIR *);
> off_t (*telldir)(FsContext *, DIR *);
> -struct dirent *(*readdir)(FsContext *, DIR *);
> +int (*readdir)(FsContext *, DIR *, struct dirent **);
> void (*seekdir)(FsContext *, DIR *, off_t);
> ssize_t (*preadv)(FsContext *, int, const struct iovec *, int, off_t);
> ssize_t (*pwritev)(FsContext *, int, const struct iovec *, int, off_t);
> diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
> index 110289f..acbbb39 100644
> --- a/hw/9pfs/codir.c
> +++ b/hw/9pfs/codir.c
> @@ -25,12 +25,7 @@ int v9fs_co_readdir(V9fsState *s, V9fsFidState *fidp,
> struct dirent **dent)
> {
> errno = 0;
> /*FIXME!! need to switch to readdir_r */
> -*dent = s->ops->readdir(&s->ctx, fidp->fs.dir);
> -if (!*dent && errno) {
> -err = -errno;
> -} else {
> -err = 0;
> -}
> +err = s->ops->readdir(&s->ctx, fidp->fs.dir, dent);
> });
> return err;
> }
> @@ -93,12 +88,7 @@ int v9fs_co_opendir(V9fsState *s, V9fsFidState *fidp)
>
> v9fs_co_run_in_worker(
> {
> -dir = s->ops->opendir(&s->ctx, fidp->path.data);
> -if (!dir) {
> -err = -errno;
> -} else {
> -err = 0;
> -}
> +err = s->ops->opendir(&s->ctx, fidp->path.data, &dir);
> });
> fidp->fs.dir = dir;
> return err;
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 77904c3..65f35eb 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -28,7 +28,7 @@ static int local_lstat(FsContext *fs_ctx, const char
> *path, struct stat *stbuf)
> char buffer[PATH_MAX];
> err = lstat(rpath(fs_ctx, path, buffer), stbuf);
> if (err) {
> -return err;
> +return -errno;
> }
> if (fs_ctx->fs_sm == SM_MAPPED) {
> /* Actual credentials are part of extended attrs */
> @@ -53,7 +53,7 @@ static int local_lstat(FsContext *fs_ctx, const char
> *path, struct stat *stbuf)
> stbuf->st_rdev = tmp_dev;
> }
> }
> -return err;
> +return 0;
> }
>
> static int local_set_xattr(const char *path, FsCred *credp)
> @@ -63,28 +63,28 @@ static int local_set_xattr(const char *path, FsCred
> *credp)
> err = setxattr(path, "user.virtfs.uid", &credp->fc_uid,
> sizeof(uid_t),
> 0);
> if (err) {
> -return err;
> +return -errno;
> }
> }
> if (credp->fc_gid != -1) {
> err = setxattr(path, "user.virtfs.gid", &credp->fc_gid,
> sizeof(gid_t),
> 0);
> if (err) {
> -return err;
> +return -errno;
> }
> }
> if (credp->fc_mode != -1) {
> err = setxattr(path, "user.virtfs.mode", &credp->fc_mode,
> sizeof(mode_t), 0);
> if (err) {
> -return err;
> +return -errno;
> }
> }
> if (credp->fc_rdev != -1) {
> err = setxattr(path, "user.virtfs.rdev", &credp->fc_rdev,
> sizeof(dev_t), 0);
> if (err) {
> -return err;
> +return -errno;
> }
> }
> return 0;
> @@ -95,7 +95,7 @@ static int local_post_create_passthrough(FsContext
> *fs_ctx, const char *path,
> {
> char buffer[PATH_MAX];
> if (chmod(rpath(fs_ctx, path, buffer), credp->
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 06/28/2011 03:07 PM, Jan Kiszka wrote: > > The point is that different buses have different widths. > target_phys_addr_t matches just one bus in the system. It needs to be > the maximum size of all buses present to be useful. Then we need a type for that. Or we need to demand that target_phys_addr_t is defined large enough to support all buses that the particular arch wants to address. Hardcoding 64 bit or anything is not appropriate for a generic subsystem. Okay, let's make t_p_a_t max(bus size in system). Do we have 32-bit targets that don't support pci (I guess, pc-isa with cpu < ppro?). Do we want to support a 32-bit variant of pci? It certainly existed at some point. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 0/4] A few cleanups of qdev users
Amit Shah writes: > On (Mon) 27 Jun 2011 [14:36:11], Markus Armbruster wrote: >> Amit Shah writes: >> >> > On (Fri) 24 Jun 2011 [13:57:28], Markus Armbruster wrote: >> >> Ping? >> > >> > There were a couple of things: >> > >> >> port 0, guest on, host off, throttle off >> > >> > guest on/off, host on/off doesn't convey much -- what's on/off? >> > >> > Also, 'throttle' could be 'thottled'? >> >> Discussion petered out with my message[*]: >> >> I chose on/off to stay consistent with how qdev shows bool >> properties (print_bit() in qdev-properties.c). May be misguided. >> Like you, I'm having difficulties coming up with a better version >> that is still consise. >> >> But: should "info qtree" show such device state? It's about >> configuration of the device tree, isn't it? Connection status is >> useful to know, but it's not device configuration. Other >> print_dev() methods may cross that line, too. For instance, >> usb_bus_dev_print() prints attached, which looks suspicious (commit >> 66a6593a). >> >> Should info qtree continue to show this information? If yes, care to >> suggest a better format? > > Don't know. I'm fine with anything the qdev guys decide. I agree > this isn't device state. Unfortunately, there's no qdev maintainer making descisions. What shall we do now? 1. Commit as is. Need an ACK then. 2. Respin with virtser_bus_dev_print() printing the same stuff prettier. Need ideas on a prettier format. 3. Respin with virtser_bus_dev_print() printing less stuff, but prettier. Need ideas on what exactly to print, and how.
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 2011-06-28 13:53, Avi Kivity wrote:
> On 06/28/2011 01:28 PM, Jan Kiszka wrote:
>> On 2011-06-28 12:03, Michael S. Tsirkin wrote:
+struct MemoryRegion {
+/* All fields are private - violators will be prosecuted */
+const MemoryRegionOps *ops;
+MemoryRegion *parent;
+uint64_t size;
+target_phys_addr_t addr;
+target_phys_addr_t offset;
+ram_addr_t ram_addr;
+bool has_ram_addr;
+MemoryRegion *alias;
+target_phys_addr_t alias_offset;
+unsigned priority;
+bool may_overlap;
+QTAILQ_HEAD(subregions, MemoryRegion) subregions;
+QTAILQ_ENTRY(MemoryRegion) subregions_link;
+QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
+const char *name;
>>>
>>> I'm never completely sure whether these should be target addresses
>>> or bus addresses or just uint64_t.
>>> With pci on a 32 bit system you can stick a 64 bit address
>>> in a BAR and the result will be that it is never accessed
>>> from the CPU.
>>>
>>
>> Memory regions are not bound to any current or future PCI
>> specifications. Any fixed bit width would be wrong here, ie. size should
>> rather be target_phys_addr_t.
>
> The point is that different buses have different widths.
> target_phys_addr_t matches just one bus in the system. It needs to be
> the maximum size of all buses present to be useful.
Then we need a type for that. Or we need to demand that
target_phys_addr_t is defined large enough to support all buses that the
particular arch wants to address. Hardcoding 64 bit or anything is not
appropriate for a generic subsystem.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol
On 28 June 2011 12:34, wrote:
> + path = xs_get_domain_path(xs, domid);
> + if (path == NULL) {
> + fprintf(stderr, "xs_get_domain_path() error\n");
> + return -1;
> + }
Don't we need to call xs_daemon_close() on these error-exit paths?
> + if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
> + fprintf(stderr, "xs_write for '%s' fail", string);
> + return -1;
> + }
...and this one's leaking the path string too.
> +void xenstore_store_pv_console_info(int i, CharDriverState *chr)
> +{
> + char buf[32];
> +
> + if (i == 0) {
> + snprintf(buf, sizeof(buf), "/console");
> + store_dev_info(xen_domid, chr, buf);
Am I missing something, or could you just pass "/console" straight
to store_dev_info() without bothering to copy it into buf[] here?
-- PMM
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 06/28/2011 01:28 PM, Jan Kiszka wrote:
On 2011-06-28 12:03, Michael S. Tsirkin wrote:
>> +struct MemoryRegion {
>> +/* All fields are private - violators will be prosecuted */
>> +const MemoryRegionOps *ops;
>> +MemoryRegion *parent;
>> +uint64_t size;
>> +target_phys_addr_t addr;
>> +target_phys_addr_t offset;
>> +ram_addr_t ram_addr;
>> +bool has_ram_addr;
>> +MemoryRegion *alias;
>> +target_phys_addr_t alias_offset;
>> +unsigned priority;
>> +bool may_overlap;
>> +QTAILQ_HEAD(subregions, MemoryRegion) subregions;
>> +QTAILQ_ENTRY(MemoryRegion) subregions_link;
>> +QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
>> +const char *name;
>
> I'm never completely sure whether these should be target addresses
> or bus addresses or just uint64_t.
> With pci on a 32 bit system you can stick a 64 bit address
> in a BAR and the result will be that it is never accessed
> from the CPU.
>
Memory regions are not bound to any current or future PCI
specifications. Any fixed bit width would be wrong here, ie. size should
rather be target_phys_addr_t.
The point is that different buses have different widths.
target_phys_addr_t matches just one bus in the system. It needs to be
the maximum size of all buses present to be useful.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Enforce pselect6 sigset size restrictions
On 28 June 2011 12:51, Riku Voipio wrote: > On Tue, Jun 28, 2011 at 12:21:57PM +0100, Peter Maydell wrote: >> Enforce the same restriction on the size of the sigset passed to >> pselect6 as the Linux kernel does. This is both correct and silences >> a gcc 4.6 warning about a write-only variable. > > Odd but true, after all the trouble of passing the size as packed variable, > even the kernel bothers nothing but check that it matches with > sizeof(sigset_t)... I assume they're leaving the door open for implementing that properly at some later date. Incidentally, if the qemu target's sigset_t and the host's sigset_t are different sizes then not just this syscall but I suspect all the others that use sigset_t will have trouble. Luckily only one flavour of MIPS has a non-standard sigset_t size :-) -- PMM
Re: [Qemu-devel] [PATCH] linux-user/syscall.c: Enforce pselect6 sigset size restrictions
On Tue, Jun 28, 2011 at 12:21:57PM +0100, Peter Maydell wrote:
> Enforce the same restriction on the size of the sigset passed to
> pselect6 as the Linux kernel does. This is both correct and silences
> a gcc 4.6 warning about a write-only variable.
Odd but true, after all the trouble of passing the size as packed variable,
even the kernel bothers nothing but check that it matches with
sizeof(sigset_t)...
I'll include this and your other two patches for the next round.
Riku
> Signed-off-by: Peter Maydell
> ---
> This really is the last gcc 4.6 warning fix!
>
> linux-user/syscall.c |5 +
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index fed7a8f..feb2501 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5684,6 +5684,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
>
> if (arg_sigset) {
> sig.set = &set;
> +if (arg_sigsize != sizeof(*target_sigset)) {
> +/* Like the kernel, we enforce correct size sigsets
> */
> +ret = -TARGET_EINVAL;
> +goto fail;
> +}
> target_sigset = lock_user(VERIFY_READ, arg_sigset,
>sizeof(*target_sigset), 1);
> if (!target_sigset) {
> --
> 1.7.5.3
Re: [Qemu-devel] [PATCH 0/2] netdev fixes
Ping?
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 06/28/2011 01:03 PM, Michael S. Tsirkin wrote:
On Mon, Jun 27, 2011 at 04:21:48PM +0300, Avi Kivity wrote:
...
> +static bool memory_region_access_valid(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + unsigned size)
> +{
> +if (!mr->ops->valid.unaligned&& (addr& (size - 1))) {
> +return false;
> +}
> +
> +/* Treat zero as compatibility all valid */
> +if (!mr->ops->valid.max_access_size) {
> +return true;
> +}
> +
> +if (size> mr->ops->valid.max_access_size
> +|| size< mr->ops->valid.min_access_size) {
> +return false;
> +}
> +return true;
> +}
> +
> +static uint32_t memory_region_read_thunk_n(void *_mr,
> + target_phys_addr_t addr,
> + unsigned size)
> +{
> +MemoryRegion *mr = _mr;
> +unsigned access_size, access_size_min, access_size_max;
> +uint64_t access_mask;
> +uint32_t data = 0, tmp;
> +unsigned i;
> +
> +if (!memory_region_access_valid(mr, addr, size)) {
> +return -1U; /* FIXME: better signalling */
> +}
> +
> +/* FIXME: support unaligned access */
> +
> +access_size_min = mr->ops->impl.max_access_size;
min = max: Intentional? Cut&paste error?
Bug; thanks.
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 000..a67ff94
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,201 @@
> +#ifndef MEMORY_H
> +#define MEMORY_H
> +
> +#ifndef CONFIG_USER_ONLY
What's the story with this ifdef?
There are no stubs provided ...
No callers either - I build with a full configuration. I prefer the
#ifdef here rather than all call sites.
> +
> +struct MemoryRegion {
> +/* All fields are private - violators will be prosecuted */
> +const MemoryRegionOps *ops;
> +MemoryRegion *parent;
> +uint64_t size;
> +target_phys_addr_t addr;
> +target_phys_addr_t offset;
> +ram_addr_t ram_addr;
> +bool has_ram_addr;
> +MemoryRegion *alias;
> +target_phys_addr_t alias_offset;
> +unsigned priority;
> +bool may_overlap;
> +QTAILQ_HEAD(subregions, MemoryRegion) subregions;
> +QTAILQ_ENTRY(MemoryRegion) subregions_link;
> +QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
> +const char *name;
I'm never completely sure whether these should be target addresses
or bus addresses or just uint64_t.
With pci on a 32 bit system you can stick a 64 bit address
in a BAR and the result will be that it is never accessed
from the CPU.
I agree. Anyone objects to making the memory API 64-bit?
It will reduce performance slightly for 32-on-32, but these
configurations are getting rarer, and the performance loss is quite small.
Maybe we should make t_p_a_t 64-bit unconditionally. Note that sizes
have to be 64-bit in any case, otherwise you can't express a 4G range
without tricks.
--
error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] virtio-blk: Turn drive serial into a qdev property
Markus Armbruster writes: > 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. Ping?
[Qemu-devel] [PATCH] xen_console: support the new extended xenstore protocol
From: Stefano Stabellini
Since CS 21994 on xen-unstable.hg and CS
466608f3a32e1f9808acdf832a5843af37e5fcec on qemu-xen-unstable.git, few
changes have been introduced to the PV console xenstore protocol, as
described by the document docs/misc/console.txt under xen-unstable.hg.
>From the Qemu point of view, very few modifications are needed to
correctly support the protocol: read from xenstore the "output" node
that tell us what the output of the PV console is going to be.
In case the output is a tty, write to xenstore the device name.
Signed-off-by: Stefano Stabellini
---
hw/xen.h |1 +
hw/xen_console.c | 16 -
xen-all.c| 62 ++
3 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/hw/xen.h b/hw/xen.h
index d435ca0..dad0ca0 100644
--- a/hw/xen.h
+++ b/hw/xen.h
@@ -50,6 +50,7 @@ qemu_irq *xen_interrupt_controller_init(void);
int xen_init(void);
int xen_hvm_init(void);
void xen_vcpu_init(void);
+void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
#if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size);
diff --git a/hw/xen_console.c b/hw/xen_console.c
index 519d5f5..e81afcd 100644
--- a/hw/xen_console.c
+++ b/hw/xen_console.c
@@ -179,8 +179,9 @@ static void xencons_send(struct XenConsole *con)
static int con_init(struct XenDevice *xendev)
{
struct XenConsole *con = container_of(xendev, struct XenConsole, xendev);
-char *type, *dom;
+char *type, *dom, label[32];
int ret = 0;
+const char *output;
/* setup */
dom = xs_get_domain_path(xenstore, con->xendev.dom);
@@ -194,11 +195,14 @@ static int con_init(struct XenDevice *xendev)
goto out;
}
-if (!serial_hds[con->xendev.dev])
- xen_be_printf(xendev, 1, "WARNING: serial line %d not configured\n",
- con->xendev.dev);
-else
-con->chr = serial_hds[con->xendev.dev];
+output = xenstore_read_str(con->console, "output");
+/* output is a pty by default */
+if (output == NULL) {
+output = "pty";
+}
+snprintf(label, sizeof(label), "xencons%d", con->xendev.dev);
+con->chr = qemu_chr_open(label, output, NULL);
+xenstore_store_pv_console_info(con->xendev.dev, con->chr);
out:
qemu_free(type);
diff --git a/xen-all.c b/xen-all.c
index 6099bff..ee51324 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -737,6 +737,68 @@ static void cpu_handle_ioreq(void *opaque)
}
}
+static int store_dev_info(int domid, CharDriverState *cs, const char *string)
+{
+struct xs_handle *xs;
+char *path;
+char *newpath;
+char *pts;
+
+/*
+ * Only continue if we're talking to a pty
+ */
+if (strncmp(cs->filename, "pty:", 4)) {
+return 0;
+}
+pts = cs->filename + 4;
+
+/* We now have everything we need to set the xenstore entry. */
+xs = xs_daemon_open();
+if (xs == NULL) {
+fprintf(stderr, "Could not contact XenStore\n");
+return -1;
+}
+
+path = xs_get_domain_path(xs, domid);
+if (path == NULL) {
+fprintf(stderr, "xs_get_domain_path() error\n");
+return -1;
+}
+newpath = realloc(path, (strlen(path) + strlen(string) +
+strlen("/tty") + 1));
+if (newpath == NULL) {
+free(path); /* realloc errors leave old block */
+fprintf(stderr, "realloc error\n");
+return -1;
+}
+path = newpath;
+
+strcat(path, string);
+strcat(path, "/tty");
+if (!xs_write(xs, XBT_NULL, path, pts, strlen(pts))) {
+fprintf(stderr, "xs_write for '%s' fail", string);
+return -1;
+}
+
+free(path);
+xs_daemon_close(xs);
+
+return 0;
+}
+
+void xenstore_store_pv_console_info(int i, CharDriverState *chr)
+{
+char buf[32];
+
+if (i == 0) {
+snprintf(buf, sizeof(buf), "/console");
+store_dev_info(xen_domid, chr, buf);
+} else {
+snprintf(buf, sizeof(buf), "/device/console/%d", i);
+store_dev_info(xen_domid, chr, buf);
+}
+}
+
static void xenstore_record_dm_state(XenIOState *s, const char *state)
{
char path[50];
--
1.7.2.3
[Qemu-devel] [PATCH] Documentation: Remove outdated host_device note
People shouldn't explicitly specify host_device any more. raw is doing the
Right Thing.
Signed-off-by: Kevin Wolf
---
qemu-img.texi |6 --
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/qemu-img.texi b/qemu-img.texi
index ced64a4..526474c 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -173,12 +173,6 @@ Linux or NTFS on Windows), then only the written sectors
will reserve
space. Use @code{qemu-img info} to know the real size used by the
image or @code{ls -ls} on Unix/Linux.
-@item host_device
-
-Host device format. This format should be used instead of raw when
-converting to block devices or other devices where "holes" are not
-supported.
-
@item qcow2
QEMU image format, the most versatile format. Use it to have smaller
images (useful if your filesystem does not supports holes, for example
--
1.7.5.4
Re: [Qemu-devel] [PATCH 3/3] xen: implement unplug protocol in xen_platform
On Mon, 27 Jun 2011, Kevin Wolf wrote:
> Am 27.06.2011 17:34, schrieb Stefano Stabellini:
> > On Mon, 27 Jun 2011, Kevin Wolf wrote:
> >> hw/ide/pci.h is just as internal as internal.h is. And even if you
> >> managed to access the same things without any IDE header file, I still
> >> think it's not the right level of abstraction because it relies on the
> >> implementation details of IDE.
> >>
> >> Just this line: pci_ide->bus[di->bus].ifs[di->unit].bs = NULL; Does this
> >> really look right to you to do anywhere outside IDE?
> >>
> >> I'm basically looking for the same as Michael who wanted to have network
> >> unplug handled through qdev, just that the IDE code doesn't support
> >> unplug yet.
> >
> > I understand.
> >
> > I created pci_piix3_xen_ide_init and moved the unplug code to
> > hw/ide/piix.c so that we don't have any internal knowledge of IDE code
> > in xen_platform.c any more, see below.
> >
> > One remaining problem is that the generic pci unplug function is not
> > what I need in this case so I have to setup my own; but I hope that in
> > general the changes are going in the right direction.
>
> Looks much better to me now. :-)
great :)
> Just a few comments inline:
>
> > diff --git a/hw/ide.h b/hw/ide.h
> > index 34d9394..a490cbb 100644
> > --- a/hw/ide.h
> > +++ b/hw/ide.h
> > @@ -13,6 +13,7 @@ ISADevice *isa_ide_init(int iobase, int iobase2, int
> > isairq,
> > /* ide-pci.c */
> > void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
> > int secondary_ide_enabled);
> > +PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
> > devfn);
> > PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int
> > devfn);
> > PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int
> > devfn);
> > void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> > diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> > index c349644..8ae9ff0 100644
> > --- a/hw/ide/piix.c
> > +++ b/hw/ide/piix.c
> > @@ -167,6 +167,41 @@ static int pci_piix4_ide_initfn(PCIDevice *dev)
> > return pci_piix_ide_initfn(d);
> > }
> >
> > +static int pci_piix3_xen_ide_unplug(DeviceState *dev)
> > +{
> > +PCIDevice *pci_dev;
> > +PCIIDEState *pci_ide;
> > +DriveInfo *di;
> > +int i = 0;
> > +
> > +pci_dev = DO_UPCAST(PCIDevice, qdev, dev);
> > +pci_ide = DO_UPCAST(PCIIDEState, dev, pci_dev);
> > +
> > +for (; i < 3; i++) {
> > +di = drive_get_by_index(IF_IDE, i);
> > +if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
> > +DeviceState *ds = bdrv_get_attached(di->bdrv);
> > +if (ds)
> > +bdrv_detach(di->bdrv, ds);
>
> Missing braces
This time I run checkpatch.pl on it.
>
> > +bdrv_close(di->bdrv);
> > +pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
> > +drive_put_ref(di);
> > +}
> > +}
> > +qdev_reset_all(&(pci_ide->dev.qdev));
> > +return 0;
> > +}
> > +
> > +PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
> > devfn)
> > +{
> > +PCIDevice *dev;
> > +
> > +dev = pci_create_simple(bus, devfn, "piix3-ide");
> > +dev->qdev.info->unplug = pci_piix3_xen_ide_unplug;
>
> Doesn't this modify the piix3-ide definition? Shouldn't matter in
> practice because you can't have additional IDE controllers anyway, but
> maybe worth a comment stating this.
>
> The other, probably cleaner way of doing it would be adding another
> PCIDeviceInfo for xen-ide.
I have added piix3-ide-xen, that doesn't set no_hotplug = 1, so now the
code should be more coherent.
I'll send the new version of the patch separately.
Re: [Qemu-devel] [PATCH 0/4] A few cleanups of qdev users
On (Mon) 27 Jun 2011 [14:36:11], Markus Armbruster wrote: > Amit Shah writes: > > > On (Fri) 24 Jun 2011 [13:57:28], Markus Armbruster wrote: > >> Ping? > > > > There were a couple of things: > > > >> port 0, guest on, host off, throttle off > > > > guest on/off, host on/off doesn't convey much -- what's on/off? > > > > Also, 'throttle' could be 'thottled'? > > Discussion petered out with my message[*]: > > I chose on/off to stay consistent with how qdev shows bool > properties (print_bit() in qdev-properties.c). May be misguided. > Like you, I'm having difficulties coming up with a better version > that is still consise. > > But: should "info qtree" show such device state? It's about > configuration of the device tree, isn't it? Connection status is > useful to know, but it's not device configuration. Other > print_dev() methods may cross that line, too. For instance, > usb_bus_dev_print() prints attached, which looks suspicious (commit > 66a6593a). > > Should info qtree continue to show this information? If yes, care to > suggest a better format? Don't know. I'm fine with anything the qdev guys decide. I agree this isn't device state. Amit
[Qemu-devel] [PATCH v3] xen: implement unplug protocol in xen_platform
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.
Changes in v2:
- use PCI_CLASS constants;
- replace pci_unplug_device with qdev_unplug;
- do not import hw/ide/internal.h in xen_platform.c;
Changes in v3:
- introduce piix3-ide-xen, that support hot-unplug;
- move the unplug code to hw/ide/piix.c;
- just call qdev_unplug from xen_platform.c to unplug the IDE disks;
Signed-off-by: Stefano Stabellini
---
hw/ide.h |1 +
hw/ide/piix.c | 41 +
hw/pc_piix.c |6 +-
hw/xen_platform.c | 43 ++-
4 files changed, 89 insertions(+), 2 deletions(-)
diff --git a/hw/ide.h b/hw/ide.h
index 34d9394..a490cbb 100644
--- a/hw/ide.h
+++ b/hw/ide.h
@@ -13,6 +13,7 @@ ISADevice *isa_ide_init(int iobase, int iobase2, int isairq,
/* ide-pci.c */
void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
int secondary_ide_enabled);
+PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
devfn);
PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
void vt82c686b_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index c349644..50ab7c5 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -167,6 +167,42 @@ static int pci_piix4_ide_initfn(PCIDevice *dev)
return pci_piix_ide_initfn(d);
}
+static int pci_piix3_xen_ide_unplug(DeviceState *dev)
+{
+PCIDevice *pci_dev;
+PCIIDEState *pci_ide;
+DriveInfo *di;
+int i = 0;
+
+pci_dev = DO_UPCAST(PCIDevice, qdev, dev);
+pci_ide = DO_UPCAST(PCIIDEState, dev, pci_dev);
+
+for (; i < 3; i++) {
+di = drive_get_by_index(IF_IDE, i);
+if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) {
+DeviceState *ds = bdrv_get_attached(di->bdrv);
+if (ds) {
+bdrv_detach(di->bdrv, ds);
+}
+bdrv_close(di->bdrv);
+pci_ide->bus[di->bus].ifs[di->unit].bs = NULL;
+drive_put_ref(di);
+}
+}
+qdev_reset_all(&(pci_ide->dev.qdev));
+return 0;
+}
+
+PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
+{
+PCIDevice *dev;
+
+dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
+dev->qdev.info->unplug = pci_piix3_xen_ide_unplug;
+pci_ide_create_devs(dev, hd_table);
+return dev;
+}
+
/* hd_table must contain 4 block drivers */
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
@@ -197,6 +233,11 @@ static PCIDeviceInfo piix_ide_info[] = {
.no_hotplug = 1,
.init = pci_piix3_ide_initfn,
},{
+.qdev.name= "piix3-ide-xen",
+.qdev.size= sizeof(PCIIDEState),
+.qdev.no_user = 1,
+.init = pci_piix3_ide_initfn,
+},{
.qdev.name= "piix4-ide",
.qdev.size= sizeof(PCIIDEState),
.qdev.no_user = 1,
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 8dbeb0c..b59adcc 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -155,7 +155,11 @@ static void pc_init1(ram_addr_t ram_size,
ide_drive_get(hd, MAX_IDE_BUS);
if (pci_enabled) {
PCIDevice *dev;
-dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
+if (xen_enabled()) {
+dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
+} else {
+dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
+}
idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
} else {
diff --git a/hw/xen_platform.c b/hw/xen_platform.c
index b167eee..a271369 100644
--- a/hw/xen_platform.c
+++ b/hw/xen_platform.c
@@ -76,6 +76,35 @@ 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 void unplug_nic(PCIBus *b, PCIDevice *d)
+{
+if (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
+PCI_CLASS_NETWORK_ETHERNET) {
+qdev_unplug(&(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 (pci_get_word(d->config + PCI_CLASS_DEVICE) ==
+PCI_CLASS_
[Qemu-devel] [PATCH] linux-user/syscall.c: Enforce pselect6 sigset size restrictions
Enforce the same restriction on the size of the sigset passed to
pselect6 as the Linux kernel does. This is both correct and silences
a gcc 4.6 warning about a write-only variable.
Signed-off-by: Peter Maydell
---
This really is the last gcc 4.6 warning fix!
linux-user/syscall.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index fed7a8f..feb2501 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5684,6 +5684,11 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
if (arg_sigset) {
sig.set = &set;
+if (arg_sigsize != sizeof(*target_sigset)) {
+/* Like the kernel, we enforce correct size sigsets */
+ret = -TARGET_EINVAL;
+goto fail;
+}
target_sigset = lock_user(VERIFY_READ, arg_sigset,
sizeof(*target_sigset), 1);
if (!target_sigset) {
--
1.7.5.3
Re: [Qemu-devel] [PATCH v2] qemu_ram_ptr_length: take ram_addr_t as arguments
On 27 June 2011 18:26, wrote: > From: Stefano Stabellini > > qemu_ram_ptr_length should take ram_addr_t as argument rather than > target_phys_addr_t because is doing comparisons with RAMBlock addresses. > > cpu_physical_memory_map should create a ram_addr_t address to pass to > qemu_ram_ptr_length from PhysPageDesc phys_offset. > > Remove code after abort() in qemu_ram_ptr_length. Reviewed-by: Peter Maydell Tested and confirmed this fixes vexpress and looks good to me -- thanks. -- PMM
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On 2011-06-28 12:03, Michael S. Tsirkin wrote:
>> +struct MemoryRegion {
>> +/* All fields are private - violators will be prosecuted */
>> +const MemoryRegionOps *ops;
>> +MemoryRegion *parent;
>> +uint64_t size;
>> +target_phys_addr_t addr;
>> +target_phys_addr_t offset;
>> +ram_addr_t ram_addr;
>> +bool has_ram_addr;
>> +MemoryRegion *alias;
>> +target_phys_addr_t alias_offset;
>> +unsigned priority;
>> +bool may_overlap;
>> +QTAILQ_HEAD(subregions, MemoryRegion) subregions;
>> +QTAILQ_ENTRY(MemoryRegion) subregions_link;
>> +QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
>> +const char *name;
>
> I'm never completely sure whether these should be target addresses
> or bus addresses or just uint64_t.
> With pci on a 32 bit system you can stick a 64 bit address
> in a BAR and the result will be that it is never accessed
> from the CPU.
>
Memory regions are not bound to any current or future PCI
specifications. Any fixed bit width would be wrong here, ie. size should
rather be target_phys_addr_t.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC v2 01/20] Hierarchical memory region API
On Mon, Jun 27, 2011 at 04:21:48PM +0300, Avi Kivity wrote:
...
> +static bool memory_region_access_valid(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + unsigned size)
> +{
> +if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
> +return false;
> +}
> +
> +/* Treat zero as compatibility all valid */
> +if (!mr->ops->valid.max_access_size) {
> +return true;
> +}
> +
> +if (size > mr->ops->valid.max_access_size
> +|| size < mr->ops->valid.min_access_size) {
> +return false;
> +}
> +return true;
> +}
> +
> +static uint32_t memory_region_read_thunk_n(void *_mr,
> + target_phys_addr_t addr,
> + unsigned size)
> +{
> +MemoryRegion *mr = _mr;
> +unsigned access_size, access_size_min, access_size_max;
> +uint64_t access_mask;
> +uint32_t data = 0, tmp;
> +unsigned i;
> +
> +if (!memory_region_access_valid(mr, addr, size)) {
> +return -1U; /* FIXME: better signalling */
> +}
> +
> +/* FIXME: support unaligned access */
> +
> +access_size_min = mr->ops->impl.max_access_size;
min = max: Intentional? Cut&paste error?
> +if (!access_size_min) {
> +access_size_min = 1;
> +}
> +access_size_max = mr->ops->impl.max_access_size;
> +if (!access_size_max) {
> +access_size_max = 4;
> +}
...
> diff --git a/memory.h b/memory.h
> new file mode 100644
> index 000..a67ff94
> --- /dev/null
> +++ b/memory.h
> @@ -0,0 +1,201 @@
> +#ifndef MEMORY_H
> +#define MEMORY_H
> +
> +#ifndef CONFIG_USER_ONLY
What's the story with this ifdef?
There are no stubs provided ...
> +
> +#include
> +#include
> +#include "qemu-common.h"
> +#include "cpu-common.h"
> +#include "targphys.h"
> +#include "qemu-queue.h"
> +
> +typedef struct MemoryRegionOps MemoryRegionOps;
> +typedef struct MemoryRegion MemoryRegion;
> +
> +/* Must match *_DIRTY_FLAGS in cpu-all.h. To be replaced with dynamic
> + * registration.
> + */
> +#define DIRTY_MEMORY_VGA 0
> +#define DIRTY_MEMORY_CODE 1
> +#define DIRTY_MEMORY_MIGRATION 3
> +
> +/*
> + * Memory region callbacks
> + */
> +struct MemoryRegionOps {
> +/* Read from the memory region. @addr is relative to @mr; @size is
> + * in bytes. */
> +uint64_t (*read)(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + unsigned size);
> +/* Write to the memory region. @addr is relative to @mr; @size is
> + * in bytes. */
> +void (*write)(MemoryRegion *mr,
> + target_phys_addr_t addr,
> + uint64_t data,
> + unsigned size);
> +
> +enum device_endian endianness;
> +/* Guest-visible constraints: */
> +struct {
> +/* If nonzero, specify bounds on access sizes beyond which a machine
> + * check is thrown.
> + */
> +unsigned min_access_size;
> +unsigned max_access_size;
> +/* If true, unaligned accesses are supported. Otherwise unaligned
> + * accesses throw machine checks.
> + */
> + bool unaligned;
> +} valid;
> +/* Internal implementation constraints: */
> +struct {
> +/* If nonzero, specifies the minimum size implemented. Smaller sizes
> + * will be rounded upwards and a partial result will be returned.
> + */
> +unsigned min_access_size;
> +/* If nonzero, specifies the maximum size implemented. Larger sizes
> + * will be done as a series of accesses with smaller sizes.
> + */
> +unsigned max_access_size;
> +/* If true, unaligned accesses are supported. Otherwise all accesses
> + * are converted to (possibly multiple) naturally aligned accesses.
> + */
> + bool unaligned;
> +} impl;
> +};
> +
> +typedef struct CoalescedMemoryRange CoalescedMemoryRange;
> +
> +struct MemoryRegion {
> +/* All fields are private - violators will be prosecuted */
> +const MemoryRegionOps *ops;
> +MemoryRegion *parent;
> +uint64_t size;
> +target_phys_addr_t addr;
> +target_phys_addr_t offset;
> +ram_addr_t ram_addr;
> +bool has_ram_addr;
> +MemoryRegion *alias;
> +target_phys_addr_t alias_offset;
> +unsigned priority;
> +bool may_overlap;
> +QTAILQ_HEAD(subregions, MemoryRegion) subregions;
> +QTAILQ_ENTRY(MemoryRegion) subregions_link;
> +QTAILQ_HEAD(coalesced_ranges, CoalescedMemoryRange) coalesced;
> +const char *name;
I'm never completely sure whether these should be target addresses
or bus addresses or just uint64_t.
With pci on a 32 bit system you can stick a 64 bit address
in a BAR and the result will be that it is never accessed
from the CPU.
--
MST
Re: [Qemu-devel] [RFC PATCH v2] Specification for qcow2 version 3
2011/6/27 Kevin Wolf :
> This is the second draft for what I think could be added when we increase
> qcow2's
> version number to 3. This includes points that have been made by several
> people
> over the past few months. We're probably not going to implement this next
> week,
> but I think it's important to get discussions started early, so here it is.
>
> Changes implemented in this RFC:
>
> - Added compatible/incompatible/auto-clear feature bits plus an optional
> feature name table to allow useful error messages even if an older version
> doesn't know some feature at all.
>
> - Added a dirty flag which tells that the refcount may not be accurate ("QED
> mode"). This means that we can save writes to the refcount table with
> cache=writethrough, but isn't really useful otherwise since Qcow2Cache.
>
> - Configurable refcount width. If you don't want to use internal snapshots,
> make refcounts one bit and save cache space and I/O.
>
> - Added subclusters. This separate the COW size (one subcluster, I'm thinking
> of 64k default size here) from the allocation size (one cluster, 2M). Less
> fragmentation, less metadata, but still reasonable COW granularity.
>
> This also allows to preallocate clusters, but none of their subclusters. You
> can have an image that is like raw + COW metadata, and you can also
> preallocate metadata for images with backing files.
>
> - Zero cluster flags. This allows discard even with a backing file that
> doesn't
> contain zeros. It is also useful for copy-on-read/image streaming, as you'll
> want to keep sparseness without accessing the remote image for an unallocated
> cluster all the time.
>
> - Fixed internal snapshot metadata to use 64 bit VM state size. You can't save
> a snapshot of a VM with >= 4 GB RAM today.
>
> Possible future additions:
>
> - Add per-L2-table dirty flag to L1?
> - Add per-refcount-block full flag to refcount table?
Hi,
thinking about image improvement I would add
- GUID for image and backing file
- relative path for backing file
This would help finding images in a distributed environment or if file
are moved, ie: gfs/nfs/ocfs mounted in different mount points, backing
used a template in a different images directory and move this
directory somewhere else. Also with GUID a possible higher level could
manage a GUID <-> file image db.
I was also think about a "backing file length" field to support
resizing but probably can be implemented with zero cluster. Assume you
have a image of 5gb, create a new image with first image as backing
one, now resize second image from 5gb to 3gb then resize it again
(after some works) to 10gb, part from 3gb to 5gb should not be read
from backing file.
Also a bit in l2 offset to say "there is no l2 table" cause all
clusters in l2 are contiguous so we avoid entirely l2. Obviously this
require an optimization step to detect or create such condition.
For check perhaps it would be helpful to save not only a flag but also
a size where data are ok (for instance already allocated and with
refcount saved correctly).
A possible optimization for refcount would be to initialize refcount
to 1 instead of 0. When clusters are allocated at end-of-file this
would not require refcount change and would be easy to check file size
to see which clusters are marked as allocated but not present.
Fields for sectors and heads to support old CHS systems ??
This mail sound quite strange to me, I thought qed would be the future
of qcow2 but I must be really wrong.
I think a big limit for current qed and qcow2 implementation is the
serialization of metadata informations (qcow2 use synchronous
operation while qed use a queue). I used bonnie++ program to test
speed and performances allocating data is about 15-20% of allocated
one. I'm working (in the few spare time I have) improving it.
VirtualBox and ESX use large clusters (1mb) to mitigate
allocation/metadata problem. Perhaps raising default cluster size
would help changing a spread idea of bad qemu i/o performance.
Regards
Frediano Ziglio
Re: [Qemu-devel] [PATCH v2] virtio-serial: Fix segfault on guest boot
On (Wed) 22 Jun 2011 [09:53:35], Luiz Capitulino wrote:
> On Wed, 22 Jun 2011 09:49:22 +0530
> Amit Shah wrote:
> > >
> > > -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) {
> >
> > What we lose after this re-arrangement is the check that port is NULL
> > when this message is received. i.e., a guest bug where port is set to
> > a valid value when this message arrives. (I think I pointed this out
> > in a previous mail?)
>
> I'm not sure I follow you here, the current code doesn't return if
> cpkt.event == VIRTIO_CONSOLE_DEVICE_READY:
>
> port = find_port_by_id(vser, ldl_p(&gcpkt->id));
> if (!port && cpkt.event != VIRTIO_CONSOLE_DEVICE_READY)
> return;
Ah; right. Anyway it's a small thing, nothing to be worried about.
Amit
[Qemu-devel] [Qemu devel] qemu fpu state in synch with hw fpu state
Hello, We are working on a record replaying tool in qemu and kvm. We have successfully implemented record replaying individually in both the systems. So, we can record executions of VM in qemu and replay it in qemu and similarly in kvm. The next interesting stuff would be to implement a cross system where we can record execution in kvm and asynchronously replay it in qemu. There are some interesting applications of being able to do this (eg. asynchronous taint analysis). We maintain a record log where we record non deterministic information during record and while replaying, the record log is used. For eg. we store interrupt info, IO in this record log. For cross record replay to work, it is important that the entire state of the system remains same across all instructions in both qemu and kvm (HW). We have done most of this work, but it seems still much is left. We are facing issues to get the floating point state consistent across all floating point instructions. Any pointers here will be appreciated. We find that floating point status word and floating point control word are not consistent with the actual hardware state. We also tried the new patch where i386 is made compatible with softfloat, but there still seems to be issues with it. What would be the likely effort required to get qemu fpu in synch with hw fpu? Thanks, Mehul
Re: [Qemu-devel] [PATCH] Make SLIRP Ethernet packets size to 64 bytes minimuma
On 28/06/2011 10:34, Stefan Hajnoczi wrote: > On Mon, Jun 27, 2011 at 5:12 PM, Fabien Chouteau wrote: >> On 27/06/2011 17:39, Stefan Hajnoczi wrote: >>> On Mon, Jun 27, 2011 at 3:23 PM, Fabien Chouteau >>> wrote: On 27/06/2011 15:50, Stefan Hajnoczi wrote: > On Mon, Jun 27, 2011 at 1:41 PM, Fabien Chouteau > wrote: >> >> Signed-off-by: Fabien Chouteau >> --- >> slirp/slirp.c |4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) > > Any particular bug that this fixes? > > There have been 64 byte minimum padding patches to several emulated > NICs. There has also been discussion about where the best place to do > this is. Why is this patch necessary? > This patch is necessary because some NICs are configured to drop short frames, therefore the OS will not receive some of the packets generated by Qemu. There's a first patch to fix this issue: http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=dbf3c4b4baceb91eb64d09f787cbe92d65188813 My patch fixes two other sources of short frames. >>> >>> Thanks for the explanation. I stepped back from the discussion on >>> where the right place to fix this is last time around. Now I'm >>> wondering why do anything in slirp when the other sources (tap, ...) >>> aren't padding to 64 bytes? >> >> I think that packets generated by Qemu must follow RFC. For other sources, >> qemu >> should keep original size when possible and put padding otherwise. > > slirp interfaces to net.h which does not emulate Ethernet. For > example, it doesn't carry the Frame Check Sequence (FCS) as per the > Ethernet standard. NICs that want to report FCS to the guest > calculate it themselves, just like they do with 64-byte padding. So I > find it weird to say we should honor Ethernet when the transport we > are using is not Ethernet. In this case it's always Ethernet frames, look at the two patched lines, there's an Ethernet header: uint8_t arp_req[max(ETH_HLEN + sizeof(struct arphdr), 64)]; slirp_output(slirp->opaque, buf, max(ip_data_len + ETH_HLEN, 64)); > > This patch doesn't hurt but we'd be just as well off without it. > > Did you do this to fix a bug? If so, then something else in QEMU > needs to be fixed, not slirp. When Qemu generates bad Ethernet frames, I think it's a bug. And again, this is the extension of a previous patch. If this patch is not valid then we should revert the first, it's also a question of consistency. -- Fabien Chouteau
Re: [Qemu-devel] IO errors in guest caused by LTP dio test
Am 27.06.2011 18:08, schrieb Andi Kleen: > On Mon, Jun 27, 2011 at 05:59:41PM +0200, Kevin Wolf wrote: >> Am 23.06.2011 01:36, schrieb Andi Kleen: >>> >>> Running LTP testcases/kernel/io/direct_io/test_dma_thread_diotest7 >>> causes IO errors in the guest. There are no IO errors on the host. >>> >>> Kernel Linux 3.0.0-rc* >>> Using a standard emulated IDE -hda image. >> >> Couldn't reproduce this in a quick test with a random guest I had >> available. What is your complete qemu command line? Is the problem new > > -m 1500M -smp 2 \ > -hda ~/qemu/suse-11.1-64.img\ > -kernel arch/x86/boot/bzImage -append "root=/dev/sda1 debug $EXTRA" "$@" > > Could it be related to the image? > > suse11.3-64.img: Qemu Image, Format: Qcow , Version: 1 , Disk Size could be: > 83886080 * 256 bytes Oh, this is a qcow1 image? Interesting, I didn't expect that people are still using this format. :-) Definitely worth trying a raw image or at least qcow2. The old qcow1 format driver hasn't been actively maintained for quite a while now. >> with the 3.0 RCs or does it also happen with kernels that my existing >> guests are likely to have in use - say, the F15 kernel? > > It happened on older kernels too. >> >> I guess with virtio-blk you don't see the problems? > > Will try later. Ok, thanks. It looked like an IDE problem at first, but now that you mention it's a qcow1 image, I think it could just as well be an image format driver bug. Kevin
[Qemu-devel] qemu state in synch with hw state
Hello, We are working on a record replaying tool in qemu and kvm. We have successfully implemented record replaying individually in both the systems. So, we can record executions of VM in qemu and replay it in qemu and similarly in kvm. The next interesting stuff would be to implement a cross system where we can record execution in kvm and asynchronously replay it in qemu. There are some interesting applications of being able to do this (eg. asynchronous taint analysis). We maintain a record log where we record non deterministic information during record and while replaying, the record log is used. For eg. we store interrupt info, IO in this record log. For cross record replay to work, it is important that the entire state of the system remains same across all instructions in both qemu and kvm (HW). We have done most of this work, but it seems still much is left. We are facing issues to get the floating point state consistent across all floating point instructions. Any pointers here will be appreciated. We find that floating point status word and floating point control word are not consistent with the actual hardware state. We also tried the new patch where i386 is made compatible with softfloat, but there still seems to be issues with it. What would be the likely effort required to get qemu fpu in synch with hw fpu? Thanks, Mehul
Re: [Qemu-devel] [Bug 802588] [NEW] Latest GIT version fails to compile on Linux - setjmp.h problem
On Mon, Jun 27, 2011 at 4:28 PM, Nigel Horne <802...@bugs.launchpad.net> wrote: > Git version: f26e428da505709ec03b2ed2c9eb3db82b30bd7b Fixed in 2fb0c09f4ff036f68474277ed4edc036f6529de8. Stefan
Re: [Qemu-devel] [PATCH] Make SLIRP Ethernet packets size to 64 bytes minimum
On Mon, Jun 27, 2011 at 5:12 PM, Fabien Chouteau wrote: > On 27/06/2011 17:39, Stefan Hajnoczi wrote: >> On Mon, Jun 27, 2011 at 3:23 PM, Fabien Chouteau >> wrote: >>> On 27/06/2011 15:50, Stefan Hajnoczi wrote: On Mon, Jun 27, 2011 at 1:41 PM, Fabien Chouteau wrote: > > Signed-off-by: Fabien Chouteau > --- > slirp/slirp.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) Any particular bug that this fixes? There have been 64 byte minimum padding patches to several emulated NICs. There has also been discussion about where the best place to do this is. Why is this patch necessary? >>> >>> This patch is necessary because some NICs are configured to drop short >>> frames, >>> therefore the OS will not receive some of the packets generated by Qemu. >>> >>> There's a first patch to fix this issue: >>> http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=dbf3c4b4baceb91eb64d09f787cbe92d65188813 >>> >>> My patch fixes two other sources of short frames. >> >> Thanks for the explanation. I stepped back from the discussion on >> where the right place to fix this is last time around. Now I'm >> wondering why do anything in slirp when the other sources (tap, ...) >> aren't padding to 64 bytes? > > I think that packets generated by Qemu must follow RFC. For other sources, > qemu > should keep original size when possible and put padding otherwise. slirp interfaces to net.h which does not emulate Ethernet. For example, it doesn't carry the Frame Check Sequence (FCS) as per the Ethernet standard. NICs that want to report FCS to the guest calculate it themselves, just like they do with 64-byte padding. So I find it weird to say we should honor Ethernet when the transport we are using is not Ethernet. This patch doesn't hurt but we'd be just as well off without it. Did you do this to fix a bug? If so, then something else in QEMU needs to be fixed, not slirp. Stefan
Re: [Qemu-devel] [PATCH] Add e500 instructions dcblc, dcbtls and dcbtstl as no-op
On 27/06/2011 18:28, Scott Wood wrote:
> On Mon, 27 Jun 2011 15:15:55 +0200
> Fabien Chouteau wrote:
>
>> +/* dcbtls */
>> +static void gen_dcbtls(DisasContext *ctx)
>> +{
>> +/* interpreted as no-op */
>> +}
>> +
>> +/* dcbtstls */
>> +static void gen_dcbtstls(DisasContext *ctx)
>> +{
>> +/* interpreted as no-op */
>> +}
>
> Set L1CSR0[CUL] (unable to lock)?
Why do you want to set this bit? Can't we consider that the instruction is
always effective?
--
Fabien Chouteau
