[Qemu-devel] [PATCH 02/16] vnc: JPEG should be disabled if the client don't set tight quality
Disable JPEG compression by default and only enable it if the VNC client has sent the requested quality. Signed-off-by: Corentin Chary corenti...@iksaif.net --- vnc.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/vnc.c b/vnc.c index ed0e096..9cf38d1 100644 --- a/vnc.c +++ b/vnc.c @@ -1644,7 +1644,7 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings) vs-features = 0; vs-vnc_encoding = 0; vs-tight_compression = 9; -vs-tight_quality = 9; +vs-tight_quality = -1; /* Lossless by default */ vs-absolute = -1; /* -- 1.7.1
[Qemu-devel] [PATCH 03/16] vnc: add lossy option
The lossy option can be used to enable lossy compression
methods like gradient or jpeg. This patch disable them by
default.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
qemu-options.hx |7 +++
vnc-encoding-tight.c |4
vnc.c|2 ++
vnc.h|2 ++
4 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index a6928b7..cf33a54 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -828,6 +828,13 @@ empty, with a @code{deny} policy. Thus no one will be
allowed to
use the VNC server until the ACLs have been loaded. This can be
achieved using the @code{acl} monitor command.
+...@item lossy
+
+Enable lossy compression methods (gradient, JPEG, ...). If this
+option is set, VNC client may receive lossy framebuffer updates
+depending on its encoding settings. Enabling this option can save
+a lot of bandwidth at the expense of quality.
+
@end table
ETEXI
diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index 5b69ff0..c1a292b 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -228,6 +228,10 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
int compression = vs-tight_compression;
int quality = vs-tight_quality;
+if (!vs-vd-lossy) {
+return 0;
+}
+
if (ds_get_bytes_per_pixel(vs-ds) == 1 ||
vs-clientds.pf.bytes_per_pixel == 1 ||
w VNC_TIGHT_DETECT_MIN_WIDTH || h VNC_TIGHT_DETECT_MIN_HEIGHT) {
diff --git a/vnc.c b/vnc.c
index 9cf38d1..ccd7aad 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2482,6 +2482,8 @@ int vnc_display_open(DisplayState *ds, const char
*display)
#endif
} else if (strncmp(options, acl, 3) == 0) {
acl = 1;
+} else if (strncmp(options, lossy, 5) == 0) {
+vs-lossy = true;
}
}
diff --git a/vnc.h b/vnc.h
index 2a9024d..ec90cd3 100644
--- a/vnc.h
+++ b/vnc.h
@@ -33,6 +33,7 @@
#include monitor.h
#include audio/audio.h
#include zlib.h
+#include stdbool.h
#include keymaps.h
@@ -111,6 +112,7 @@ struct VncDisplay
char *display;
char *password;
int auth;
+bool lossy;
#ifdef CONFIG_VNC_TLS
int subauth; /* Used by VeNCrypt */
VncDisplayTLS tls;
--
1.7.1
[Qemu-devel] [PATCH 06/16] vnc: tight: don't forget do at the last color
While using indexed colors, the last color was never added to the palette.
Triggered with ubuntu livecd.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc-enc-tight.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 358221d..ade8e5f 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -393,11 +393,11 @@ static int tight_palette_insert(QDict *palette, uint32_t
rgb, int bpp, int max)
if (data[i] == ci) {\
continue; \
} else {\
+ci = data[i]; \
if (!tight_palette_insert(*palette, (uint32_t)ci, \
bpp, max)) { \
return 0; \
} \
-ci = data[i]; \
} \
} \
\
--
1.7.1
[Qemu-devel] [PATCH 04/16] ui: move all ui components in ui/
Move sdl, vnc, curses and cocoa UI into ui/ to cleanup the root directory. Also remove some unnecessary explicit targets from Makefile. Signed-off-by: Corentin Chary corenti...@iksaif.net --- Makefile | 38 +++- Makefile.objs | 22 ++- cocoa.m = ui/cocoa.m |0 curses.c = ui/curses.c|0 curses_keys.h = ui/curses_keys.h |0 d3des.c = ui/d3des.c |0 d3des.h = ui/d3des.h |0 keymaps.c = ui/keymaps.c |0 keymaps.h = ui/keymaps.h |0 sdl.c = ui/sdl.c |0 sdl_keysym.h = ui/sdl_keysym.h|0 sdl_zoom.c = ui/sdl_zoom.c|0 sdl_zoom.h = ui/sdl_zoom.h|0 sdl_zoom_template.h = ui/sdl_zoom_template.h |0 vnc-auth-sasl.c = ui/vnc-auth-sasl.c |0 vnc-auth-sasl.h = ui/vnc-auth-sasl.h |0 vnc-auth-vencrypt.c = ui/vnc-auth-vencrypt.c |0 vnc-auth-vencrypt.h = ui/vnc-auth-vencrypt.h |0 .../vnc-encoding-hextile.c |0 vnc-encoding-tight.c = ui/vnc-encoding-tight.c|0 vnc-encoding-tight.h = ui/vnc-encoding-tight.h|0 vnc-encoding-zlib.c = ui/vnc-encoding-zlib.c |0 vnc-tls.c = ui/vnc-tls.c |0 vnc-tls.h = ui/vnc-tls.h |0 vnc.c = ui/vnc.c |0 vnc.h = ui/vnc.h |0 vnc_keysym.h = ui/vnc_keysym.h|0 vnchextile.h = ui/vnchextile.h|0 x_keymap.c = ui/x_keymap.c|0 x_keymap.h = ui/x_keymap.h|0 30 files changed, 17 insertions(+), 43 deletions(-) rename cocoa.m = ui/cocoa.m (100%) rename curses.c = ui/curses.c (100%) rename curses_keys.h = ui/curses_keys.h (100%) rename d3des.c = ui/d3des.c (100%) rename d3des.h = ui/d3des.h (100%) rename keymaps.c = ui/keymaps.c (100%) rename keymaps.h = ui/keymaps.h (100%) rename sdl.c = ui/sdl.c (100%) rename sdl_keysym.h = ui/sdl_keysym.h (100%) rename sdl_zoom.c = ui/sdl_zoom.c (100%) rename sdl_zoom.h = ui/sdl_zoom.h (100%) rename sdl_zoom_template.h = ui/sdl_zoom_template.h (100%) rename vnc-auth-sasl.c = ui/vnc-auth-sasl.c (100%) rename vnc-auth-sasl.h = ui/vnc-auth-sasl.h (100%) rename vnc-auth-vencrypt.c = ui/vnc-auth-vencrypt.c (100%) rename vnc-auth-vencrypt.h = ui/vnc-auth-vencrypt.h (100%) rename vnc-encoding-hextile.c = ui/vnc-encoding-hextile.c (100%) rename vnc-encoding-tight.c = ui/vnc-encoding-tight.c (100%) rename vnc-encoding-tight.h = ui/vnc-encoding-tight.h (100%) rename vnc-encoding-zlib.c = ui/vnc-encoding-zlib.c (100%) rename vnc-tls.c = ui/vnc-tls.c (100%) rename vnc-tls.h = ui/vnc-tls.h (100%) rename vnc.c = ui/vnc.c (100%) rename vnc.h = ui/vnc.h (100%) rename vnc_keysym.h = ui/vnc_keysym.h (100%) rename vnchextile.h = ui/vnchextile.h (100%) rename x_keymap.c = ui/x_keymap.c (100%) rename x_keymap.h = ui/x_keymap.h (100%) diff --git a/Makefile b/Makefile index 221fbd8..f0295da 100644 --- a/Makefile +++ b/Makefile @@ -96,42 +96,14 @@ audio/audio.o audio/fmodaudio.o: QEMU_CFLAGS += $(FMOD_CFLAGS) QEMU_CFLAGS+=$(CURL_CFLAGS) -cocoa.o: cocoa.m +ui/cocoa.o: ui/cocoa.m -keymaps.o: keymaps.c keymaps.h +ui/sdl.o audio/sdlaudio.o ui/sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) -sdl_zoom.o: sdl_zoom.c sdl_zoom.h sdl_zoom_template.h - -sdl.o: sdl.c keymaps.h sdl_keysym.h sdl_zoom.h - -sdl.o audio/sdlaudio.o sdl_zoom.o baum.o: QEMU_CFLAGS += $(SDL_CFLAGS) - -acl.o: acl.h acl.c - -vnc.h: vnc-tls.h vnc-auth-vencrypt.h vnc-auth-sasl.h keymaps.h - -vnc.o: vnc.c vnc.h vnc_keysym.h vnchextile.h d3des.c d3des.h acl.h - -vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) - -vnc-tls.o: vnc-tls.c vnc.h - -vnc-auth-vencrypt.o: vnc-auth-vencrypt.c vnc.h - -vnc-auth-sasl.o: vnc-auth-sasl.c vnc.h - -vnc-encoding-zlib.o: vnc-encoding-zlib.c vnc.h - -vnc-encoding-hextile.o: vnc-encoding-hextile.c vnc.h - -vnc-encoding-tight.o: vnc-encoding-tight.c vnc.h vnc-encoding-tight.h - -curses.o: curses.c keymaps.h curses_keys.h +ui/vnc.o: QEMU_CFLAGS += $(VNC_TLS_CFLAGS) bt-host.o: QEMU_CFLAGS += $(BLUEZ_CFLAGS) -iov.o: iov.c iov.h - ## qemu-img.o: qemu-img-cmds.h @@ -159,7 +131,7 @@ clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h rm -f *.o *.d *.a $(TOOLS) TAGS cscope.* *.pod *~ */*~ - rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d net/*.o net/*.d + rm -f slirp/*.o
[Qemu-devel] [PATCH 05/16] vnc: rename vnc-encoding-* vnc-enc-*
For the same reason that we don't use vnc-authentication-sasl.c but
vnc-auth-sals.c. Because it's to long.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
Makefile.objs|4 ++--
ui/{vnchextile.h = vnc-enc-hextile-template.h} |0
ui/{vnc-encoding-hextile.c = vnc-enc-hextile.c} | 12 ++--
ui/{vnc-encoding-tight.c = vnc-enc-tight.c} |2 +-
ui/{vnc-encoding-tight.h = vnc-enc-tight.h} |0
ui/{vnc-encoding-zlib.c = vnc-enc-zlib.c} |0
6 files changed, 9 insertions(+), 9 deletions(-)
rename ui/{vnchextile.h = vnc-enc-hextile-template.h} (100%)
rename ui/{vnc-encoding-hextile.c = vnc-enc-hextile.c} (93%)
rename ui/{vnc-encoding-tight.c = vnc-enc-tight.c} (99%)
rename ui/{vnc-encoding-tight.h = vnc-enc-tight.h} (100%)
rename ui/{vnc-encoding-zlib.c = vnc-enc-zlib.c} (100%)
diff --git a/Makefile.objs b/Makefile.objs
index bbb5518..e63c5eb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -106,8 +106,8 @@ ui-obj-y += keymaps.o
ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
ui-obj-$(CONFIG_CURSES) += curses.o
ui-obj-y += vnc.o d3des.o
-ui-obj-y += vnc-encoding-zlib.o vnc-encoding-hextile.o
-ui-obj-y += vnc-encoding-tight.o
+ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o
+ui-obj-y += vnc-enc-tight.o
ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
ui-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/vnchextile.h b/ui/vnc-enc-hextile-template.h
similarity index 100%
rename from ui/vnchextile.h
rename to ui/vnc-enc-hextile-template.h
diff --git a/ui/vnc-encoding-hextile.c b/ui/vnc-enc-hextile.c
similarity index 93%
rename from ui/vnc-encoding-hextile.c
rename to ui/vnc-enc-hextile.c
index 728f25e..fa4b264 100644
--- a/ui/vnc-encoding-hextile.c
+++ b/ui/vnc-enc-hextile.c
@@ -33,32 +33,32 @@ static void hextile_enc_cord(uint8_t *ptr, int x, int y,
int w, int h)
}
#define BPP 8
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#define BPP 16
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#define BPP 32
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#define GENERIC
#define BPP 8
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#undef GENERIC
#define GENERIC
#define BPP 16
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#undef GENERIC
#define GENERIC
#define BPP 32
-#include vnchextile.h
+#include vnc-enc-hextile-template.h
#undef BPP
#undef GENERIC
diff --git a/ui/vnc-encoding-tight.c b/ui/vnc-enc-tight.c
similarity index 99%
rename from ui/vnc-encoding-tight.c
rename to ui/vnc-enc-tight.c
index c1a292b..358221d 100644
--- a/ui/vnc-encoding-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -37,7 +37,7 @@
#include qdict.h
#include qint.h
#include vnc.h
-#include vnc-encoding-tight.h
+#include vnc-enc-tight.h
/* Compression level stuff. The following array contains various
encoder parameters for each of 10 compression levels (0..9).
diff --git a/ui/vnc-encoding-tight.h b/ui/vnc-enc-tight.h
similarity index 100%
rename from ui/vnc-encoding-tight.h
rename to ui/vnc-enc-tight.h
diff --git a/ui/vnc-encoding-zlib.c b/ui/vnc-enc-zlib.c
similarity index 100%
rename from ui/vnc-encoding-zlib.c
rename to ui/vnc-enc-zlib.c
--
1.7.1
[Qemu-devel] [PATCH 07/16] vnc: tight: remove a memleak in send_jpeg_rect()
buf was never freed.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc-enc-tight.c |5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index ade8e5f..4ff88a8 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -1247,8 +1247,6 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int
w, int h, int quality)
if (ds_get_bytes_per_pixel(vs-ds) == 1)
return send_full_color_rect(vs, w, h);
-buf = qemu_malloc(w * 3);
-row[0] = buf;
buffer_reserve(vs-tight_jpeg, 2048);
cinfo.err = jpeg_std_error(jerr);
@@ -1270,10 +1268,13 @@ static int send_jpeg_rect(VncState *vs, int x, int y,
int w, int h, int quality)
jpeg_start_compress(cinfo, true);
+buf = qemu_malloc(w * 3);
+row[0] = buf;
for (dy = 0; dy h; dy++) {
jpeg_prepare_row(vs, buf, x, y + dy, w);
jpeg_write_scanlines(cinfo, row, 1);
}
+qemu_free(buf);
jpeg_finish_compress(cinfo);
jpeg_destroy_compress(cinfo);
--
1.7.1
[Qemu-devel] [PATCH 09/16] vnc: tight: specific zlib level and filters for each compression level
Disable png filters for lower compression levels. This should lower
the CPU consumption and reduce encoding time.
This isn't in tight_conf because:
* tight_conf structure must not change, because it's shared with other
tight implementations (libvncserver, etc..).
* it'd exceed the 80 col limit.
* PNG_ macros are only defined if CONFIG_VNC_PNG is defined
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc-enc-tight.c | 19 ++-
1 files changed, 18 insertions(+), 1 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 007d88f..9e1f214 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -69,6 +69,21 @@ static const struct {
};
#ifdef CONFIG_VNC_PNG
+static const struct {
+int png_zlib_level, png_filters;
+} tight_png_conf[] = {
+{ 0, PNG_NO_FILTERS },
+{ 1, PNG_NO_FILTERS },
+{ 2, PNG_NO_FILTERS },
+{ 3, PNG_NO_FILTERS },
+{ 4, PNG_NO_FILTERS },
+{ 5, PNG_ALL_FILTERS },
+{ 6, PNG_ALL_FILTERS },
+{ 7, PNG_ALL_FILTERS },
+{ 8, PNG_ALL_FILTERS },
+{ 9, PNG_ALL_FILTERS },
+};
+
static int send_png_rect(VncState *vs, int x, int y, int w, int h,
QDict *palette);
@@ -1424,7 +1439,8 @@ static int send_png_rect(VncState *vs, int x, int y, int
w, int h,
png_infop info_ptr;
png_colorp png_palette = NULL;
size_t offset;
-int level = tight_conf[vs-tight_compression].raw_zlib_level;
+int level = tight_png_conf[vs-tight_compression].png_zlib_level;
+int filters = tight_png_conf[vs-tight_compression].png_filters;
uint8_t *buf;
int dy;
@@ -1443,6 +1459,7 @@ static int send_png_rect(VncState *vs, int x, int y, int
w, int h,
png_set_write_fn(png_ptr, (void *) vs, png_write_data, png_flush_data);
png_set_compression_level(png_ptr, level);
+png_set_filter(png_ptr, PNG_FILTER_TYPE_DEFAULT, filters);
if (palette) {
color_type = PNG_COLOR_TYPE_PALETTE;
--
1.7.1
[Qemu-devel] [PATCH 01/16] vnc: tight: add JPEG and gradient subencoding with smooth image detection
Add gradient filter and JPEG compression with an heuristic to detect how
lossy the comppression will be. This code has been adapted from
libvncserver/tight.c.
JPEG support can be enabled/disabled at compile time with --enable-vnc-jpeg
and --disable-vnc-jpeg.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
Makefile.target |1 +
configure| 33 +++
vnc-encoding-tight.c | 559 +-
vnc-encoding-tight.h |5 +
vnc.h|4 +
5 files changed, 601 insertions(+), 1 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 478b89d..d9e888a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -177,6 +177,7 @@ LIBS+=-lz
QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
+QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
# xen backend driver support
obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
diff --git a/configure b/configure
index c0d8aa5..304b4b0 100755
--- a/configure
+++ b/configure
@@ -268,6 +268,7 @@ uuid=
vde=
vnc_tls=
vnc_sasl=
+vnc_jpeg=
xen=
linux_aio=
vhost_net=
@@ -574,6 +575,10 @@ for opt do
;;
--enable-vnc-sasl) vnc_sasl=yes
;;
+ --disable-vnc-jpeg) vnc_jpeg=no
+ ;;
+ --enable-vnc-jpeg) vnc_jpeg=yes
+ ;;
--disable-slirp) slirp=no
;;
--disable-uuid) uuid=no
@@ -818,6 +823,8 @@ echo --disable-vnc-tlsdisable TLS encryption for
VNC server
echo --enable-vnc-tls enable TLS encryption for VNC server
echo --disable-vnc-sasl disable SASL encryption for VNC server
echo --enable-vnc-saslenable SASL encryption for VNC server
+echo --disable-vnc-jpeg disable JPEG lossy compression for VNC server
+echo --enable-vnc-jpegenable JPEG lossy compression for VNC server
echo --disable-curses disable curses output
echo --enable-curses enable curses output
echo --disable-curl disable curl connectivity
@@ -1237,6 +1244,27 @@ EOF
fi
##
+# VNC JPEG detection
+if test $vnc_jpeg = yes ; then
+cat $TMPC EOF
+#include stdio.h
+#include jpeglib.h
+int main(void) { struct jpeg_compress_struct s; jpeg_create_compress(s);
return 0; }
+EOF
+vnc_jpeg_cflags=
+vnc_jpeg_libs=-ljpeg
+ if compile_prog $vnc_jpeg_cflags $vnc_jpeg_libs ; then
+vnc_jpeg=yes
+libs_softmmu=$vnc_jpeg_libs $libs_softmmu
+ else
+if test $vnc_jpeg = yes ; then
+ feature_not_found vnc-jpeg
+fi
+vnc_jpeg=no
+ fi
+fi
+
+##
# fnmatch() probe, used for ACL routines
fnmatch=no
cat $TMPC EOF
@@ -2064,6 +2092,7 @@ echo Block whitelist $block_drv_whitelist
echo Mixer emulation $mixemu
echo VNC TLS support $vnc_tls
echo VNC SASL support $vnc_sasl
+echo VNC JPEG support $vnc_jpeg
if test -n $sparc_cpu; then
echo Target Sparc Arch $sparc_cpu
fi
@@ -2200,6 +2229,10 @@ if test $vnc_sasl = yes ; then
echo CONFIG_VNC_SASL=y $config_host_mak
echo VNC_SASL_CFLAGS=$vnc_sasl_cflags $config_host_mak
fi
+if test $vnc_jpeg = yes ; then
+ echo CONFIG_VNC_JPEG=y $config_host_mak
+ echo VNC_JPEG_CFLAGS=$vnc_jpeg_cflags $config_host_mak
+fi
if test $fnmatch = yes ; then
echo CONFIG_FNMATCH=y $config_host_mak
fi
diff --git a/vnc-encoding-tight.c b/vnc-encoding-tight.c
index faba483..5b69ff0 100644
--- a/vnc-encoding-tight.c
+++ b/vnc-encoding-tight.c
@@ -26,6 +26,14 @@
* THE SOFTWARE.
*/
+#include qemu-common.h
+
+#ifdef CONFIG_VNC_JPEG
+#include stdio.h
+#include jpeglib.h
+#endif
+
+#include bswap.h
#include qdict.h
#include qint.h
#include vnc.h
@@ -56,6 +64,206 @@ static const struct {
};
/*
+ * Code to guess if given rectangle is suitable for smooth image
+ * compression (by applying gradient filter or JPEG coder).
+ */
+
+static uint
+tight_detect_smooth_image24(VncState *vs, int w, int h)
+{
+int off;
+int x, y, d, dx;
+uint c;
+uint stats[256];
+int pixels = 0;
+int pix, left[3];
+uint errors;
+unsigned char *buf = vs-tight.buffer;
+
+/*
+ * If client is big-endian, color samples begin from the second
+ * byte (offset 1) of a 32-bit pixel value.
+ */
+off = !!(vs-clientds.flags QEMU_BIG_ENDIAN_FLAG);
+
+memset(stats, 0, sizeof (stats));
+
+for (y = 0, x = 0; y h x w;) {
+for (d = 0; d h - y d w - x - VNC_TIGHT_DETECT_SUBROW_WIDTH;
+ d++) {
+for (c = 0; c 3; c++) {
+left[c] = buf[((y+d)*w+x+d)*4+off+c] 0xFF;
+}
+for (dx = 1; dx = VNC_TIGHT_DETECT_SUBROW_WIDTH; dx++) {
+for (c = 0; c 3; c++) {
+pix = buf[((y+d)*w+x+d+dx)*4+off+c] 0xFF;
+stats[abs(pix - left[c])]++;
+left[c] = pix;
+}
+pixels++;
+}
+}
+if (w h) {
+x += h;
+y = 0;
+} else {
+x = 0;
+
[Qemu-devel] [PATCH 00/16] VNC updates for 0.13
Tight JPEG and Move to ui = This set starts by adding JPEG and gradient to tight, then move all ui code in the ui/ subdirectory. Thanks, Since v1: * Format patch with rename detection * Add lossy parameter instead of lossless * Disable lossy encodings by default * Add a small tight fix (for indexed colors) Since v2: * Rebased on current master * Removed a leak in send_jpeg_rect() Misc === * Add the missing last color while filling palette * Rewrite the palette code without using qdict. I did some profiling using `perf` and a lot of cpu time was spent in qdict, mainly due to memory allocation, hash, and qobject conversion. The new code is faster and uses less memory. Tight PNG == This set introduce a new encoding: VNC_ENCODING_TIGHT_PNG (0x17) and a new tight filter VNC_TIGHT_PNG (0x0A). When the client tells it supports the 0x17 encoding, the server will use tight, but will always send encoding pixels using PNG instead of zlib. If the client also told it support JPEG, then the server can send JPEG, because PNG will only be used in the cases zlib was used in normal tight. This encoding was introduced to speed up HTML5 based VNC clients like noVNC [1], but can also be used on devices like iPhone where PNG can be rendered in hardware. I also made a quick patch to add support for PNG in gtk-vnc [2] and noVNC already support PNG encoding. Note: There is a bug in gtk-vnc when using pixbuf on a 16bit display, which also happens with JPEG. [1] http://github.com/kanaka/noVNC/ [2] http://xf.iksaif.net/dev/vnc/gtk-vnc/0001-add-png-support.patch Threaded Server === Since v1: * Moved locks from VncState to VncDisplay because it's only used in vnc_refresh * Use trylock in vnc_refresh. If there is an encoding task still running, reschedule the refresh. This really boost performances and make the vnc server truly asynchroneous. The only blocking lock is the output_mutex which is only held during a simple memcpy(). * Fixed issues found by Paolo, except the exit condition, mainly because we can only have one queue per VncState (due to zstreams), so this is not really an issue. * Rebased on top of jpeg and ui/ patchs Since v2: * renamed vnc-jobs.c vnc-jobs-async.c * added vnc-jobs.h, refactor functions declarations, export vnc_[un]lock_display() and vnc_[un]lock_output() and use them in vnc-jobs-async.c (reported by Avi) * rework exit condition for vnc_worker_thread_loop (Paolo) * abord - abort (Paolo) * call qemu_thread_self() (Paolo) * Coding style issues (Alexander) * Move from empty macros to empty statis inline (Alexander) Alexander also suggested me to use stw_be_p() defined in cpu-all.h, but when I tried to include cpu-all.h, it broke every thing. Anyway it can be done later since this code is already present in vnc.c. Also vnc_async_encoding_start() could be cleaner if encoding members where in a specific structure, but this is a lot of changes, and as I'm also working on encodings, I want this patch to be easy to rebase. So I'll do as soon as the VNC server is merged. Since v3: * Encoding are data is now in specific structures, that makes vnc_async_encoding_start a lot cleaner. * Added a missing vnc_output_lock(vs) Corentin Chary (16): vnc: tight: add JPEG and gradient subencoding with smooth image detection vnc: JPEG should be disabled if the client don't set tight quality vnc: add lossy option ui: move all ui components in ui/ vnc: rename vnc-encoding-* vnc-enc-* vnc: tight: don't forget do at the last color vnc: tight: remove a memleak in send_jpeg_rect() vnc: tight add PNG encoding vnc: tight: specific zlib level and filters for each compression level vnc: tight: stop using qdict for palette stuff vnc: encapsulate encoding members vnc: fix tight png memory leak qemu-thread: add qemu_mutex/cond_destroy and qemu_mutex_exit vnc: threaded VNC server vnc: add missing lock for vnc_cursor_define() vnc: tight: don't limit png rect size Makefile | 38 +- Makefile.objs | 29 +- Makefile.target|2 + configure | 83 ++ qemu-options.hx|7 + qemu-thread.c | 22 + qemu-thread.h |4 + cocoa.m = ui/cocoa.m |0 curses.c = ui/curses.c|0 curses_keys.h = ui/curses_keys.h |0 d3des.c = ui/d3des.c |0 d3des.h = ui/d3des.h |0 keymaps.c = ui/keymaps.c |0 keymaps.h = ui/keymaps.h |0 sdl.c = ui/sdl.c |0 sdl_keysym.h = ui/sdl_keysym.h|0 sdl_zoom.c = ui/sdl_zoom.c|0 sdl_zoom.h = ui/sdl_zoom.h
[Qemu-devel] [PATCH 13/16] qemu-thread: add qemu_mutex/cond_destroy and qemu_mutex_exit
Add some missing functions in qemu-thread. Currently qemu-thread
is only used for io-thread but it will used by the vnc server soon
and we need those functions instead of calling pthread directly.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
qemu-thread.c | 22 ++
qemu-thread.h |4
2 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/qemu-thread.c b/qemu-thread.c
index faf4061..fbc78fe 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -34,6 +34,15 @@ void qemu_mutex_init(QemuMutex *mutex)
error_exit(err, __func__);
}
+void qemu_mutex_destroy(QemuMutex *mutex)
+{
+int err;
+
+err = pthread_mutex_destroy(mutex-lock);
+if (err)
+error_exit(err, __func__);
+}
+
void qemu_mutex_lock(QemuMutex *mutex)
{
int err;
@@ -90,6 +99,15 @@ void qemu_cond_init(QemuCond *cond)
error_exit(err, __func__);
}
+void qemu_cond_destroy(QemuCond *cond)
+{
+int err;
+
+err = pthread_cond_destroy(cond-cond);
+if (err)
+error_exit(err, __func__);
+}
+
void qemu_cond_signal(QemuCond *cond)
{
int err;
@@ -168,3 +186,7 @@ int qemu_thread_equal(QemuThread *thread1, QemuThread
*thread2)
return pthread_equal(thread1-thread, thread2-thread);
}
+void qemu_thread_exit(void *retval)
+{
+pthread_exit(retval);
+}
diff --git a/qemu-thread.h b/qemu-thread.h
index 5ef4a3a..19bb30c 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -20,12 +20,14 @@ typedef struct QemuCond QemuCond;
typedef struct QemuThread QemuThread;
void qemu_mutex_init(QemuMutex *mutex);
+void qemu_mutex_destroy(QemuMutex *mutex);
void qemu_mutex_lock(QemuMutex *mutex);
int qemu_mutex_trylock(QemuMutex *mutex);
int qemu_mutex_timedlock(QemuMutex *mutex, uint64_t msecs);
void qemu_mutex_unlock(QemuMutex *mutex);
void qemu_cond_init(QemuCond *cond);
+void qemu_cond_destroy(QemuCond *cond);
void qemu_cond_signal(QemuCond *cond);
void qemu_cond_broadcast(QemuCond *cond);
void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
@@ -37,4 +39,6 @@ void qemu_thread_create(QemuThread *thread,
void qemu_thread_signal(QemuThread *thread, int sig);
void qemu_thread_self(QemuThread *thread);
int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+void qemu_thread_exit(void *retval);
+
#endif
--
1.7.1
[Qemu-devel] [PATCH 08/16] vnc: tight add PNG encoding
Introduce a new encoding: VNC_ENCODING_TIGHT_PNG (0x17) and a new
tight filter VNC_TIGHT_PNG (0x0A). When the client tells it supports the 0x17
encoding, the server will use tight, but will always send encoding pixels using
PNG instead of zlib. If the client also told it support JPEG, then the server
can
send JPEG, because PNG will only be used in the cases zlib was used in normal
tight.
This encoding was introduced to speed up HTML5 based VNC clients like noVNC
[1], but
can also be used on devices like iPhone where PNG can be rendered in hardware.
[1] http://github.com/kanaka/noVNC/
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
Makefile.target|1 +
configure | 37 +++
ui/vnc-enc-tight.c | 277 ++--
ui/vnc-enc-tight.h | 17 ++--
ui/vnc.c |9 +-
ui/vnc.h | 11 ++
6 files changed, 311 insertions(+), 41 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index d9e888a..a3ea025 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -178,6 +178,7 @@ LIBS+=-lz
QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
+QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
# xen backend driver support
obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
diff --git a/configure b/configure
index 304b4b0..d4c34f6 100755
--- a/configure
+++ b/configure
@@ -269,6 +269,7 @@ vde=
vnc_tls=
vnc_sasl=
vnc_jpeg=
+vnc_png=
xen=
linux_aio=
vhost_net=
@@ -579,6 +580,10 @@ for opt do
;;
--enable-vnc-jpeg) vnc_jpeg=yes
;;
+ --disable-vnc-png) vnc_png=no
+ ;;
+ --enable-vnc-png) vnc_png=yes
+ ;;
--disable-slirp) slirp=no
;;
--disable-uuid) uuid=no
@@ -825,6 +830,8 @@ echo --disable-vnc-sasl disable SASL encryption
for VNC server
echo --enable-vnc-saslenable SASL encryption for VNC server
echo --disable-vnc-jpeg disable JPEG lossy compression for VNC server
echo --enable-vnc-jpegenable JPEG lossy compression for VNC server
+echo --disable-vnc-pngdisable PNG compression for VNC server
+echo --enable-vnc-png enable PNG compression for VNC server
echo --disable-curses disable curses output
echo --enable-curses enable curses output
echo --disable-curl disable curl connectivity
@@ -1265,6 +1272,31 @@ EOF
fi
##
+# VNC PNG detection
+if test $vnc_png = yes ; then
+cat $TMPC EOF
+//#include stdio.h
+#include png.h
+int main(void) {
+png_structp png_ptr;
+png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
+return 0;
+}
+EOF
+vnc_png_cflags=
+vnc_png_libs=-lpng
+ if compile_prog $vnc_png_cflags $vnc_png_libs ; then
+vnc_png=yes
+libs_softmmu=$vnc_png_libs $libs_softmmu
+ else
+if test $vnc_png = yes ; then
+ feature_not_found vnc-png
+fi
+vnc_png=no
+ fi
+fi
+
+##
# fnmatch() probe, used for ACL routines
fnmatch=no
cat $TMPC EOF
@@ -2093,6 +2125,7 @@ echo Mixer emulation $mixemu
echo VNC TLS support $vnc_tls
echo VNC SASL support $vnc_sasl
echo VNC JPEG support $vnc_jpeg
+echo VNC PNG support $vnc_png
if test -n $sparc_cpu; then
echo Target Sparc Arch $sparc_cpu
fi
@@ -2233,6 +2266,10 @@ if test $vnc_jpeg = yes ; then
echo CONFIG_VNC_JPEG=y $config_host_mak
echo VNC_JPEG_CFLAGS=$vnc_jpeg_cflags $config_host_mak
fi
+if test $vnc_png = yes ; then
+ echo CONFIG_VNC_PNG=y $config_host_mak
+ echo VNC_PNG_CFLAGS=$vnc_png_cflags $config_host_mak
+fi
if test $fnmatch = yes ; then
echo CONFIG_FNMATCH=y $config_host_mak
fi
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 4ff88a8..007d88f 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -26,13 +26,18 @@
* THE SOFTWARE.
*/
-#include qemu-common.h
+#include config-host.h
+#ifdef CONFIG_VNC_PNG
+#include png.h
+#endif
#ifdef CONFIG_VNC_JPEG
#include stdio.h
#include jpeglib.h
#endif
+#include qemu-common.h
+
#include bswap.h
#include qdict.h
#include qint.h
@@ -63,6 +68,28 @@ static const struct {
{ 65536, 2048, 32, 8192, 9, 9, 9, 6, 200, 500, 96, 80, 200, 500 }
};
+#ifdef CONFIG_VNC_PNG
+static int send_png_rect(VncState *vs, int x, int y, int w, int h,
+ QDict *palette);
+
+static bool tight_can_send_png_rect(VncState *vs, int w, int h)
+{
+if (!vnc_has_feature(vs, VNC_FEATURE_TIGHT_PNG)) {
+return false;
+}
+
+if (ds_get_bytes_per_pixel(vs-ds) == 1 ||
+vs-clientds.pf.bytes_per_pixel == 1) {
+return false;
+}
+
+if (w * h VNC_TIGHT_PNG_MIN_RECT_SIZE) {
+return false;
+}
+return true;
+}
+#endif
+
/*
* Code to guess if given rectangle is suitable for smooth image
* compression (by applying gradient filter or JPEG coder).
@@ -466,6 +493,7 @@ static void print_palette(const char *key, QObject
[Qemu-devel] [PATCH 15/16] vnc: add missing lock for vnc_cursor_define()
All vnc_write() calls must be locked (except the ones present before
the protocol initialization).
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index cffe238..61f9a05 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -794,6 +794,7 @@ static int vnc_cursor_define(VncState *vs)
int isize;
if (vnc_has_feature(vs, VNC_FEATURE_RICH_CURSOR)) {
+vnc_lock_output(vs);
vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
vnc_write_u8(vs, 0); /* padding */
vnc_write_u16(vs, 1); /* # of rects */
@@ -802,6 +803,7 @@ static int vnc_cursor_define(VncState *vs)
isize = c-width * c-height * vs-clientds.pf.bytes_per_pixel;
vnc_write_pixels_generic(vs, pf, c-data, isize);
vnc_write(vs, vs-vd-cursor_mask, vs-vd-cursor_msize);
+vnc_unlock_output(vs);
return 0;
}
return -1;
--
1.7.1
[Qemu-devel] [PATCH 10/16] vnc: tight: stop using qdict for palette stuff
Profiling with callgrind seems to show that a lot of time is spent
in the palette code (mostly due to memory allocation and qdict to int
conversion).
This patch adds a VncPalette implementation. The palette is stored
in a hash table, like qdict, but which does way less memory allocations,
and doesn't suffer from the QObject overhead.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
Makefile.objs |2 +-
ui/vnc-enc-tight.c | 163 +++
ui/vnc-palette.c | 136 +++
ui/vnc-palette.h | 63
4 files changed, 235 insertions(+), 129 deletions(-)
create mode 100644 ui/vnc-palette.c
create mode 100644 ui/vnc-palette.h
diff --git a/Makefile.objs b/Makefile.objs
index e63c5eb..0ee9346 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -107,7 +107,7 @@ ui-obj-$(CONFIG_SDL) += sdl.o sdl_zoom.o x_keymap.o
ui-obj-$(CONFIG_CURSES) += curses.o
ui-obj-y += vnc.o d3des.o
ui-obj-y += vnc-enc-zlib.o vnc-enc-hextile.o
-ui-obj-y += vnc-enc-tight.o
+ui-obj-y += vnc-enc-tight.o vnc-palette.o
ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
ui-obj-$(CONFIG_COCOA) += cocoa.o
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 9e1f214..870e215 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -39,10 +39,10 @@
#include qemu-common.h
#include bswap.h
-#include qdict.h
#include qint.h
#include vnc.h
#include vnc-enc-tight.h
+#include vnc-palette.h
/* Compression level stuff. The following array contains various
encoder parameters for each of 10 compression levels (0..9).
@@ -85,7 +85,7 @@ static const struct {
};
static int send_png_rect(VncState *vs, int x, int y, int w, int h,
- QDict *palette);
+ VncPalette *palette);
static bool tight_can_send_png_rect(VncState *vs, int w, int h)
{
@@ -312,74 +312,13 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
/*
* Code to determine how many different colors used in rectangle.
*/
-
-static void tight_palette_rgb2buf(uint32_t rgb, int bpp, uint8_t buf[6])
-{
-memset(buf, 0, 6);
-
-if (bpp == 32) {
-buf[0] = ((rgb 24) 0xFF);
-buf[1] = ((rgb 16) 0xFF);
-buf[2] = ((rgb 8) 0xFF);
-buf[3] = ((rgb 0) 0xFF);
-buf[4] = ((buf[0] 1) == 0) 3 | ((buf[1] 1) == 0) 2;
-buf[4]|= ((buf[2] 1) == 0) 1 | ((buf[3] 1) == 0) 0;
-buf[0] |= 1;
-buf[1] |= 1;
-buf[2] |= 1;
-buf[3] |= 1;
-}
-if (bpp == 16) {
-buf[0] = ((rgb 8) 0xFF);
-buf[1] = ((rgb 0) 0xFF);
-buf[2] = ((buf[0] 1) == 0) 1 | ((buf[1] 1) == 0) 0;
-buf[0] |= 1;
-buf[1] |= 1;
-}
-}
-
-static uint32_t tight_palette_buf2rgb(int bpp, const uint8_t *buf)
-{
-uint32_t rgb = 0;
-
-if (bpp == 32) {
-rgb |= ((buf[0] ~1) | !((buf[4] 3) 1)) 24;
-rgb |= ((buf[1] ~1) | !((buf[4] 2) 1)) 16;
-rgb |= ((buf[2] ~1) | !((buf[4] 1) 1)) 8;
-rgb |= ((buf[3] ~1) | !((buf[4] 0) 1)) 0;
-}
-if (bpp == 16) {
-rgb |= ((buf[0] ~1) | !((buf[2] 1) 1)) 8;
-rgb |= ((buf[1] ~1) | !((buf[2] 0) 1)) 0;
-}
-return rgb;
-}
-
-
-static int tight_palette_insert(QDict *palette, uint32_t rgb, int bpp, int max)
-{
-uint8_t key[6];
-int idx = qdict_size(palette);
-bool present;
-
-tight_palette_rgb2buf(rgb, bpp, key);
-present = qdict_haskey(palette, (char *)key);
-if (idx = max !present) {
-return 0;
-}
-if (!present) {
-qdict_put(palette, (char *)key, qint_from_int(idx));
-}
-return qdict_size(palette);
-}
-
#define DEFINE_FILL_PALETTE_FUNCTION(bpp) \
\
static int \
tight_fill_palette##bpp(VncState *vs, int x, int y, \
int max, size_t count, \
uint32_t *bg, uint32_t *fg, \
-struct QDict **palette) { \
+VncPalette **palette) { \
uint##bpp##_t *data;\
uint##bpp##_t c0, c1, ci; \
int i, n0, n1; \
@@ -426,24 +365,23 @@ static int tight_palette_insert(QDict *palette, uint32_t
rgb, int bpp, int max)
return 0; \
} \
\
-*palette = qdict_new();
[Qemu-devel] [PATCH 11/16] vnc: encapsulate encoding members
This will allow to implement the threaded VNC server in a
more cleaner way.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc-enc-hextile.c | 14 ++--
ui/vnc-enc-tight.c | 194 +-
ui/vnc-enc-zlib.c| 34 +-
ui/vnc.c |8 +-
ui/vnc.h | 58 ---
5 files changed, 157 insertions(+), 151 deletions(-)
diff --git a/ui/vnc-enc-hextile.c b/ui/vnc-enc-hextile.c
index fa4b264..364a491 100644
--- a/ui/vnc-enc-hextile.c
+++ b/ui/vnc-enc-hextile.c
@@ -75,7 +75,7 @@ int vnc_hextile_send_framebuffer_update(VncState *vs, int x,
has_fg = has_bg = 0;
for (j = y; j (y + h); j += 16) {
for (i = x; i (x + w); i += 16) {
-vs-send_hextile_tile(vs, i, j,
+vs-hextile.send_tile(vs, i, j,
MIN(16, x + w - i), MIN(16, y + h - j),
last_bg, last_fg, has_bg, has_fg);
}
@@ -91,25 +91,25 @@ void vnc_hextile_set_pixel_conversion(VncState *vs, int
generic)
if (!generic) {
switch (vs-ds-surface-pf.bits_per_pixel) {
case 8:
-vs-send_hextile_tile = send_hextile_tile_8;
+vs-hextile.send_tile = send_hextile_tile_8;
break;
case 16:
-vs-send_hextile_tile = send_hextile_tile_16;
+vs-hextile.send_tile = send_hextile_tile_16;
break;
case 32:
-vs-send_hextile_tile = send_hextile_tile_32;
+vs-hextile.send_tile = send_hextile_tile_32;
break;
}
} else {
switch (vs-ds-surface-pf.bits_per_pixel) {
case 8:
-vs-send_hextile_tile = send_hextile_tile_generic_8;
+vs-hextile.send_tile = send_hextile_tile_generic_8;
break;
case 16:
-vs-send_hextile_tile = send_hextile_tile_generic_16;
+vs-hextile.send_tile = send_hextile_tile_generic_16;
break;
case 32:
-vs-send_hextile_tile = send_hextile_tile_generic_32;
+vs-hextile.send_tile = send_hextile_tile_generic_32;
break;
}
}
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 870e215..dbde08d 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -120,7 +120,7 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
int pixels = 0;
int pix, left[3];
uint errors;
-unsigned char *buf = vs-tight.buffer;
+unsigned char *buf = vs-tight.tight.buffer;
/*
* If client is big-endian, color samples begin from the second
@@ -187,7 +187,7 @@ tight_detect_smooth_image24(VncState *vs, int w, int h)
int pixels = 0; \
int sample, sum, left[3]; \
uint errors;\
-unsigned char *buf = vs-tight.buffer; \
+unsigned char *buf = vs-tight.tight.buffer;\
\
endian = ((vs-clientds.flags QEMU_BIG_ENDIAN_FLAG) !=\
(vs-ds-surface-flags QEMU_BIG_ENDIAN_FLAG)); \
@@ -267,8 +267,8 @@ static int
tight_detect_smooth_image(VncState *vs, int w, int h)
{
uint errors;
-int compression = vs-tight_compression;
-int quality = vs-tight_quality;
+int compression = vs-tight.compression;
+int quality = vs-tight.quality;
if (!vs-vd-lossy) {
return 0;
@@ -280,7 +280,7 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
return 0;
}
-if (vs-tight_quality != -1) {
+if (vs-tight.quality != -1) {
if (w * h VNC_TIGHT_JPEG_MIN_RECT_SIZE) {
return 0;
}
@@ -291,9 +291,9 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
}
if (vs-clientds.pf.bytes_per_pixel == 4) {
-if (vs-tight_pixel24) {
+if (vs-tight.pixel24) {
errors = tight_detect_smooth_image24(vs, w, h);
-if (vs-tight_quality != -1) {
+if (vs-tight.quality != -1) {
return (errors tight_conf[quality].jpeg_threshold24);
}
return (errors tight_conf[compression].gradient_threshold24);
@@ -323,7 +323,7 @@ tight_detect_smooth_image(VncState *vs, int w, int h)
uint##bpp##_t c0, c1, ci; \
int i, n0, n1; \
\
-data = (uint##bpp##_t *)vs-tight.buffer; \
+data = (uint##bpp##_t *)vs-tight.tight.buffer; \
[Qemu-devel] [Bug 594944] [NEW] --enable-debug error
Public bug reported:
This bug was already reported in 0.12.3, and in the present 0.12.4 is still
present:
CCsparc-softmmu/disas.o
CCsparc64-softmmu/ide/qdev.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error:
incompatible type for argument 3 of ‘tcg_gen_add_i32’
CCsparc64-softmmu/ide/pci.o
CCsparc-softmmu/i386-dis.o
make[1]: *** [translate.o] Error 1
make: *** [subdir-sparc64-linux-user] Error 2
make: *** Waiting for unfinished jobs
CCsparc64-softmmu/ide/cmd646.o
CCsparc64-softmmu/vga.o
CCsparc64-softmmu/vga-pci.o
CCsparc64-softmmu/fdc.o
CCsparc64-softmmu/mc146818rtc.o
CCsparc-softmmu/sparc-dis.o
CCsparc64-softmmu/serial.o
CCsparc64-softmmu/cirrus_vga.o
CCsparc64-softmmu/parallel.o
CCsparc64-softmmu/exec.o
CCsparc64-softmmu/translate-all.o
ARsparc-softmmu/libqemu.a
LINK sparc-softmmu/qemu-system-sparc
CCsparc64-softmmu/cpu-exec.o
CCsparc64-softmmu/translate.o
CCsparc64-softmmu/tcg/tcg.o
CCsparc64-softmmu/fpu/softfloat.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error:
incompatible type for argument 3 of ‘tcg_gen_add_i32’
make[1]: *** [translate.o] Error 1
make[1]: *** Waiting for unfinished jobs
make: *** [subdir-sparc64-softmmu] Error 2
The following patch seemed to work:
diff -Nurp target-sparc/translate.c.orig target-sparc/translate.c
--- target-sparc/translate.c.orig 2010-06-16 13:58:26.395527708 +0800
+++ target-sparc/translate.c2010-06-16 14:09:18.683573175 +0800
@@ -1663,27 +1663,28 @@ static inline TCGv get_src2(unsigned int
#ifdef TARGET_SPARC64
static inline void gen_load_trap_state_at_tl(TCGv_ptr r_tsptr, TCGv_ptr
cpu_env)
{
-TCGv r_tl = tcg_temp_new();
+TCGv_i32 r_tl = tcg_temp_new_i32();
/* load env-tl into r_tl */
-{
-TCGv_i32 r_tl_tmp = tcg_temp_new_i32();
-tcg_gen_ld_i32(r_tl_tmp, cpu_env, offsetof(CPUSPARCState, tl));
-tcg_gen_ext_i32_tl(r_tl, r_tl_tmp);
-tcg_temp_free_i32(r_tl_tmp);
-}
+
+tcg_gen_ld_i32(r_tl, cpu_env, offsetof(CPUSPARCState, tl));
/* tl = [0 ... MAXTL_MASK] where MAXTL_MASK must be power of 2 */
-tcg_gen_andi_tl(r_tl, r_tl, MAXTL_MASK);
+tcg_gen_andi_i32(r_tl, r_tl, MAXTL_MASK);
/* calculate offset to current trap state from env-ts, reuse r_tl */
-tcg_gen_muli_tl(r_tl, r_tl, sizeof (trap_state));
+tcg_gen_muli_i32(r_tl, r_tl, sizeof (trap_state));
tcg_gen_addi_ptr(r_tsptr, cpu_env, offsetof(CPUState, ts));
/* tsptr = env-ts[env-tl MAXTL_MASK] */
-tcg_gen_add_ptr(r_tsptr, r_tsptr, r_tl);
+{
+TCGv_ptr r_tl_tmp = tcg_temp_new_ptr();
+tcg_gen_ext_i32_ptr(r_tl_tmp, r_tl);
+tcg_gen_add_ptr(r_tsptr, r_tsptr, r_tl_tmp);
+tcg_temp_free_i32(r_tl_tmp);
+}
-tcg_temp_free(r_tl);
+tcg_temp_free_i32(r_tl);
}
#endif
It is following the previous bug fixs by Jay Foad:
http://www.mail-archive.com/qemu-devel@nongnu.org/msg25585.html
Not sure if it is correct?
** Affects: qemu
Importance: Undecided
Status: New
--
--enable-debug error
https://bugs.launchpad.net/bugs/594944
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
Status in QEMU: New
Bug description:
This bug was already reported in 0.12.3, and in the present 0.12.4 is still
present:
CCsparc-softmmu/disas.o
CCsparc64-softmmu/ide/qdev.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error:
incompatible type for argument 3 of ‘tcg_gen_add_i32’
CCsparc64-softmmu/ide/pci.o
CCsparc-softmmu/i386-dis.o
make[1]: *** [translate.o] Error 1
make: *** [subdir-sparc64-linux-user] Error 2
make: *** Waiting for unfinished jobs
CCsparc64-softmmu/ide/cmd646.o
CCsparc64-softmmu/vga.o
CCsparc64-softmmu/vga-pci.o
CCsparc64-softmmu/fdc.o
CCsparc64-softmmu/mc146818rtc.o
CCsparc-softmmu/sparc-dis.o
CCsparc64-softmmu/serial.o
CCsparc64-softmmu/cirrus_vga.o
CCsparc64-softmmu/parallel.o
CCsparc64-softmmu/exec.o
CCsparc64-softmmu/translate-all.o
ARsparc-softmmu/libqemu.a
LINK sparc-softmmu/qemu-system-sparc
CCsparc64-softmmu/cpu-exec.o
CCsparc64-softmmu/translate.o
CCsparc64-softmmu/tcg/tcg.o
CCsparc64-softmmu/fpu/softfloat.o
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c: In function
‘gen_load_trap_state_at_tl’:
/root/download/qemu/qemu-0.12.4/target-sparc/translate.c:1684: error:
incompatible type for argument 3 of ‘tcg_gen_add_i32’
make[1]: *** [translate.o] Error 1
make[1]: ***
[Qemu-devel] [PATCH 12/16] vnc: fix tight png memory leak
The tight.png buffer was never released. Signed-off-by: Corentin Chary corenti...@iksaif.net --- ui/vnc-enc-tight.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index dbde08d..1d926c9 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -1659,4 +1659,7 @@ void vnc_tight_clear(VncState *vs) #ifdef CONFIG_VNC_JPEG buffer_free(vs-tight.jpeg); #endif +#ifdef CONFIG_VNC_PNG +buffer_free(vs-tight.png); +#endif } -- 1.7.1
[Qemu-devel] [PATCH 16/16] vnc: tight: don't limit png rect size
PNG was introduced because some vnc HTML5 clients like noVNC
have slow zlib decoding, but really fast PNG rendering. This
means that if PNG is enabled we should send only PNG (and JPEG, fill),
and never something compressed directly with zlib.
Signed-off-by: Corentin Chary corenti...@iksaif.net
---
ui/vnc-enc-tight.c |4
ui/vnc-enc-tight.h |1 -
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 1d926c9..e171074 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -97,10 +97,6 @@ static bool tight_can_send_png_rect(VncState *vs, int w, int
h)
vs-clientds.pf.bytes_per_pixel == 1) {
return false;
}
-
-if (w * h VNC_TIGHT_PNG_MIN_RECT_SIZE) {
-return false;
-}
return true;
}
#endif
diff --git a/ui/vnc-enc-tight.h b/ui/vnc-enc-tight.h
index dc7150a..a3add78 100644
--- a/ui/vnc-enc-tight.h
+++ b/ui/vnc-enc-tight.h
@@ -176,7 +176,6 @@
#define VNC_TIGHT_MAX_SPLIT_TILE_SIZE 16
#define VNC_TIGHT_JPEG_MIN_RECT_SIZE 4096
-#define VNC_TIGHT_PNG_MIN_RECT_SIZE 4096
#define VNC_TIGHT_DETECT_SUBROW_WIDTH7
#define VNC_TIGHT_DETECT_MIN_WIDTH 8
#define VNC_TIGHT_DETECT_MIN_HEIGHT 8
--
1.7.1
[Qemu-devel] [PATCH 14/16] vnc: threaded VNC server
Implement a threaded VNC server using the producer-consumer model. The main thread will push encoding jobs (a list a rectangles to update) in a queue, and the VNC worker thread will consume that queue and send framebuffer updates to the output buffer. The threaded VNC server can be enabled with ./configure --enable-vnc-thread. If you don't want it, just use ./configure --disable-vnc-thread and a syncrhonous queue of job will be used (which as exactly the same behavior as the old queue). If you disable the VNC thread, all thread related code will not be built and there will be no overhead. Signed-off-by: Corentin Chary corenti...@iksaif.net --- Makefile.objs |7 +- configure | 13 ++ ui/vnc-jobs-async.c | 331 +++ ui/vnc-jobs-sync.c | 73 +++ ui/vnc-jobs.h | 87 ++ ui/vnc.c| 144 +++ ui/vnc.h| 53 - 7 files changed, 682 insertions(+), 26 deletions(-) create mode 100644 ui/vnc-jobs-async.c create mode 100644 ui/vnc-jobs-sync.c create mode 100644 ui/vnc-jobs.h diff --git a/Makefile.objs b/Makefile.objs index 0ee9346..866ddf1 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -111,10 +111,15 @@ ui-obj-y += vnc-enc-tight.o vnc-palette.o ui-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o ui-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o ui-obj-$(CONFIG_COCOA) += cocoa.o +ifdef CONFIG_VNC_THREAD +ui-obj-y += vnc-jobs-async.o +else +ui-obj-y += vnc-jobs-sync.o +endif common-obj-y += $(addprefix ui/, $(ui-obj-y)) common-obj-y += iov.o acl.o -common-obj-$(CONFIG_IOTHREAD) += qemu-thread.o +common-obj-$(CONFIG_THREAD) += qemu-thread.o common-obj-y += notify.o event_notifier.o common-obj-y += qemu-timer.o diff --git a/configure b/configure index d4c34f6..f5fdb8f 100755 --- a/configure +++ b/configure @@ -270,6 +270,7 @@ vnc_tls= vnc_sasl= vnc_jpeg= vnc_png= +vnc_thread= xen= linux_aio= vhost_net= @@ -584,6 +585,10 @@ for opt do ;; --enable-vnc-png) vnc_png=yes ;; + --disable-vnc-thread) vnc_thread=no + ;; + --enable-vnc-thread) vnc_thread=yes + ;; --disable-slirp) slirp=no ;; --disable-uuid) uuid=no @@ -832,6 +837,8 @@ echo --disable-vnc-jpeg disable JPEG lossy compression for VNC server echo --enable-vnc-jpegenable JPEG lossy compression for VNC server echo --disable-vnc-pngdisable PNG compression for VNC server echo --enable-vnc-png enable PNG compression for VNC server +echo --disable-vnc-thread disable threaded VNC server +echo --enable-vnc-thread enable threaded VNC server echo --disable-curses disable curses output echo --enable-curses enable curses output echo --disable-curl disable curl connectivity @@ -2126,6 +2133,7 @@ echo VNC TLS support $vnc_tls echo VNC SASL support $vnc_sasl echo VNC JPEG support $vnc_jpeg echo VNC PNG support $vnc_png +echo VNC thread$vnc_thread if test -n $sparc_cpu; then echo Target Sparc Arch $sparc_cpu fi @@ -2270,6 +2278,10 @@ if test $vnc_png = yes ; then echo CONFIG_VNC_PNG=y $config_host_mak echo VNC_PNG_CFLAGS=$vnc_png_cflags $config_host_mak fi +if test $vnc_thread = yes ; then + echo CONFIG_VNC_THREAD=y $config_host_mak + echo CONFIG_THREAD=y $config_host_mak +fi if test $fnmatch = yes ; then echo CONFIG_FNMATCH=y $config_host_mak fi @@ -2346,6 +2358,7 @@ if test $xen = yes ; then fi if test $io_thread = yes ; then echo CONFIG_IOTHREAD=y $config_host_mak + echo CONFIG_THREAD=y $config_host_mak fi if test $linux_aio = yes ; then echo CONFIG_LINUX_AIO=y $config_host_mak diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c new file mode 100644 index 000..6e9cf08 --- /dev/null +++ b/ui/vnc-jobs-async.c @@ -0,0 +1,331 @@ +/* + * QEMU VNC display driver + * + * Copyright (C) 2006 Anthony Liguori anth...@codemonkey.ws + * Copyright (C) 2006 Fabrice Bellard + * Copyright (C) 2009 Red Hat, Inc + * Copyright (C) 2010 Corentin Chary corentin.ch...@gmail.com + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
Re: [Qemu-devel] Re: KVM call minutes for June 15
Anthony Liguori anth...@codemonkey.ws writes: On 06/15/2010 10:41 AM, Christoph Hellwig wrote: On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote: KVM/qemu patches - patch rate is high, documentation is low, review is low - patches need to include better descriptions and documentation - will slow down patch writers - will make it easier for patch reviewers What is the qemu patch review policy anyway? We don't really have a coherent policy. Suggestions for improvement are always appreciated. There are no Reviewed-by: included in the actual commits, Reviewed-by/Ack-by's are pretty helpful for me. In terms of including them in commit messages, if there's a strong feeling that that would be helpful then it's something I can look at doing but it also requires a fair bit of manual work during commit. Can't hurt reviewer motivation. Could it be automated? Find replies, extract tags. If you want your acks to be picked up, you better make sure your References header works, and your tags are formatted correctly. [...]
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
Gleb Natapov wrote:
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
From: Jan Kiszka jan.kis...@siemens.com
There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic
interface for exploring supported fw-cfg keys?
Jan
Hence special value. I used it incorrectly in may v2 seabios patch. Will
resend asap. Will teach me to not change logic at the last minute :( I
removed valid field between v1 and v2 of the patches and introduces
special value for count instead. As a result I made one bug in qemu and
one is seabios. Heh.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/hpet.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/hw/hpet.c b/hw/hpet.c
index d5c406c..ed4e995 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -74,7 +74,7 @@ typedef struct HPETState {
uint8_t hpet_id; /* instance id */
} HPETState;
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
static uint32_t hpet_in_legacy_mode(HPETState *s)
{
@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
int i, iomemtype;
HPETTimer *timer;
-if (hpet_cfg.count == UINT8_MAX) {
-/* first instance */
-hpet_cfg.count = 0;
-}
-
if (hpet_cfg.count == 8) {
fprintf(stderr, Only 8 instances of HPET is allowed\n);
return -1;
--
1.6.0.2
--
Gleb.
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: Using Qemu 0.12.3 and gdb 7.1 on Ubuntu Lucid. I'm trying to debug some bootloader code. Using qemu -s -S to run the bootloader and gdb to connect to qemu, I set the breakpoint at 0x7c00. Then I type continue in gdb. The breakpoint is hit and gdb shows debug information. However qemu apparently continues to execute the code of the bootloader as the text is printed on the screen etc.
Re: [Qemu-devel] [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? J
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote:
Gleb Natapov wrote:
On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote:
From: Jan Kiszka jan.kis...@siemens.com
There is no need starting with the special value for hpet_cfg.count.
Either Seabios is aware of the new firmware interface and properly
interprets the counter or it simply ignores it anyway.
I want seabios to be able to distinguish between old qemu and new one.
I see now. But isn't it a good chance to introduce a proper generic
interface for exploring supported fw-cfg keys?
Having such interface would be nice. Pity we haven't introduced it from
the start. If we do it now seabios will have to find out somehow that
qemu support such interface. Chicken and egg ;)
Jan
Hence special value. I used it incorrectly in may v2 seabios patch. Will
resend asap. Will teach me to not change logic at the last minute :( I
removed valid field between v1 and v2 of the patches and introduces
special value for count instead. As a result I made one bug in qemu and
one is seabios. Heh.
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
hw/hpet.c |7 +--
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/hw/hpet.c b/hw/hpet.c
index d5c406c..ed4e995 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -74,7 +74,7 @@ typedef struct HPETState {
uint8_t hpet_id; /* instance id */
} HPETState;
-struct hpet_fw_config hpet_cfg = {.count = UINT8_MAX};
+struct hpet_fw_config hpet_cfg;
static uint32_t hpet_in_legacy_mode(HPETState *s)
{
@@ -682,11 +682,6 @@ static int hpet_init(SysBusDevice *dev)
int i, iomemtype;
HPETTimer *timer;
-if (hpet_cfg.count == UINT8_MAX) {
-/* first instance */
-hpet_cfg.count = 0;
-}
-
if (hpet_cfg.count == 8) {
fprintf(stderr, Only 8 instances of HPET is allowed\n);
return -1;
--
1.6.0.2
--
Gleb.
--
Gleb.
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? thanks, J
Re: [Qemu-devel] [PATCH 1/4] savevm: refactor qemu_loadvm_state().
Anthony Liguori wrote:
On 06/14/2010 09:01 PM, Yoshiaki Tamura wrote:
Anthony Liguori wrote:
On 06/03/2010 02:22 AM, Yoshiaki Tamura wrote:
Split qemu_loadvm_state(), and introduce
qemu_loadvm_state_{begin,iterate,complete,async}.
qemu_loadvm_state_async() is a function to handle a single incoming
section.
Signed-off-by: Yoshiaki Tamuratamura.yoshi...@lab.ntt.co.jp
---
savevm.c | 206
+++---
sysemu.h | 2 +
2 files changed, 146 insertions(+), 62 deletions(-)
diff --git a/savevm.c b/savevm.c
index dc20390..aa4f98c 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1005,6 +1005,8 @@ typedef struct SaveStateEntry {
static QTAILQ_HEAD(savevm_handlers, SaveStateEntry) savevm_handlers =
QTAILQ_HEAD_INITIALIZER(savevm_handlers);
+static QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
+ QLIST_HEAD_INITIALIZER(loadvm_handlers);
static int global_section_id;
static int calculate_new_instance_id(const char *idstr)
@@ -1460,14 +1462,9 @@ typedef struct LoadStateEntry {
int version_id;
} LoadStateEntry;
-int qemu_loadvm_state(QEMUFile *f)
+int qemu_loadvm_state_begin(QEMUFile *f)
{
- QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
- QLIST_HEAD_INITIALIZER(loadvm_handlers);
- LoadStateEntry *le, *new_le;
- uint8_t section_type;
unsigned int v;
- int ret;
v = qemu_get_be32(f);
if (v != QEMU_VM_FILE_MAGIC)
@@ -1481,73 +1478,157 @@ int qemu_loadvm_state(QEMUFile *f)
if (v != QEMU_VM_FILE_VERSION)
return -ENOTSUP;
- while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
- uint32_t instance_id, version_id, section_id;
- SaveStateEntry *se;
- char idstr[257];
- int len;
+ return 0;
+}
- switch (section_type) {
- case QEMU_VM_SECTION_START:
- case QEMU_VM_SECTION_FULL:
- /* Read section start */
- section_id = qemu_get_be32(f);
- len = qemu_get_byte(f);
- qemu_get_buffer(f, (uint8_t *)idstr, len);
- idstr[len] = 0;
- instance_id = qemu_get_be32(f);
- version_id = qemu_get_be32(f);
-
- /* Find savevm section */
- se = find_se(idstr, instance_id);
- if (se == NULL) {
- fprintf(stderr, Unknown savevm section or instance '%s' %d\n,
idstr, instance_id);
- ret = -EINVAL;
- goto out;
- }
+static int qemu_loadvm_state_iterate(QEMUFile *f)
+{
+ LoadStateEntry *le;
+ uint32_t section_id;
+ int ret;
- /* Validate version */
- if (version_id se-version_id) {
- fprintf(stderr, savevm: unsupported version %d for '%s' v%d\n,
- version_id, idstr, se-version_id);
- ret = -EINVAL;
- goto out;
- }
+ section_id = qemu_get_be32(f);
But we're still blocking in qemu_get_be32() so I don't see how this
makes it async.
In that sense, it's not completely async, but still better than being
in the while loop that doesn't accept any commands on the incoming side.
What's confusing me is I don't understand how it's accepting command on
the incoming side.
By my reading of the code, you set a callback on read and then in the
read callback, you invoke iterate(). When iterate completes, you fall
back to the main loop. This sort of works only because the effect is
that after each iteration, by falling back to the main loop you can run
all handlers (including the monitor's handlers).
But there are some serious problems with this approach. When iterate()
completes, you've not necessarily exhausted the QEMUFile's buffer. If
the buffer is holding the full contents of the final stage of migration,
then there is not going to be any more data available to read from the
socket which means that when you drop to the main loop, you'll never
execute async again. You could probably easily reproduce this problem by
making the QEMUFile buffer very large. I think you're getting lucky
because the final stage is probably larger than 32k in most circumstances.
In the very least, you should use a bottom half instead
qemu_set_fd_handler2. It's still not async but I'm not sure whether it's
a good enough solution.
Thank you for pointing out the problem.
So instead of waiting data with select which is event driven, bottom half would
at least give us a chance to check the QEMUFile's buffer with specific interval.
This should fix the problem you mentioned above. However, if the network
connection is lost, as you imagine, we'll still be blocked at fill_buffer(), I
guess.
What I eventually need is a mechanism to prevent from blocking during migration,
so using bottom half instead of qemu_set_fd_handler2 isn't perfect solution for
me. Probably, introducing a header which describes the data to be received into
the savevm format, and select until incoming side receives the expected data
could be a solution. But changing the savevm format just for this doesn't seem
to be good.
I believe some people also needs async incoming migration framework too. (Juan?)
What I would like to ask is, should I keep working to improve this approach?
If it's not a good approach for making other people happy, I would like to drop
this work for now, and discuss a better solution in the future.
Thanks,
Yoshi
Regards,
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. That will not support holes in key space, and our key space is already sparse. -- Gleb.
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
[Please keep the list in CC] chimerane...@gmail.com wrote: I never thought about this, since I always used qemu + gdb without having this kind of trouble.. I just finished compiling version 0.12.4, and I will surely try what you are suggesting, I really appreciate your help. The restriction doesn't bite you that hard when in flat mode, the mode modern OSes run in most of the time. And you can manually work around it to some degree by letting qemu dump the registers (monitor info registers) and doing some manual calculation, e.g. for offset-based gs/fs addressing (per-CPU variable under Linux). Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, Jun 16, 2010 at 4:49 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. do you mean gdb has no command to show the values of these registers? or you mean it doenst have anyway to get notified when these registers are modified? (i dont see how this is useful for debugging, anway) thanks, J
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. That will not support holes in key space, and our key space is already sparse. Then add a service to obtain a bitmap of supported keys. If that bitmap is empty... Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Jun Koi wrote: On Wed, Jun 16, 2010 at 4:49 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. do you mean gdb has no command to show the values of these registers? or you mean it doenst have anyway to get notified when these registers are modified? (i dont see how this is useful for debugging, anway) Both: Neither supports gdb them as part of its register set nor does the remote gdb protocol transport them. You need this for segmented addressing, either in real mode (linear address = segment * 16 + offset) or in segmented protected mode (less common in modern OSes, but at least still used for per-CPU variables in Linux). And you need a way to detect the current operation mode at all to switch between 16/32, and 64 bit registers (set arch i386 vs. i386:x86-64). You don't need all this for application-level debugging, and that's why gdb lacks it so far. Jan signature.asc Description: OpenPGP digital signature
[Qemu-devel] Re: Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Ok this is it: I've just compiled and tried qemu 0.12.4, this bug has been fixed and it works perfectly again, I'm adding a report to launchpad within the next few seconds.. Thank you for the support.
[Qemu-devel] [PATCH v6 3/4] Use cpu_physical_memory_set_dirty_range() to update dirty bitmap.
Modifies kvm_physical_sync_dirty_bitmap to use
cpu_physical_memory_set_dirty_range() to update the row of the
bit-based dirty bitmap at once.
Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
kvm-all.c | 24 +---
1 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/kvm-all.c b/kvm-all.c
index c238f54..0d29798 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -290,8 +290,8 @@ static int kvm_get_dirty_pages_log_range(unsigned long
start_addr,
unsigned long offset,
unsigned long mem_size)
{
-unsigned int i, j;
-unsigned long page_number, addr, addr1, c;
+unsigned int i;
+unsigned long page_number, addr, addr1;
ram_addr_t ram_addr;
unsigned int len = ((mem_size / TARGET_PAGE_SIZE) + HOST_LONG_BITS - 1) /
HOST_LONG_BITS;
@@ -302,23 +302,17 @@ static int kvm_get_dirty_pages_log_range(unsigned long
start_addr,
*/
for (i = 0; i len; i++) {
if (bitmap[i] != 0) {
-c = leul_to_cpu(bitmap[i]);
-do {
-j = ffsl(c) - 1;
-c = ~(1ul j);
-page_number = i * HOST_LONG_BITS + j;
-addr1 = page_number * TARGET_PAGE_SIZE;
-addr = offset + addr1;
-ram_addr = cpu_get_physical_page_desc(addr);
-cpu_physical_memory_set_dirty(ram_addr);
-} while (c != 0);
+page_number = i * HOST_LONG_BITS;
+addr1 = page_number * TARGET_PAGE_SIZE;
+addr = offset + addr1;
+ram_addr = cpu_get_physical_page_desc(addr);
+cpu_physical_memory_set_dirty_range(ram_addr,
+leul_to_cpu(bitmap[i]));
}
}
return 0;
}
-#define ALIGN(x, y) (((x)+(y)-1) ~((y)-1))
-
/**
* kvm_physical_sync_dirty_bitmap - Grab dirty bitmap from kernel space
* This function updates qemu's dirty bitmap using
cpu_physical_memory_set_dirty().
@@ -343,7 +337,7 @@ static int
kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr,
break;
}
-size = ALIGN(((mem-memory_size) TARGET_PAGE_BITS), HOST_LONG_BITS)
/ 8;
+size = BITMAP_SIZE(mem-memory_size);
if (!d.dirty_bitmap) {
d.dirty_bitmap = qemu_malloc(size);
} else if (size allocated_size) {
--
1.7.0.31.g1df487
[Qemu-devel] [PATCH v6 0/4] Introduce bit-based dirty bitmap, and bit-based dirty page checker.
Updated the series to conform to newly introduced RAMList struct.
The dirty and non-dirty pages are checked one by one. When most of
the memory is not dirty, checking the dirty and non-dirty pages by
multiple page size should be much faster than checking them one by
one. We introduced bit-based dirty bitmap for VGA, CODE, MIGRATION,
MASTER, and cpu_physical_memory_get_dirty_range() for this purpose.
The following numbers show the speed up of bit-based dirty bitmap.
The speed up grows when the number of rows, whose contents are 0, gets
larger.
Test Environment:
CPU: 4x Intel Xeon Quad Core 2.66GHz
Mem size: 96GB
Host OS: CentOS (kernel 2.6.33)
Guest OS: Debian/GNU Linux lenny (kernel 2.6.26)
Guest Mem size: 512MB
Conditions of experiments are as follows:
Cond1: Guest OS periodically makes the 256MB continuous dirty pages.
Cond2: Guest OS periodically makes the 256MB dirty pages and non-dirty pages
in turn.
Cond3: Guest OS read 1GB file, which is bigger than memory.
Cond4: Guest OS write 1GB file, which is bigger than memory.
Experimental results:
Cond1: 5 ??? 83 times speed up
Cond2: 5 ??? 52 times speed up
Cond3: 5 ??? 132 times speed up
Cond4: 5 ??? 57 times speed up
Changes from v5 to v6 are:
- Conformed to newly introduced RAMList struct.
- Rebased to HEAD (fd42deeb4cb42f90084046e3ebdb4383953195e3)
Changes from v4 to v5 are:
- Rebased to HEAD (0ffbba357c557d9fa5caf9476878a4b9c155a614)
- Use BITMAP_SIZE() in kvm_physical_sync_dirty_bitmap() (3/4)
Changes from v3 to v4 are:
- Merged {1,2,3}/6 to compile correctly.
- Fix setting bits after phys_ram_dirty allocation.
- renamed DIRTY_FLAG and DIRTY_IDX converter function.
Changes from v2 to v3 are:
- Change FLAGS value to (1,2,4,8), and add IDX (0,1,2,3)
- Use ffs to convert FLAGS to IDX.
- Add a helper function which takes IDX.
- Change the behavior of MASTER as a buffer.
- Change dirty bitmap access to a loop.
- Add brace after if ()
Yoshiaki Tamura (4):
Modify DIRTY_FLAG value and introduce DIRTY_IDX to use as indexes of
bit-based dirty bitmap.
Introduce cpu_physical_memory_get_dirty_range().
Use cpu_physical_memory_set_dirty_range() to update dirty bitmap.
Use cpu_physical_memory_get_dirty_range() to check multiple dirty
pages.
arch_init.c | 58 --
cpu-all.h | 131 -
exec.c| 81 +--
kvm-all.c | 24 ---
qemu-common.h |3 +
5 files changed, 236 insertions(+), 61 deletions(-)
[Qemu-devel] [PATCH v6 1/4] Modify DIRTY_FLAG value and introduce DIRTY_IDX to use as indexes of bit-based dirty bitmap.
Replaces byte-based dirty bitmap with four (MASTER, VGA, CODE,
MIGRATION) bit-based dirty bitmap. On allocation, it sets all bits in
the bitmap. It uses ffs() to convert DIRTY_FLAG to DIRTY_IDX.
Modifies wrapper functions for byte-based dirty bitmap to bit-based
dirty bitmap. MASTER works as a buffer, and upon get_diry() or
get_dirty_flags(), it calls cpu_physical_memory_sync_master() to
update VGA and MIGRATION.
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
---
cpu-all.h | 127 -
exec.c| 14 +--
qemu-common.h |3 +
3 files changed, 120 insertions(+), 24 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index e31c2de..fcccf6f 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -37,6 +37,9 @@
#include softfloat.h
+/* to use ffs in flag_to_idx() */
+#include strings.h
+
#if defined(HOST_WORDS_BIGENDIAN) != defined(TARGET_WORDS_BIGENDIAN)
#define BSWAP_NEEDED
#endif
@@ -861,6 +864,18 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUState *env,
target_ulong addr);
extern int phys_ram_fd;
extern ram_addr_t ram_size;
+/* Use DIRTY_IDX as indexes of bit-based phys_ram_dirty. */
+#define MASTER_DIRTY_IDX0
+#define VGA_DIRTY_IDX 1
+#define CODE_DIRTY_IDX 2
+#define MIGRATION_DIRTY_IDX 3
+#define NUM_DIRTY_IDX 4
+
+#define MASTER_DIRTY_FLAG(1 MASTER_DIRTY_IDX)
+#define VGA_DIRTY_FLAG (1 VGA_DIRTY_IDX)
+#define CODE_DIRTY_FLAG (1 CODE_DIRTY_IDX)
+#define MIGRATION_DIRTY_FLAG (1 MIGRATION_DIRTY_IDX)
+
typedef struct RAMBlock {
uint8_t *host;
ram_addr_t offset;
@@ -869,7 +884,7 @@ typedef struct RAMBlock {
} RAMBlock;
typedef struct RAMList {
-uint8_t *phys_dirty;
+unsigned long *phys_dirty[NUM_DIRTY_IDX];
ram_addr_t last_offset;
QLIST_HEAD(ram, RAMBlock) blocks;
} RAMList;
@@ -896,51 +911,123 @@ extern int mem_prealloc;
/* Set if TLB entry is an IO callback. */
#define TLB_MMIO(1 5)
-#define VGA_DIRTY_FLAG 0x01
-#define CODE_DIRTY_FLAG 0x02
-#define MIGRATION_DIRTY_FLAG 0x08
+static inline int dirty_flag_to_idx(int flag)
+{
+return ffs(flag) - 1;
+}
+
+static inline int dirty_idx_to_flag(int idx)
+{
+return 1 idx;
+}
/* read dirty bit (return 0 or 1) */
static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
{
-return ram_list.phys_dirty[addr TARGET_PAGE_BITS] == 0xff;
+unsigned long mask;
+ram_addr_t index = (addr TARGET_PAGE_BITS) / HOST_LONG_BITS;
+int offset = (addr TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+
+mask = 1UL offset;
+return (ram_list.phys_dirty[MASTER_DIRTY_IDX][index] mask) == mask;
+}
+
+static inline void cpu_physical_memory_sync_master(ram_addr_t index)
+{
+if (ram_list.phys_dirty[MASTER_DIRTY_IDX][index]) {
+ram_list.phys_dirty[VGA_DIRTY_IDX][index]
+|= ram_list.phys_dirty[MASTER_DIRTY_IDX][index];
+ram_list.phys_dirty[MIGRATION_DIRTY_IDX][index]
+|= ram_list.phys_dirty[MASTER_DIRTY_IDX][index];
+ram_list.phys_dirty[MASTER_DIRTY_IDX][index] = 0UL;
+}
}
static inline int cpu_physical_memory_get_dirty_flags(ram_addr_t addr)
{
-return ram_list.phys_dirty[addr TARGET_PAGE_BITS];
+ unsigned long mask;
+ ram_addr_t index = (addr TARGET_PAGE_BITS) / HOST_LONG_BITS;
+ int offset = (addr TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+ int ret = 0, i;
+
+ mask = 1UL offset;
+ cpu_physical_memory_sync_master(index);
+
+ for (i = VGA_DIRTY_IDX; i = MIGRATION_DIRTY_IDX; i++) {
+ if (ram_list.phys_dirty[i][index] mask) {
+ ret |= dirty_idx_to_flag(i);
+ }
+ }
+
+ return ret;
+}
+
+static inline int cpu_physical_memory_get_dirty_idx(ram_addr_t addr,
+int dirty_idx)
+{
+unsigned long mask;
+ram_addr_t index = (addr TARGET_PAGE_BITS) / HOST_LONG_BITS;
+int offset = (addr TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+
+mask = 1UL offset;
+cpu_physical_memory_sync_master(index);
+return (ram_list.phys_dirty[dirty_idx][index] mask) == mask;
}
static inline int cpu_physical_memory_get_dirty(ram_addr_t addr,
int dirty_flags)
{
-return ram_list.phys_dirty[addr TARGET_PAGE_BITS] dirty_flags;
+return cpu_physical_memory_get_dirty_idx(addr,
+ dirty_flag_to_idx(dirty_flags));
}
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
{
-ram_list.phys_dirty[addr TARGET_PAGE_BITS] = 0xff;
+unsigned long mask;
+ram_addr_t index = (addr TARGET_PAGE_BITS) / HOST_LONG_BITS;
+int offset = (addr TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+
+mask = 1UL offset;
+ram_list.phys_dirty[MASTER_DIRTY_IDX][index] |= mask;
}
-static inline int cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
-
[Qemu-devel] [PATCH v6 4/4] Use cpu_physical_memory_get_dirty_range() to check multiple dirty pages.
Modifies ram_save_block() and ram_save_remaining() to use
cpu_physical_memory_get_dirty_range() to check multiple dirty and
non-dirty pages at once.
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp
---
arch_init.c | 58 --
1 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index eb5b67c..e0cf400 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -108,32 +108,39 @@ static int ram_save_block(QEMUFile *f)
static ram_addr_t current_addr = 0;
ram_addr_t saved_addr = current_addr;
ram_addr_t addr = 0;
-int bytes_sent = 0;
+ram_addr_t dirty_rams[HOST_LONG_BITS];
+int i, found, bytes_sent = 0;
while (addr ram_list.last_offset) {
-if (cpu_physical_memory_get_dirty(current_addr, MIGRATION_DIRTY_FLAG))
{
+if ((found = cpu_physical_memory_get_dirty_range(
+ current_addr, ram_list.last_offset, dirty_rams,
HOST_LONG_BITS,
+ MIGRATION_DIRTY_FLAG))) {
uint8_t *p;
-cpu_physical_memory_reset_dirty(current_addr,
-current_addr + TARGET_PAGE_SIZE,
-MIGRATION_DIRTY_FLAG);
-
-p = qemu_get_ram_ptr(current_addr);
-
-if (is_dup_page(p, *p)) {
-qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_COMPRESS);
-qemu_put_byte(f, *p);
-bytes_sent = 1;
-} else {
-qemu_put_be64(f, current_addr | RAM_SAVE_FLAG_PAGE);
-qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
-bytes_sent = TARGET_PAGE_SIZE;
+for (i = 0; i found; i++) {
+ram_addr_t page_addr = dirty_rams[i];
+cpu_physical_memory_reset_dirty(page_addr,
+page_addr + TARGET_PAGE_SIZE,
+MIGRATION_DIRTY_FLAG);
+
+p = qemu_get_ram_ptr(page_addr);
+
+if (is_dup_page(p, *p)) {
+qemu_put_be64(f, page_addr | RAM_SAVE_FLAG_COMPRESS);
+qemu_put_byte(f, *p);
+bytes_sent++;
+} else {
+qemu_put_be64(f, page_addr | RAM_SAVE_FLAG_PAGE);
+qemu_put_buffer(f, p, TARGET_PAGE_SIZE);
+bytes_sent += TARGET_PAGE_SIZE;
+}
}
break;
+} else {
+addr += dirty_rams[0];
+current_addr = (saved_addr + addr) % ram_list.last_offset;
}
-addr += TARGET_PAGE_SIZE;
-current_addr = (saved_addr + addr) % ram_list.last_offset;
}
return bytes_sent;
@@ -143,12 +150,19 @@ static uint64_t bytes_transferred;
static ram_addr_t ram_save_remaining(void)
{
-ram_addr_t addr;
+ram_addr_t addr = 0;
ram_addr_t count = 0;
+ram_addr_t dirty_rams[HOST_LONG_BITS];
+int found = 0;
-for (addr = 0; addr ram_list.last_offset; addr += TARGET_PAGE_SIZE) {
-if (cpu_physical_memory_get_dirty(addr, MIGRATION_DIRTY_FLAG)) {
-count++;
+while (addr ram_list.last_offset) {
+if ((found = cpu_physical_memory_get_dirty_range(
+ addr, ram_list.last_offset, dirty_rams,
+ HOST_LONG_BITS, MIGRATION_DIRTY_FLAG))) {
+count += found;
+addr = dirty_rams[found - 1] + TARGET_PAGE_SIZE;
+} else {
+addr += dirty_rams[0];
}
}
--
1.7.0.31.g1df487
[Qemu-devel] [PATCH v6 2/4] Introduce cpu_physical_memory_get_dirty_range().
It checks the first row and puts dirty addr in the array. If the
first row is empty, it skips to the first non-dirty row or the end
addr, and put the length in the first entry of the array.
Signed-off-by: Yoshiaki Tamura tamura.yoshi...@lab.ntt.co.jp
Signed-off-by: OHMURA Kei ohmura@lab.ntt.co.jp
---
cpu-all.h |4 +++
exec.c| 67 +
2 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/cpu-all.h b/cpu-all.h
index fcccf6f..cdcd591 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -1030,6 +1030,10 @@ static inline void
cpu_physical_memory_mask_dirty_range(ram_addr_t start,
}
}
+int cpu_physical_memory_get_dirty_range(ram_addr_t start, ram_addr_t end,
+ram_addr_t *dirty_rams, int length,
+int dirty_flags);
+
void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
int dirty_flags);
void cpu_tlb_update_dirty(CPUState *env);
diff --git a/exec.c b/exec.c
index 24f0f67..f2950c7 100644
--- a/exec.c
+++ b/exec.c
@@ -2018,6 +2018,73 @@ static inline void tlb_reset_dirty_range(CPUTLBEntry
*tlb_entry,
}
}
+/* It checks the first row and puts dirty addrs in the array.
+ If the first row is empty, it skips to the first non-dirty row
+ or the end addr, and put the length in the first entry of the array. */
+int cpu_physical_memory_get_dirty_range(ram_addr_t start, ram_addr_t end,
+ram_addr_t *dirty_rams, int length,
+int dirty_flag)
+{
+unsigned long p = 0, page_number;
+ram_addr_t addr;
+ram_addr_t s_idx = (start TARGET_PAGE_BITS) / HOST_LONG_BITS;
+ram_addr_t e_idx = (end TARGET_PAGE_BITS) / HOST_LONG_BITS;
+int i, j, offset, dirty_idx = dirty_flag_to_idx(dirty_flag);
+
+/* mask bits before the start addr */
+offset = (start TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+cpu_physical_memory_sync_master(s_idx);
+p |= ram_list.phys_dirty[dirty_idx][s_idx] ~((1UL offset) - 1);
+
+if (s_idx == e_idx) {
+/* mask bits after the end addr */
+offset = (end TARGET_PAGE_BITS) (HOST_LONG_BITS - 1);
+p = (1UL offset) - 1;
+}
+
+if (p == 0) {
+/* when the row is empty */
+ram_addr_t skip;
+if (s_idx == e_idx) {
+skip = end;
+} else {
+/* skip empty rows */
+while (s_idx e_idx) {
+s_idx++;
+cpu_physical_memory_sync_master(s_idx);
+
+if (ram_list.phys_dirty[dirty_idx][s_idx] != 0) {
+break;
+}
+}
+skip = (s_idx * HOST_LONG_BITS * TARGET_PAGE_SIZE);
+}
+dirty_rams[0] = skip - start;
+i = 0;
+
+} else if (p == ~0UL) {
+/* when the row is fully dirtied */
+addr = start;
+for (i = 0; i length; i++) {
+dirty_rams[i] = addr;
+addr += TARGET_PAGE_SIZE;
+}
+} else {
+/* when the row is partially dirtied */
+i = 0;
+do {
+j = ffsl(p) - 1;
+p = ~(1UL j);
+page_number = s_idx * HOST_LONG_BITS + j;
+addr = page_number * TARGET_PAGE_SIZE;
+dirty_rams[i] = addr;
+i++;
+} while (p != 0 i length);
+}
+
+return i;
+}
+
/* Note: start and end must be within the same ram block. */
void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
int dirty_flags)
--
1.7.0.31.g1df487
[Qemu-devel] [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
Ok this issue has been fixed in qemu 0.12.4. Just type 'qemu --version' to see what version you have, it is probably outdated. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. Status in QEMU: New Bug description: Using Qemu 0.12.3 and gdb 7.1 on Ubuntu Lucid. I'm trying to debug some bootloader code. Using qemu -s -S to run the bootloader and gdb to connect to qemu, I set the breakpoint at 0x7c00. Then I type continue in gdb. The breakpoint is hit and gdb shows debug information. However qemu apparently continues to execute the code of the bootloader as the text is printed on the screen etc.
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson alex.william...@redhat.com writes: On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote: Alex proposed to disambiguate by adding identified properties of the immediate parent bus and device to the path component. For PCI, these are dev.fn. Likewise for any other bus where devices have unambigous bus address. The driver name carries no information! From user POV, driver names are very handly to address a device intuitively - except for the case you have tones of devices on the same bus that are handled by the same driver. For that case we need to augment the device name with a useful per-bus ID, derived from the bus address where available, otherwise based on instance numbers. This is where I think you're missing a trick. We don't need to augment the name, we just need to allow the bus id to be used instead. For the case of a hot remove, I agree. If the user specifies pci_del pci.0/03.0, that's completely sufficient because we don't care what's in that slot, just remove it. However, I still see some use cases for device names in the path. Take for example: (A): /i440FX-pcihost/pci.0/e1000.05.0 vs (B): /pci.0/05.0 (removing both the root bridge driver name and the device driver name) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. If we attach a pci option rom to the device, create some ram to store the option rom and name the ram block $(PATH)/rom, with (A) we know more about the hierarchy to get to the actual devfn device, and we know the type of device that's in the slot. With (B), there's no robustness. If we migrated using (B), we could be stuffing a pc e1000 option rom into a ppc lsi895, just because it happened to live that the same place and have a ram block named rom. Including driver names increases the uniqueness of the path. Another example; if we have two drivers that create a vmstate with name foo, plug one driver into slot 03.0 on the migration source, the other into slot 03.0 on the migration destination, what happens? It seems likely that the destination will try to load the vmstate from a different driver and fail in wonderful and bizarre ways. If we use (A), each path automatically has it's own namespace. ISA is also a good example even though it doesn't do hotplug. Given this set: /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300 versus this set: /pci.0.01.0/isa.0 /pci.0.01.0/isa.0/0x3f8 /pci.0.01.0/isa.0/0x378 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0/0x300 Which one has devices that are easier to uniquely identify? For other buses, we need to make something up. Note that addressing by bus address rather than name is generally useful, not just in the context of savevm. For instance, I'd appreciate being able to say something like device_del pci.0/04.0. And I prefer device_del [.../]pci.0/e1000. Otherwise you need to dump the bus first before you can identify which device you want to remove. We can allow both. A bus address is sufficient to uniquely identify a device. A bus address (assuming it exists) is sufficient to uniquely identify a device, on a given VM. A bus address only identifies a location when comparing two separate VMs. Identifying a device on a given VM is all a qdev path does. If you want to check two VMs have the same device in the same slot, then you need to compare *devices*, not their names. You propose to encode *one* property of the device in the name, namely its driver. This is far from sufficient. If you tell me you need it anyway for migration, I'll have to take that at face value (I'm not an expert there). But please do not call it qdev path, because it ain't. I see no reason to require the driver name, or to include it in the canonical device address. Migration. Including the driver name extends our ability to uniquely identify a device across separate VMs. It's then up to the vmstate code to figure out whether the device are compatible for migrate state. Migration needs to recreate the same qdev tree on the destination. Driver name is just *one* property of a device. Migration needs to transfer *all* properties. Why encode driver name in the path, but not the rest? Why can't you put the driver name wherever you put the rest? An easy way to get that is to reserve part of the name space for bus addresses. If
Re: [Qemu-devel] Re: [RFC PATCH 0/5] Introduce canonical device hierarchy string
Markus Armbruster arm...@redhat.com writes: Alex Williamson alex.william...@redhat.com writes: On Tue, 2010-06-15 at 10:53 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: [...] virtio-serial doesn't seem to make a DeviceState per port, so I think it can be skipped. Really? Anyway, its port number should do as bus address. Maybe I'm not specifying it correctly. I see a max_nr_ports property, but I don't see that each port is a separate qdev. I see property nr in virtconsole_info and virtserialport_info. I can't see any other virtio-serial devices. Same issue as with ide-drive's unit: should be a bus property. [...]
Re: [Qemu-devel] Re: [RFC PATCH 0/5] Introduce canonical device hierarchy string
Alex Williamson alex.william...@redhat.com writes: On Tue, 2010-06-15 at 10:53 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: On Mon, 2010-06-14 at 09:02 +0200, Gerd Hoffmann wrote: Hi, My premise with this attempt is that we walk the hierarchy and use the names to create the base of the path. As we get to the device, particularly to the parent bus of the device, we need to start looking at properties to ensure uniqueness. You'll need that for every bus along the way down to the device. Create a virtual machine with two lsi scsi host adapters, then attach a disk with scsi id 0 to each. Just the scsi id isn't good enougth to identify the device. You'll need the lsi pci address too. Yep, see below. For now, the only properties I've tagged as path properties are PCI bus addresses and MAC addresses. mac address isn't needed here. You need the property which specifies the bus address. For PCI this obviously is the PCI address. For scsi the scsi id. For ISA you can use the I/O port base. virtio-serial the port number, ... PCI: addr SCSI: scsi-id ISA: serial/parallel = iobase, others?? If there's no iobase (pathological case), require ID. ide-drive: unit Bus name is IDE, but it's clear enough what you mean :) I put ide-drive here because the unit is a property of the device, not the bus. I consider that a (very minor) bug. I2C: address virtio-serial doesn't seem to make a DeviceState per port, so I think it can be skipped. Really? Anyway, its port number should do as bus address. Maybe I'm not specifying it correctly. I see a max_nr_ports property, but I don't see that each port is a separate qdev. I see property nr in virtconsole_info and virtserialport_info. I can't see any other virtio-serial devices. I'm sure I'm still missing some... s390-virtio SSI System I'll need some help coming up with useful properties to key on for these. I had hoped there's only one System bus. USB usb-storage seems to have a useful drive property that lets me distinguish these devices: /i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0/scsi.0/scsi-disk.0 /i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb1/scsi.0/scsi-disk.0 drive But otherwise USB is disappointingly devoid of useful properties at the bus level. Paul suggested physical ports. Doesn't look like we have them, but that should be fixable.
Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit
+static void usb_host_cleanup(struct Notifier* n)
+{
+struct USBHostDevice *s;
+
+QTAILQ_FOREACH(s,hostdevs, next) {
+if (s-fd != -1) {
+ioctl(s-fd, USBDEVFS_RESET);
+}
+}
+}
Well. The point of exit notifiers is that you don't need global
variables for your cleanup work because the notifier function gets
passed in a handle to your state data.
In that specific case the global hostdevs is needed anyway for other
reasons. Nevertheless I don't want usb-linux.c set a bad example, but
provide a good reference implementation for others to look at.
Patch attached (untested).
cheers,
Gerd
From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Wed, 16 Jun 2010 10:29:59 +0200
Subject: [PATCH] usb-host: make sure we release the device.
Call USBDEVFS_RESET ioctl in usb_host_close.
Use exit notifiers to make sure we do it on exit too.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
usb-linux.c | 15 +++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 88273ff..a089fb6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -33,6 +33,7 @@
#include qemu-common.h
#include qemu-timer.h
#include monitor.h
+#include sysemu.h
#include dirent.h
#include sys/ioctl.h
@@ -132,6 +133,7 @@ typedef struct USBHostDevice {
int configuration;
int ninterfaces;
int closing;
+Notifier exit;
struct ctrl_struct ctrl;
struct endp_data endp_table[MAX_ENDPOINTS];
@@ -404,6 +406,7 @@ static void usb_host_handle_destroy(USBDevice *dev)
usb_host_close(s);
QTAILQ_REMOVE(hostdevs, s, next);
+qemu_remove_exit_notifier(s-exit);
}
static int usb_linux_update_endp_table(USBHostDevice *s);
@@ -991,11 +994,21 @@ static int usb_host_close(USBHostDevice *dev)
async_complete(dev);
dev-closing = 0;
usb_device_detach(dev-dev);
+ioctl(s-fd, USBDEVFS_RESET);
close(dev-fd);
dev-fd = -1;
return 0;
}
+static void usb_host_exit_notifier(struct Notifier* n)
+{
+USBHostDevice *s = container_of(n, USBHostDevice, exit);
+
+if (s-fd != -1) {
+ioctl(s-fd, USBDEVFS_RESET);
+}
+}
+
static int usb_host_initfn(USBDevice *dev)
{
USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
@@ -1003,6 +1016,8 @@ static int usb_host_initfn(USBDevice *dev)
dev-auto_attach = 0;
s-fd = -1;
QTAILQ_INSERT_TAIL(hostdevs, s, next);
+s-exit.notify = usb_host_exit_notifier;
+qemu_add_exit_notifier(s-exit);
usb_host_auto_check(NULL);
return 0;
}
--
1.6.5.2
[Qemu-devel] Re: [PATCH 5/5] ide: fix migration in the middle of a bmdma transfer
On 06/15/2010 03:31 PM, Juan Quintela wrote: Signed-off-by: Juan Quintelaquint...@redhat.com Sorry if this has been discussed to death before (if so I must have missed it...). With subsections available, what about taking advantage of the new protocol extension and add to the subsection info about the size of the subsection? Also, with the size information, would it make sense to specify optional subsections that the receiver could choose to ignore? Mandatory subsections are something such that round-trip A-B-A fail unless B understands the subsection, while optional subsections are such that A can provide a default. IDE subsections would be optional, for example. Paolo
[Qemu-devel] Re: [PATCH 2/7] megasas: Update to 1.02
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote: This patchset updates the megasas HBA emulation to v1.02. Fixed issues; - Fixup frame context handling - Endianness fixes - Implement reset function - Improve DCMD bounds checking - Improve dump_frame() output - Sanitize I/O settings - Return correct SCSI status codes Signed-off-by: Hannes Reinecke h...@suse.de --- hw/megasas.c | 155 +++--- hw/mfi.h |1 + 2 files changed, 95 insertions(+), 61 deletions(-) Commited, Thanks! --nab
[Qemu-devel] Re: [PATCH 1/7] scsi-generic: Handle queue full
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote:
The sg driver currently has a hardcoded limit of commands it
can handle simultaneously. When this limit is reached the
driver will return -EDOM. So we need to capture this to
enable proper return values here.
And we shouldn't forget we're using linux-style status codes,
which are shift by one.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/scsi-generic.c | 20 +---
1 files changed, 13 insertions(+), 7 deletions(-)
Commited, thanks Hannes!
--nab
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index 6c58742..af76826 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -96,13 +96,19 @@ static void scsi_command_complete(void *opaque, int ret)
s-senselen = r-io_header.sb_len_wr;
if (ret != 0) {
-scsi_req_print(r-req);
-fprintf(stderr, %s: ret %d (%s)\n, __FUNCTION__,
-ret, strerror(-ret));
-s-senselen = scsi_build_sense(SENSE_CODE(INVALID_FIELD),
-s-sensebuf, SCSI_SENSE_BUF_SIZE, 0);
-s-driver_status = SG_ERR_DRIVER_SENSE;
-r-req.status = CHECK_CONDITION;
+ if (ret == -EDOM) {
+ /* sg driver uses EDOM to signal queue busy */
+ fprintf(stderr, %s: sg queue busy\n, __FUNCTION__);
+ r-req.status = QUEUE_FULL 1;
+ } else {
+ scsi_req_print(r-req);
+ fprintf(stderr, %s: ret %d (%s)\n, __FUNCTION__,
+ ret, strerror(-ret));
+ s-senselen = scsi_build_sense(SENSE_CODE(INVALID_FIELD),
+s-sensebuf, SCSI_SENSE_BUF_SIZE, 0);
+ s-driver_status = SG_ERR_DRIVER_SENSE;
+ r-req.status = CHECK_CONDITION 1;
+ }
} else {
if (s-driver_status SG_ERR_DRIVER_TIMEOUT) {
scsi_req_print(r-req);
[Qemu-devel] Re: [PATCH 5/7] scsi-generic: Codingstyle fixes
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote: Updated indentation to match CodingStyle. Signed-off-by: Hannes Reinecke h...@suse.de --- hw/scsi-generic.c | 45 +++-- 1 files changed, 23 insertions(+), 22 deletions(-) Commited, thanks! --nab
[Qemu-devel] Re: [PATCH 3/7] megasas: Codingstyle fixes
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote: Fixup code to match Codingstyle as suggested by blueswirl. Signed-off-by: Hannes Reinecke h...@suse.de --- hw/megasas.c | 1209 +++--- 1 files changed, 562 insertions(+), 647 deletions(-) Commited. --nab
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. That will not support holes in key space, and our key space is already sparse. Then add a service to obtain a bitmap of supported keys. If that bitmap is empty... Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that? -- Gleb.
[Qemu-devel] Re: [PATCH 6/7] scsi: Update sense handling
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote:
Each driver uses it's own means of storing the sense data, so the
sense code should be stored within the driver-specific structure.
And we should not allow a direct access to it but rather implement
an accessor 'scsi_req_sense' to copy the sense data into a
device-supplied buffer.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/megasas.c | 52 +---
hw/scsi-bus.c |9 +++--
hw/scsi-disk.c| 24
hw/scsi-generic.c | 18 +-
hw/scsi.h |4 +++-
5 files changed, 84 insertions(+), 23 deletions(-)
Makes sense to me, commited.
--nab
diff --git a/hw/megasas.c b/hw/megasas.c
index bc35566..a75872b 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -180,23 +180,46 @@ static void megasas_unmap_sgl(struct megasas_cmd_t *cmd)
/*
* passthrough sense and io sense are at the same offset
*/
-static void megasas_build_sense(struct megasas_cmd_t *cmd, SCSISense sense)
+static int megasas_build_sense(struct megasas_cmd_t *cmd, uint8_t *sense_ptr,
+uint8_t sense_len)
{
-uint8_t *sense_ptr;
-uint8_t sense_len;
target_phys_addr_t pa, pa_hi = 0, pa_lo;
uint16_t flags = le16_to_cpu(cmd-frame-header.flags);
int is_sense64 = (flags MFI_FRAME_SENSE64) ? 1 : 0;
-sense_ptr = qemu_mallocz(cmd-frame-header.sense_len);
-sense_len = scsi_build_sense(SENSE_CODE(INVALID_OPCODE), sense_ptr,
- cmd-frame-header.sense_len, 0);
+if (sense_len cmd-frame-header.sense_len)
+sense_len = cmd-frame-header.sense_len;
+
pa_lo = le32_to_cpu(cmd-frame-pass.sense_addr_lo);
if (is_sense64)
pa_hi = le32_to_cpu(cmd-frame-pass.sense_addr_hi);
pa = ((uint64_t) pa_hi 32) | pa_lo;
cpu_physical_memory_write(pa, sense_ptr, sense_len);
-megasas_frame_set_sense_len(cmd-pa, sense_len);
+cmd-frame-header.sense_len = sense_len;
+return sense_len;
+}
+
+static void megasas_write_sense(struct megasas_cmd_t *cmd, SCSISense sense)
+{
+uint8_t *sense_ptr;
+uint8_t sense_len;
+
+sense_ptr = qemu_mallocz(cmd-frame-header.sense_len);
+sense_len = scsi_build_sense(sense, sense_ptr,
+ cmd-frame-header.sense_len, 0);
+megasas_build_sense(cmd, sense_ptr, sense_len);
+qemu_free(sense_ptr);
+}
+
+static void megasas_copy_sense(struct megasas_cmd_t *cmd)
+{
+uint8_t *sense_ptr;
+uint8_t sense_len;
+
+sense_ptr = qemu_mallocz(cmd-frame-header.sense_len);
+sense_len = scsi_req_sense(cmd-req, sense_ptr,
+ cmd-frame-header.sense_len);
+megasas_build_sense(cmd, sense_ptr, sense_len);
qemu_free(sense_ptr);
}
@@ -1124,8 +1147,8 @@ static int megasas_handle_scsi(MPTState *s, struct
megasas_cmd_t *cmd, int is_lo
mfi_frame_desc[cmd-frame-header.frame_cmd],
cmd-frame-header.target_id, cmd-frame-header.lun_id,
cmd-frame-header.cdb_len);
-megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
-megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION);
+megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
+cmd-frame-header.scsi_status = CHECK_CONDITION;
s-event_count++;
return MFI_STAT_SCSI_DONE_WITH_ERROR;
}
@@ -1176,8 +1199,8 @@ static int megasas_handle_io(MPTState *s, struct
megasas_cmd_t *cmd)
mfi_frame_desc[cmd-frame-header.frame_cmd],
cmd-frame-header.target_id, cmd-frame-header.lun_id,
cmd-frame-header.cdb_len);
-megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
-megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION);
+megasas_write_sense(cmd, SENSE_CODE(INVALID_OPCODE));
+cmd-frame-header.scsi_status = CHECK_CONDITION;
s-event_count++;
return MFI_STAT_SCSI_DONE_WITH_ERROR;
}
@@ -1234,14 +1257,13 @@ static void megasas_command_complete(SCSIRequest *req)
}
} else {
DPRINTF_IO(%s req %p cmd %p lun %p finished with status %x len
%u\n,
- mfi_frame_desc[cmd-frame-header.frame_cmd], req, cmd,
cmd-sdev,
- req-status, (unsigned)req-xferlen);
+ mfi_frame_desc[cmd-frame-header.frame_cmd], req, cmd,
+ cmd-sdev, req-status, (unsigned)req-xferlen);
if (req-status != GOOD) {
cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR;
}
if (req-status == CHECK_CONDITION) {
-megasas_build_sense(cmd, cmd-sdev-sense);
-scsi_dev_clear_sense(cmd-sdev);
+megasas_copy_sense(cmd);
}
megasas_unmap_sgl(cmd);
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index c0e6dd3..b7c5178 100644
---
Re: [Qemu-devel] Re: KVM call minutes for June 15
2010/6/16 Markus Armbruster arm...@redhat.com: Anthony Liguori anth...@codemonkey.ws writes: On 06/15/2010 10:41 AM, Christoph Hellwig wrote: On Tue, Jun 15, 2010 at 08:18:12AM -0700, Chris Wright wrote: KVM/qemu patches - patch rate is high, documentation is low, review is low - patches need to include better descriptions and documentation - will slow down patch writers - will make it easier for patch reviewers What is the qemu patch review policy anyway? We don't really have a coherent policy. Suggestions for improvement are always appreciated. There are no Reviewed-by: included in the actual commits, Reviewed-by/Ack-by's are pretty helpful for me. In terms of including them in commit messages, if there's a strong feeling that that would be helpful then it's something I can look at doing but it also requires a fair bit of manual work during commit. Can't hurt reviewer motivation. Could it be automated? Find replies, extract tags. If you want your acks to be picked up, you better make sure your References header works, and your tags are formatted correctly. How about letting the submitter to include acked-by or reviewed-by manually and repost? It wouldn't make the maintainers busy. Although the traffic would increase, it would show the gratitude from submitter to the reviewer. Thanks, Yoshi [...] -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Qemu-devel] Re: [PATCH 4/7] scsi: Use SAM status codes
On Tue, 2010-06-15 at 17:16 +0200, Hannes Reinecke wrote:
Any SCSI emulation is supposed to return status codes as defined
by SAM, not the linux ones which are shifted by one.
Signed-off-by: Hannes Reinecke h...@suse.de
---
hw/megasas.c | 12 +++-
hw/scsi-bus.c |4 +++-
hw/scsi-defs.h| 20 +++-
hw/scsi-generic.c | 12 ++--
4 files changed, 27 insertions(+), 21 deletions(-)
Wow, good catch. Commited.
--nab
diff --git a/hw/megasas.c b/hw/megasas.c
index 6ddb757..bc35566 100644
--- a/hw/megasas.c
+++ b/hw/megasas.c
@@ -1125,7 +1125,7 @@ static int megasas_handle_scsi(MPTState *s, struct
megasas_cmd_t *cmd, int is_lo
cmd-frame-header.target_id, cmd-frame-header.lun_id,
cmd-frame-header.cdb_len);
megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
-megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION 1);
+megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION);
s-event_count++;
return MFI_STAT_SCSI_DONE_WITH_ERROR;
}
@@ -1177,7 +1177,7 @@ static int megasas_handle_io(MPTState *s, struct
megasas_cmd_t *cmd)
cmd-frame-header.target_id, cmd-frame-header.lun_id,
cmd-frame-header.cdb_len);
megasas_build_sense(cmd, SENSE_CODE(INVALID_OPCODE));
-megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION 1);
+megasas_frame_set_scsi_status(cmd-pa, CHECK_CONDITION);
s-event_count++;
return MFI_STAT_SCSI_DONE_WITH_ERROR;
}
@@ -1236,9 +1236,11 @@ static void megasas_command_complete(SCSIRequest *req)
DPRINTF_IO(%s req %p cmd %p lun %p finished with status %x len
%u\n,
mfi_frame_desc[cmd-frame-header.frame_cmd], req, cmd,
cmd-sdev,
req-status, (unsigned)req-xferlen);
-if (req-status == CHECK_CONDITION 1) {
-megasas_build_sense(cmd, cmd-sdev-sense);
+if (req-status != GOOD) {
cmd_status = MFI_STAT_SCSI_DONE_WITH_ERROR;
+}
+if (req-status == CHECK_CONDITION) {
+megasas_build_sense(cmd, cmd-sdev-sense);
scsi_dev_clear_sense(cmd-sdev);
}
@@ -1302,7 +1304,7 @@ static void megasas_handle_frame(MPTState *s,
target_phys_addr_t frame_addr,
cmd = megasas_enqueue_frame(s, frame_addr);
if (!cmd) {
/* reply queue full */
-megasas_frame_set_scsi_status(frame_addr, BUSY 1);
+megasas_frame_set_scsi_status(frame_addr, BUSY);
frame_status = MFI_STAT_SCSI_DONE_WITH_ERROR;
s-event_count++;
goto frame_done;
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 7d80405..c0e6dd3 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -525,7 +525,9 @@ const char *scsi_status_name(uint8_t status)
[ INTERMEDIATE_C_GOOD ] = INTERMEDIATE_C_GOOD,
[ RESERVATION_CONFLICT ] = RESERVATION_CONFLICT,
[ COMMAND_TERMINATED ] = COMMAND_TERMINATED,
-[ QUEUE_FULL ] = QUEUE_FULL,
+[ TASK_SET_FULL] = TASK_SET_FULL,
+[ ACA_ACTIVE ] = ACA_ACTIVE,
+[ TASK_ABORTED ] = TASK_ABORTED,
};
if (status = ARRAY_SIZE(names) || names[status] == NULL)
diff --git a/hw/scsi-defs.h b/hw/scsi-defs.h
index a4a3518..1473ecb 100644
--- a/hw/scsi-defs.h
+++ b/hw/scsi-defs.h
@@ -111,18 +111,20 @@
#define BLANK 0xa1
/*
- * Status codes
+ * SAM Status codes
*/
#define GOOD 0x00
-#define CHECK_CONDITION 0x01
-#define CONDITION_GOOD 0x02
-#define BUSY 0x04
-#define INTERMEDIATE_GOOD0x08
-#define INTERMEDIATE_C_GOOD 0x0a
-#define RESERVATION_CONFLICT 0x0c
-#define COMMAND_TERMINATED 0x11
-#define QUEUE_FULL 0x14
+#define CHECK_CONDITION 0x02
+#define CONDITION_GOOD 0x04
+#define BUSY 0x08
+#define INTERMEDIATE_GOOD0x10
+#define INTERMEDIATE_C_GOOD 0x14
+#define RESERVATION_CONFLICT 0x18
+#define COMMAND_TERMINATED 0x22
+#define TASK_SET_FULL0x28
+#define ACA_ACTIVE 0x30
+#define TASK_ABORTED 0x40
#define STATUS_MASK 0x3e
diff --git a/hw/scsi-generic.c b/hw/scsi-generic.c
index af76826..33d7e52 100644
--- a/hw/scsi-generic.c
+++ b/hw/scsi-generic.c
@@ -99,7 +99,7 @@ static void scsi_command_complete(void *opaque, int ret)
if (ret == -EDOM) {
/* sg driver uses EDOM to signal queue busy */
fprintf(stderr, %s: sg queue busy\n, __FUNCTION__);
- r-req.status = QUEUE_FULL 1;
+ r-req.status = TASK_SET_FULL;
} else {
scsi_req_print(r-req);
fprintf(stderr, %s: ret %d (%s)\n, __FUNCTION__,
@@ -107,13 +107,13 @@ static void scsi_command_complete(void *opaque, int ret)
s-senselen = scsi_build_sense(SENSE_CODE(INVALID_FIELD),
[Qemu-devel] Re: KVM call minutes for June 15
On 06/15/2010 05:18 PM, Chris Wright wrote: - size for each section would be useful (breaks protocol) - while size is possibly useful, breaks protocol It is not necessary to break the protocol. If you're okay with only having the size information when the migration data has been saved to a file, you can put the directory at the end of the migration data, after the EOF section. Something like QEMU_VM_SECTION_EOF QEMU_VM_SECTION_DIRECTORY copy of the migration data, with the actual data replaced by a single 8-byte pointer to the beginning of the section: QEMU_VM_SECTION_START section id 5 block instance id version id 8-byte pointer QEMU_VM_SECTION_START section id 3 ram instance id version id 8-byte pointer ... QEMU_VM_SECTION_FULL section id 10 cpu_common instance id version id 8-byte pointer ... QEMU_VM_SECTION_EOF 8-byte pointer QEMU_VM_SECTION_DIRECTORY 8-byte pointer Note that by definition the last 8 bytes will point to the beginning of the directory. You can read the last 18 bytes to reduce (to almost zero) the possibility of a false positive. The directory table can be built at save time and streamed after the EOF without causing an error if the receiver closes its connection during the streaming of the directory. Paolo
[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.
On Wed, Jun 16, 2010 at 11:20:02AM +0900, Isaku Yamahata wrote:
On Tue, Jun 15, 2010 at 12:12:07PM +0300, Michael S. Tsirkin wrote:
On Tue, Jun 15, 2010 at 02:06:46PM +0900, Isaku Yamahata wrote:
Don't overwrite pci header type.
Otherwise, multi function bit which pci_init_header_type() sets
appropriately is lost.
Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
which is already zero cleared.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
...
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 31c8d70..cdf3bc2 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -428,7 +428,8 @@ static int pbm_pci_host_init(PCIDevice *d)
PCI_STATUS_DEVSEL_MEDIUM);
pci_config_set_class(d-config, PCI_CLASS_BRIDGE_HOST);
pci_set_byte(d-config + PCI_HEADER_TYPE,
- PCI_HEADER_TYPE_NORMAL);
+ (pci_get_byte(d-config + PCI_HEADER_TYPE)
+ PCI_HEADER_TYPE_MULTI_FUNCTION) |
PCI_HEADER_TYPE_NORMAL);
what is this doing?
It changes the header type to normal device(bit 1-7) without overwriting
multi function bit(bit 8).
Don't we know what the multi function bit value is?
Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
on the other hand pbc_pci_host_init() sets the register
to PCI_HEADER_TYPE_NORMAL.
To be honest I don't know why it does so, but that is what Blue wants.
BTW I think it would be prettier to have is_bridge instead of header_type
as a qdev property. Agree?
So I touch only multi function bit(bit 8) and leave other bit (bit 1-7)
unchanged.
If you don't like this hunk, I'll drop this hunk and leave it to Blue.
What do you think?
Blue Swirl, could you comment on this please?
static PCIDeviceInfo pbm_pci_host_info = {
.qdev.name = pbm,
.qdev.size = sizeof(PCIDevice),
.init = pbm_pci_host_init,
.header_type = PCI_HEADER_TYPE_BRIDGE,Here
};
--
yamahata
[Qemu-devel] Re: [PATCH 1/2] tcg: Optionally sign-extend 32-bit arguments for 64-bit hosts.
On Mon, Jun 14, 2010 at 05:35:27PM -0700, Richard Henderson wrote: Some hosts (amd64, ia64) have an ABI that ignores the high bits of the 64-bit register when passing 32-bit arguments. Others require the value to be properly sign-extended for the type. I.e. int32_t must be sign-extended and uint32_t must be zero-extended to 64-bits. To effect this, extend the sizemask parameter to tcg_gen_callN to include the signedness of the type of each parameter. If the tcg target requires it, extend each 32-bit argument into a 64-bit temp and pass that to the function call. This ABI feature is required by sparc64, ppc64 and s390x. Signed-off-by: Richard Henderson r...@twiddle.net --- def-helper.h | 38 +--- target-i386/ops_sse_header.h |3 + target-ppc/helper.h |1 + tcg/ppc64/tcg-target.h |1 + tcg/s390/tcg-target.h|2 + tcg/sparc/tcg-target.h |4 + tcg/tcg-op.h | 139 +++-- tcg/tcg.c| 41 +++-- 8 files changed, 193 insertions(+), 36 deletions(-) Thanks, applied. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. That will not support holes in key space, and our key space is already sparse. Then add a service to obtain a bitmap of supported keys. If that bitmap is empty... Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that? Isn't there some key that was always there and will always be? Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
malc wrote: On Wed, 16 Jun 2010, Jan Kiszka wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:49 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. do you mean gdb has no command to show the values of these registers? or you mean it doenst have anyway to get notified when these registers are modified? (i dont see how this is useful for debugging, anway) Both: Neither supports gdb them as part of its register set nor does the remote gdb protocol transport them. You need this for segmented addressing, either in real mode (linear address = segment * 16 + offset) or in segmented protected mode (less Not true in general (big real mode), CPU still references hidden segment cache even when protection is enabled. Unfortunately, the BIOS does not start in big real mode e.g... Jan common in modern OSes, but at least still used for per-CPU variables in Linux). And you need a way to detect the current operation mode at all to switch between 16/32, and 64 bit registers (set arch i386 vs. i386:x86-64). You don't need all this for application-level debugging, and that's why gdb lacks it so far. Jan signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, 16 Jun 2010, Jan Kiszka wrote: malc wrote: On Wed, 16 Jun 2010, Jan Kiszka wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:49 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. do you mean gdb has no command to show the values of these registers? or you mean it doenst have anyway to get notified when these registers are modified? (i dont see how this is useful for debugging, anway) Both: Neither supports gdb them as part of its register set nor does the remote gdb protocol transport them. You need this for segmented addressing, either in real mode (linear address = segment * 16 + offset) or in segmented protected mode (less Not true in general (big real mode), CPU still references hidden segment cache even when protection is enabled. ^^^ disabled Unfortunately, the BIOS does not start in big real mode e.g... It's actually fortunate since there's no access whatsoever to the cache (on a real system that is) -- mailto:av1...@comtv.ru
[Qemu-devel] Re: [PATCH] hpet: Clean up initial hpet counter
On Wed, Jun 16, 2010 at 11:33:13AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:57:35AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:51:14AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 09:03:01AM +0200, Jan Kiszka wrote: Gleb Natapov wrote: On Wed, Jun 16, 2010 at 12:40:28AM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@siemens.com There is no need starting with the special value for hpet_cfg.count. Either Seabios is aware of the new firmware interface and properly interprets the counter or it simply ignores it anyway. I want seabios to be able to distinguish between old qemu and new one. I see now. But isn't it a good chance to introduce a proper generic interface for exploring supported fw-cfg keys? Having such interface would be nice. Pity we haven't introduced it from the start. If we do it now seabios will have to find out somehow that qemu support such interface. Chicken and egg ;) That is easy: Add a key the describes the highest supported key value (looks like this is monotonously increasing). Older qemu versions will return 0. That will not support holes in key space, and our key space is already sparse. Then add a service to obtain a bitmap of supported keys. If that bitmap is empty... Bitmap will be 2k long. We can add read capability to control port. To check if key is present you select it (write its value to control port) and then read control port back. If values is non-zero the key is valid. But how to detect qemu that does not support that? Isn't there some key that was always there and will always be? FW_CFG_SIGNATURE -- Gleb.
Re: [Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, 16 Jun 2010, malc wrote: On Wed, 16 Jun 2010, Jan Kiszka wrote: malc wrote: On Wed, 16 Jun 2010, Jan Kiszka wrote: [..snip..] You need this for segmented addressing, either in real mode (linear address = segment * 16 + offset) or in segmented protected mode (less Not true in general (big real mode), CPU still references hidden segment cache even when protection is enabled. ^^^ disabled ^^^ enabled I need to get some proper sleep one of these days.. Sorry for the noise. Unfortunately, the BIOS does not start in big real mode e.g... It's actually fortunate since there's no access whatsoever to the cache (on a real system that is) -- mailto:av1...@comtv.ru
RFC qdev path semantics (was: [Qemu-devel] [RFC PATCH 0/5] Introduce canonical device hierarchy string)
A number of changes to qdev paths have been proposed in various threads. It's becoming harder to keep track of them, so let me sum them up in one place. Please correct me if I misrepresent your ideas. I'm going to describe the current state of things, and the proposed changes (marked with ###). The device tree has the main system bus as root. A child of a bus is a device. A child of a device is a bus. A qdev path consists of qdev path components separated by '/'. It resolves to a node in the device tree, either bus or device. The qdev path / resolves to the root, i.e. the main system bus. The qdev path IDENT, where IDENT is an identifier, resolves to the device whose id is IDENT. If PATH resolves to device DEV, and a child of DEV has the name IDENT, then we resolve to that bus. Bus names are chosen by the system as follows: * If the driver of the parent device model provides a name, use that. * Else, if the parent device has id ID, use ID.NUM, where NUM is the bus number, counting from zero in creation order. * Else, use TYPE.NUM, where TYPE is derived from the bus type, and NUM is the bus number, as above. ### Paul proposes to drop ID.NUM. ### Paul proposes to either drop TYPE.NUM (and require drivers to provide bus names), or make NUM count separately for each bus type. If PATH resolves to bus BUS, then we resolve PATH/IDENT as follows: * If a child of BUS has id IDENT, then we resolve to that device. ### Jan proposes to drop this rule. * Else, if a child of BUS has a driver with name IDENT, then we resolve to that device. If more than one exist, resolve to the first one. This assumes children are ordered. Order is the same as in info qtree. Currently, it's reverse creation order. This is *not* a stable address. * Else, if a child of BUS has a driver with alias name IDENT, then we resolve to that device. If more than one exist, resolve to the first one. This assumes children are ordered. Order is the same as in info qtree. Currently, it's reverse creation order. This is *not* a stable address. ### I propose: we resolve PATH/@BUS-ADDR to the child of BUS with bus address BUS-ADDR, if devices on this type of bus have bus addresses. The format of BUS-ADDR depends on the bus. ### Paul proposes to require all buses to define bus addresses. Make one up if necessary. ### Jan proposes: we resolve PATH/IDENT.NUM as follows: * If a child of BUS has a driver with name IDENT and an instance number NUM, then we resolve to that device. Need a suitable definition of instance number. Jan proposes to number devices with the same driver on the same bus. This assumes children are ordered, see above. This is *not* a stable address if the bus supports hot-plug. I propose to us bus-address as instance number. Works best together with Paul's proposal to define bus addresses. Syntax id...@bus-addr makes more sense then. * Else, same with driver alias name. ### Here's a possible synthesis of the above three proposals: require bus addresses, and permit any of PATH/IDENT PATH/@BUS-ADDR PATH/id...@bus-addr PATH/IDENT can't address instances that don't come first. IDENT in PATH/id...@bus-addr is redundant. Therefore, it can't be the canonical qdev path. That's fine, PATH/@BUS-ADDR serves. If the above rules resolve PATH to a device in a context where we expect a bus, and the device has exactly one bus, resolve it to that bus instead. ### Jan and I propose to drop this rule.
[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.
On Wed, Jun 16, 2010 at 11:54:25AM +0300, Michael S. Tsirkin wrote:
On Wed, Jun 16, 2010 at 11:20:02AM +0900, Isaku Yamahata wrote:
On Tue, Jun 15, 2010 at 12:12:07PM +0300, Michael S. Tsirkin wrote:
On Tue, Jun 15, 2010 at 02:06:46PM +0900, Isaku Yamahata wrote:
Don't overwrite pci header type.
Otherwise, multi function bit which pci_init_header_type() sets
appropriately is lost.
Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
which is already zero cleared.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
...
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 31c8d70..cdf3bc2 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -428,7 +428,8 @@ static int pbm_pci_host_init(PCIDevice *d)
PCI_STATUS_DEVSEL_MEDIUM);
pci_config_set_class(d-config, PCI_CLASS_BRIDGE_HOST);
pci_set_byte(d-config + PCI_HEADER_TYPE,
- PCI_HEADER_TYPE_NORMAL);
+ (pci_get_byte(d-config + PCI_HEADER_TYPE)
+ PCI_HEADER_TYPE_MULTI_FUNCTION) |
PCI_HEADER_TYPE_NORMAL);
what is this doing?
It changes the header type to normal device(bit 1-7) without overwriting
multi function bit(bit 8).
Don't we know what the multi function bit value is?
pci generic initialization, pci_qdev_init(), in pci.c sets (or clears) the bit
and then calls the device specific initialization function, pbm_pci_host_init()
in this case.
So we shouldn't clear the bit unconditionally.
Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
on the other hand pbc_pci_host_init() sets the register
to PCI_HEADER_TYPE_NORMAL.
To be honest I don't know why it does so, but that is what Blue wants.
BTW I think it would be prettier to have is_bridge instead of header_type
as a qdev property. Agree?
The spec version 3.0 defines three header types.
0:normal device, 1:pci-to-pci bridge, 2:card bus bridge
So I'd like the name a bit more generic than is_bridge.
Any suggestion?
So I touch only multi function bit(bit 8) and leave other bit (bit 1-7)
unchanged.
If you don't like this hunk, I'll drop this hunk and leave it to Blue.
What do you think?
Blue Swirl, could you comment on this please?
static PCIDeviceInfo pbm_pci_host_info = {
.qdev.name = pbm,
.qdev.size = sizeof(PCIDevice),
.init = pbm_pci_host_init,
.header_type = PCI_HEADER_TYPE_BRIDGE, Here
};
--
yamahata
--
yamahata
Re: [Qemu-devel] RFC v2: blockdev_add friends, brief rationale, QMP docs
On 06/15/2010 05:54 PM, Markus Armbruster wrote: Avi Kivitya...@redhat.com writes: On 06/15/2010 04:27 PM, Markus Armbruster wrote: I'm only talking about the interface, not the implementation. Internal design details shouldn't be exposed. For the implementation, I imagine you can create an empty blockdev during guest device creation and treat blockdev_add/blockdev_del as media change/eject. If blockdev_del only ejects media, then we need another command to get rid of a blockdev. No. If you have a blockdev just to satisfy the guest device's need for a blockdev pointer, you can delete it automatically when the last reference goes. That's not how netdev and chardev behave. Ok. To me duplicate argument lists suggest a lack of orthogonality, though. How about blockdev_add id=... media_attach blockdev=..., media-parameters media_detach blockdev=... blockdev_del id=... So blockdev_add/del define/remove a slot for the media, media_attach/detach connect it to media. Unless you propose that blockdev_del merely ejects media if the blockdev is being used by a device, but destroys the blockdev outright if not. But that would be sick, wouldn't it? Create a blockdev implicitly with guest device creation, and use blockdev_add (or media_attach) to attach the media. (but that creates a window where the guest device is visible but media is not yet inserted?) Think so, for hot plug. Actually, the device model driver would reject an empty blockdev, unless it's a device with removable media, such as a CD-ROM. Having a device with fixed media go through a no media yet state during initialization just complicates things. Defining the media *before* creating the device is much simpler. That is true. Maybe we should just ignore the duplication. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] Re: [Bug 581353] Re: qemu doesn't stop execution upon hitting a breakpoint
On Wed, Jun 16, 2010 at 11:34:36AM +0200, Jan Kiszka wrote: malc wrote: On Wed, 16 Jun 2010, Jan Kiszka wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:49 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:40 PM, Jan Kiszka jan.kis...@web.de wrote: Jun Koi wrote: On Wed, Jun 16, 2010 at 4:07 PM, Alfredo Mungo chimerane...@gmail.com wrote: Same thing happens to me, same versions as above.. I must turn to another app to accomplish my work while awaiting for a bug-fix, the code is perfectly executed but while gdb hits the breakpoints qemu goes on.. -- qemu doesn't stop execution upon hitting a breakpoint https://bugs.launchpad.net/bugs/581353 You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. i think this bug has been fixed in 0.12.4. have you tried that?? Or this is a well-known gdb deficit: if the bootloader operates in real-mode, you have to set two breakpoints, one at the linear address to make qemu catch it, and another one at the segment offset to avoid gdb skipping the exit due to ip != bp-addr. gdb is still fairly restricted when it comes to system-level debugging, specifically as it lacks support for special x86 registers and the segmented addressing mode. what do you mean by it lacks support for special x86 registers ? idtr, gdtr, ldtr, tr, crX - to name the most important ones. do you mean gdb has no command to show the values of these registers? or you mean it doenst have anyway to get notified when these registers are modified? (i dont see how this is useful for debugging, anway) Both: Neither supports gdb them as part of its register set nor does the remote gdb protocol transport them. You need this for segmented addressing, either in real mode (linear address = segment * 16 + offset) or in segmented protected mode (less Not true in general (big real mode), CPU still references hidden segment cache even when protection is enabled. Unfortunately, the BIOS does not start in big real mode e.g... Actually x86 cpu starts in some strange mode (not exactly big real mode). CS.base == 0x. That is why the first instruction bios does is long jump to init CS. Jan common in modern OSes, but at least still used for per-CPU variables in Linux). And you need a way to detect the current operation mode at all to switch between 16/32, and 64 bit registers (set arch i386 vs. i386:x86-64). You don't need all this for application-level debugging, and that's why gdb lacks it so far. Jan -- Gleb.
Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit
On Wed, Jun 16, 2010 at 10:48:23AM +0200, Gerd Hoffmann wrote:
Date: Wed, 16 Jun 2010 10:48:23 +0200
From: Gerd Hoffmann kra...@redhat.com
To: Shahar Havivi shah...@redhat.com
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] Return usb device to host on exit
+static void usb_host_cleanup(struct Notifier* n)
+{
+struct USBHostDevice *s;
+
+QTAILQ_FOREACH(s,hostdevs, next) {
+if (s-fd != -1) {
+ioctl(s-fd, USBDEVFS_RESET);
+}
+}
+}
Well. The point of exit notifiers is that you don't need global
variables for your cleanup work because the notifier function gets
passed in a handle to your state data.
In that specific case the global hostdevs is needed anyway for other
reasons. Nevertheless I don't want usb-linux.c set a bad example,
but provide a good reference implementation for others to look at.
Patch attached (untested).
cheers,
Gerd
Thanks for the info Gerd,
I will test it.
Shahar.
From 731761de07b73555faf96dc466efd7db2480a694 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Wed, 16 Jun 2010 10:29:59 +0200
Subject: [PATCH] usb-host: make sure we release the device.
Call USBDEVFS_RESET ioctl in usb_host_close.
Use exit notifiers to make sure we do it on exit too.
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
usb-linux.c | 15 +++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 88273ff..a089fb6 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -33,6 +33,7 @@
#include qemu-common.h
#include qemu-timer.h
#include monitor.h
+#include sysemu.h
#include dirent.h
#include sys/ioctl.h
@@ -132,6 +133,7 @@ typedef struct USBHostDevice {
int configuration;
int ninterfaces;
int closing;
+Notifier exit;
struct ctrl_struct ctrl;
struct endp_data endp_table[MAX_ENDPOINTS];
@@ -404,6 +406,7 @@ static void usb_host_handle_destroy(USBDevice *dev)
usb_host_close(s);
QTAILQ_REMOVE(hostdevs, s, next);
+qemu_remove_exit_notifier(s-exit);
}
static int usb_linux_update_endp_table(USBHostDevice *s);
@@ -991,11 +994,21 @@ static int usb_host_close(USBHostDevice *dev)
async_complete(dev);
dev-closing = 0;
usb_device_detach(dev-dev);
+ioctl(s-fd, USBDEVFS_RESET);
close(dev-fd);
dev-fd = -1;
return 0;
}
+static void usb_host_exit_notifier(struct Notifier* n)
+{
+USBHostDevice *s = container_of(n, USBHostDevice, exit);
+
+if (s-fd != -1) {
+ioctl(s-fd, USBDEVFS_RESET);
+}
+}
+
static int usb_host_initfn(USBDevice *dev)
{
USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
@@ -1003,6 +1016,8 @@ static int usb_host_initfn(USBDevice *dev)
dev-auto_attach = 0;
s-fd = -1;
QTAILQ_INSERT_TAIL(hostdevs, s, next);
+s-exit.notify = usb_host_exit_notifier;
+qemu_add_exit_notifier(s-exit);
usb_host_auto_check(NULL);
return 0;
}
--
1.6.5.2
Re: RFC qdev path semantics (was: [Qemu-devel] [RFC PATCH 0/5] Introduce canonical device hierarchy string)
* Else, use TYPE.NUM, where TYPE is derived from the bus type, and NUM is the bus number, as above. ### Paul proposes to either drop TYPE.NUM (and require drivers to provide bus names), or make NUM count separately for each bus type. I revised this proposal: Drop the .NUM part, and require drivers provide a bus name if TYPE would result in ambiguous names. Paul
Re: [Qemu-devel] RFC v2: blockdev_add friends, brief rationale, QMP docs
Avi Kivity a...@redhat.com writes: On 06/15/2010 05:54 PM, Markus Armbruster wrote: Avi Kivitya...@redhat.com writes: On 06/15/2010 04:27 PM, Markus Armbruster wrote: I'm only talking about the interface, not the implementation. Internal design details shouldn't be exposed. For the implementation, I imagine you can create an empty blockdev during guest device creation and treat blockdev_add/blockdev_del as media change/eject. If blockdev_del only ejects media, then we need another command to get rid of a blockdev. No. If you have a blockdev just to satisfy the guest device's need for a blockdev pointer, you can delete it automatically when the last reference goes. That's not how netdev and chardev behave. Ok. To me duplicate argument lists suggest a lack of orthogonality, though. How about blockdev_add id=... media_attach blockdev=..., media-parameters media_detach blockdev=... blockdev_del id=... So blockdev_add/del define/remove a slot for the media, media_attach/detach connect it to media. Yes, those are orthogonal operations. Two more are side effects of device_add and device_del: attach device model, detach device model. My proposal provides two inessential operations purely for convenience: define slot + connect media, and disconnect media + remove slot. These patterns are very common. Unless you propose that blockdev_del merely ejects media if the blockdev is being used by a device, but destroys the blockdev outright if not. But that would be sick, wouldn't it? Create a blockdev implicitly with guest device creation, and use blockdev_add (or media_attach) to attach the media. (but that creates a window where the guest device is visible but media is not yet inserted?) Think so, for hot plug. Actually, the device model driver would reject an empty blockdev, unless it's a device with removable media, such as a CD-ROM. Having a device with fixed media go through a no media yet state during initialization just complicates things. Defining the media *before* creating the device is much simpler. That is true. Maybe we should just ignore the duplication. It's fine to create empty blockdev, insert media, attach device model, even if the device model wants fixed media. Nevertheless, I think my convenience shortcuts make sense.
[Qemu-devel] Re: [PATCH 1/2] qemu-io: retry fgets() when errno is EINTR
Am 15.06.2010 19:53, schrieb MORITA Kazutaka:
posix-aio-compat sends a signal in aio operations, so we should
consider that fgets() could be interrupted here.
Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
---
cmd.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/cmd.c b/cmd.c
index 2336334..460df92 100644
--- a/cmd.c
+++ b/cmd.c
@@ -272,7 +272,10 @@ fetchline(void)
return NULL;
printf(%s, get_prompt());
fflush(stdout);
+again:
if (!fgets(line, MAXREADLINESZ, stdin)) {
+ if (errno == EINTR)
+ goto again;
free(line);
return NULL;
}
This looks like a loop replaced by goto (and braces are missing). What
about this instead?
do {
ret = fgets(...)
} while (ret == NULL errno == EINTR)
if (ret == NULL) {
fail
}
Kevin
Re: [Qemu-devel] RFC v2: blockdev_add friends, brief rationale, QMP docs
On 06/16/2010 02:02 PM, Markus Armbruster wrote: That is true. Maybe we should just ignore the duplication. It's fine to create empty blockdev, insert media, attach device model, even if the device model wants fixed media. Nevertheless, I think my convenience shortcuts make sense. OK. -- error compiling committee.c: too many arguments to function
[Qemu-devel] Re: RFC v2: blockdev_add friends, brief rationale, QMP docs
Am 15.06.2010 15:44, schrieb Avi Kivity:
On 06/10/2010 08:45 PM, Markus Armbruster wrote:
* Our config file format is in INI syntax. QemuOpts correspond to
INI sections. Sections can't be nested, so recursive QemuOpts
don't translate.
git (and probably others) use
[a b]
c = d
for
a.b.c=d
Examples:
* Single protocol:
-blockdev id=blk1,format=raw,protocol=[file,file=fedora.img]
Requires suitable syntactic sugar to get the simple form (*).
* blkdebug
-blockdev id=blk2,format=qcow2,\
protocol=[blkdebug,config=test.blkdebug,\
protocol=[file,file=test.qcow2]]
* Avi's mirror:
-blockdev id=blk3,format=raw,\
protocol=[mirror,\
[file,file=local.img],\
[nbd,domain=unix,sockert=nbd-sock]]
2. We already have a syntax to specify trees, namely JSON, so use it
If -blockdev's argument starts with '{', it's a JSON object suitable
as argument of blockdev_add in QMP.
We still provide ordinary QemuOpts syntax for the cases that can be
expressed with it, i.e. single protocol.
I figure we'd want syntactic sugar for blkdebug, to permit its use
from the command line without having to resort to JSON.
Might be nice as a general extension to QemuOpts.
I agree.
3. Stack protocols through named references
The first protocol is inlined into -blockdev. Any further
protocols need to be referenced by name.
Best explained by example:
* Single protocol:
-blockdev id=blk1,format=raw,protocol=file,file=fedora.img
To get the simple form (*), make protocol optional with a suitable
default.
* blkdebug
-blockdev id=blk2,format=qcow2,protocol=blkdebug,config=test.blkdebug,\
base=blk2-base
-blockproto id=blk2-base,protocol=file,file=test.qcow2
* Avi's mirror:
-blockdev id=blk3,format=raw,protocol=mirror,\
base=blk3-base1,base=blk3=base2
-blockproto id=blk3-base1,protocol=file,file=local.img
-blockproto id=blk3-base2,protocol=nbd,domain=unix,sockert=nbd-sock
Anything but a single protocol becomes pretty verbose. Syntactic
sugar for the blkdebug case would be possible; not sure it's worth
it.
No QemuOpts syntax changes. INI can handle this just fine.
Looks like the least painful option as no new infrastructure is needed.
I'd go with this.
But it's painful to type for the user. After all -blockdev on the
command line is for the user, as tools should use QMP. Also note that
this syntax mixes format and protocol options on one line which I
consider confusing at best.
As I told Markus already in private before he posted this, I prefer the
bracket solution for its clarity and simplicity, even though it comes at
the cost of having additional characters that need to be escaped.
Kevin
[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.
On Wed, Jun 16, 2010 at 06:43:53PM +0900, Isaku Yamahata wrote:
On Wed, Jun 16, 2010 at 11:54:25AM +0300, Michael S. Tsirkin wrote:
On Wed, Jun 16, 2010 at 11:20:02AM +0900, Isaku Yamahata wrote:
On Tue, Jun 15, 2010 at 12:12:07PM +0300, Michael S. Tsirkin wrote:
On Tue, Jun 15, 2010 at 02:06:46PM +0900, Isaku Yamahata wrote:
Don't overwrite pci header type.
Otherwise, multi function bit which pci_init_header_type() sets
appropriately is lost.
Anyway PCI_HEADER_TYPE_NORMAL is zero, so it is unnecessary to zero
which is already zero cleared.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
...
diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index 31c8d70..cdf3bc2 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -428,7 +428,8 @@ static int pbm_pci_host_init(PCIDevice *d)
PCI_STATUS_DEVSEL_MEDIUM);
pci_config_set_class(d-config, PCI_CLASS_BRIDGE_HOST);
pci_set_byte(d-config + PCI_HEADER_TYPE,
- PCI_HEADER_TYPE_NORMAL);
+ (pci_get_byte(d-config + PCI_HEADER_TYPE)
+ PCI_HEADER_TYPE_MULTI_FUNCTION) |
PCI_HEADER_TYPE_NORMAL);
what is this doing?
It changes the header type to normal device(bit 1-7) without overwriting
multi function bit(bit 8).
Don't we know what the multi function bit value is?
pci generic initialization, pci_qdev_init(), in pci.c sets (or clears) the bit
and then calls the device specific initialization function,
pbm_pci_host_init()
in this case.
So we shouldn't clear the bit unconditionally.
Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
on the other hand pbc_pci_host_init() sets the register
to PCI_HEADER_TYPE_NORMAL.
To be honest I don't know why it does so, but that is what Blue wants.
BTW I think it would be prettier to have is_bridge instead of header_type
as a qdev property. Agree?
The spec version 3.0 defines three header types.
0:normal device, 1:pci-to-pci bridge, 2:card bus bridge
So I'd like the name a bit more generic than is_bridge.
Any suggestion?
Could we just have functions that set up header for
each type, such as
pci_init_normal_header()
pci_init_p2p_bridge_header()
pci_init_cardbus_header()
So I touch only multi function bit(bit 8) and leave other bit (bit 1-7)
unchanged.
If you don't like this hunk, I'll drop this hunk and leave it to Blue.
What do you think?
Blue Swirl, could you comment on this please?
static PCIDeviceInfo pbm_pci_host_info = {
.qdev.name = pbm,
.qdev.size = sizeof(PCIDevice),
.init = pbm_pci_host_init,
.header_type = PCI_HEADER_TYPE_BRIDGE,Here
};
--
yamahata
--
yamahata
[Qemu-devel] Re: [PATCH 2/2] qemu-io: check registered fds in command_loop()
Am 15.06.2010 19:53, schrieb MORITA Kazutaka: Some block drivers use an aio handler and do I/O completion routines in it. However, the handler is not invoked if we only do aio_read/write, because registered fds are not checked at all. This patch registers a command processing function as a fd handler to STDIO, and calls qemu_aio_wait() in command_loop(). Any other handlers can be invoked when user input is idle. Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp This patch is much nicer than I would have expected it to be! Thanks, applied to the block branch. Kevin
[Qemu-devel] Re: RFC qdev path semantics
Markus Armbruster wrote: A number of changes to qdev paths have been proposed in various threads. It's becoming harder to keep track of them, so let me sum them up in one place. Please correct me if I misrepresent your ideas. I'm going to describe the current state of things, and the proposed changes (marked with ###). The device tree has the main system bus as root. A child of a bus is a device. A child of a device is a bus. A qdev path consists of qdev path components separated by '/'. It resolves to a node in the device tree, either bus or device. The qdev path / resolves to the root, i.e. the main system bus. Another aspect: A path may start with an arbitrary bus name, not only the system bus. Although this is ambiguous, we need to keep it for addressing the bus itself due to existing client use. But, IMO, we should at least start deprecating this for addressing elements below that bus (e.g. pci.0/e1000). And besides specifying devices via absolute qdev paths, we also support addressing them via their ID if present. I checked if ID/BUS[/...] was supported so far, but it wasn't. So I did not propose this yet although it might be a useful abbreviation. The qdev path IDENT, where IDENT is an identifier, resolves to the device whose id is IDENT. If PATH resolves to device DEV, and a child of DEV has the name IDENT, then we resolve to that bus. Bus names are chosen by the system as follows: * If the driver of the parent device model provides a name, use that. * Else, if the parent device has id ID, use ID.NUM, where NUM is the bus number, counting from zero in creation order. * Else, use TYPE.NUM, where TYPE is derived from the bus type, and NUM is the bus number, as above. ### Paul proposes to drop ID.NUM. ### Paul proposes to either drop TYPE.NUM (and require drivers to provide bus names), or make NUM count separately for each bus type. If PATH resolves to bus BUS, then we resolve PATH/IDENT as follows: * If a child of BUS has id IDENT, then we resolve to that device. ### Jan proposes to drop this rule. * Else, if a child of BUS has a driver with name IDENT, then we resolve to that device. If more than one exist, resolve to the first one. This assumes children are ordered. Order is the same as in info qtree. Currently, it's reverse creation order. This is *not* a stable address. * Else, if a child of BUS has a driver with alias name IDENT, then we resolve to that device. If more than one exist, resolve to the first one. This assumes children are ordered. Order is the same as in info qtree. Currently, it's reverse creation order. This is *not* a stable address. ### I propose: we resolve PATH/@BUS-ADDR to the child of BUS with bus address BUS-ADDR, if devices on this type of bus have bus addresses. The format of BUS-ADDR depends on the bus. ### Paul proposes to require all buses to define bus addresses. Make one up if necessary. ### Jan proposes: we resolve PATH/IDENT.NUM as follows: * If a child of BUS has a driver with name IDENT and an instance number NUM, then we resolve to that device. Need a suitable definition of instance number. Jan proposes to number devices with the same driver on the same bus. This assumes children are ordered, see above. This is *not* a stable address if the bus supports hot-plug. I propose to us bus-address as instance number. Works best together with Paul's proposal to define bus addresses. Syntax id...@bus-addr makes more sense then. I would be fine with this scheme, but I assume we still need instance numbers as fallback for buses without any usable addressing. Example: I have a patch queued that uses this for internal addressing of all hpet devices on the system bus (to connect them to the ISA IRQs). * Else, same with driver alias name. ### Here's a possible synthesis of the above three proposals: require bus addresses, and permit any of PATH/IDENT PATH/@BUS-ADDR PATH/id...@bus-addr PATH/IDENT can't address instances that don't come first. PATH/IDENT[.INSTANCE] would resolve the addressability. IDENT in PATH/id...@bus-addr is redundant. Therefore, it can't be the canonical qdev path. That's fine, PATH/@BUS-ADDR serves. If the above rules resolve PATH to a device in a context where we expect a bus, and the device has exactly one bus, resolve it to that bus instead. ### Jan and I propose to drop this rule. Thanks for this summary! Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.
On Wed, Jun 16, 2010 at 02:19:44PM +0300, Michael S. Tsirkin wrote: Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo, on the other hand pbc_pci_host_init() sets the register to PCI_HEADER_TYPE_NORMAL. To be honest I don't know why it does so, but that is what Blue wants. BTW I think it would be prettier to have is_bridge instead of header_type as a qdev property. Agree? The spec version 3.0 defines three header types. 0:normal device, 1:pci-to-pci bridge, 2:card bus bridge So I'd like the name a bit more generic than is_bridge. Any suggestion? Could we just have functions that set up header for each type, such as pci_init_normal_header() pci_init_p2p_bridge_header() pci_init_cardbus_header() I see. You mean device specific initialization function should call one of them. Then header_type property will be dropped. I'll split pci p2p bridge related functions into a file at first. Then introduce helper functions. -- yamahata
Antwort: Re: [Qemu-devel] [Bug 319014] Re: serial usb-device can't be passed-through to a guest
Hi David,i've tried your suggestion and now it works.I'am sure, it didn't work last time i've tried that way. But now,I'am fine.Thanks.NicoP.-"David S. Ahern" daah...@cisco.com schrieb: -An: Nico Prenzel nico.pren...@pn-systeme.deVon: "David S. Ahern" daah...@cisco.comDatum: 03.06.2010 19:23Kopie: qemu-devel@nongnu.orgBetreff: Re: [Qemu-devel] [Bug 319014] Re: serial usb-device can't be passed-through to a guestOn 06/03/10 09:41, Nico Prenzel wrote: Hello Anthony, which qemu-kvm version do you expect to work with serial usb devices? After you've changed this ticket status and I've checked it again with qemu-kvm version 0.12.4. The reported error message is gone away and the device is present in the guest. The cdc_acm driver loads too without any visible problems, but the device is still unusable. The device data led flash if it gets un/initialized by minicom. But if I try to make a test dial, nothing happens after I call a phone number. To be more precise: -host dosn't have loaded the cdc_acm driver -guest loads the cdc_acm driver -a dial with minicom to the same phone number works on the host, if I load the cdc_acm within the host I would like to get this fixed, as I really want to use this usb-device as my dusty fax again. One option I resorted to is using Qemu's emulated USB serial device andconnecting it to the USB serial device host side. The stack then looks like:.-.| VM || || /dev/ttyUSB0 ||-|| Qemu - serial device|'-' |.-.| Host: /dev/ttyUSB0 |'-' |.-.| USB serial port |-- character stream --'-'A hack, but it works. :-)David
[Qemu-devel] Re: RFC qdev path semantics
Markus Armbruster wrote: A number of changes to qdev paths have been proposed in various threads. It's becoming harder to keep track of them, so let me sum them up in one place. Please correct me if I misrepresent your ideas. I'm going to describe the current state of things, and the proposed changes (marked with ###). The device tree has the main system bus as root. A child of a bus is a device. A child of a device is a bus. A qdev path consists of qdev path components separated by '/'. It resolves to a node in the device tree, either bus or device. The qdev path / resolves to the root, i.e. the main system bus. Another aspect: A path may start with an arbitrary bus name, not only the system bus. Although this is ambiguous, we need to keep it for addressing the bus itself due to existing client use. But, IMO, we should at least start deprecating this for addressing elements below that bus (e.g. pci.0/e1000). I think this would be better served by adding explicit aliases/IDs for those use-cases. i.e. define the global ID pci.0 to be an alias for /i440FX-pcihost/pci Paul
Re: [Qemu-devel] Re: KVM call minutes for June 15
On Wednesday 16 June 2010, Markus Armbruster wrote: Can't hurt reviewer motivation. Could it be automated? Find replies, extract tags. If you want your acks to be picked up, you better make sure your References header works, and your tags are formatted correctly. I think pwclient (https://patchwork.kernel.org/) can do this for you. Arnd
[Qemu-devel] [PATCH V4 1/3] qemu: Add qemu-barrier support to qemu-thread framework.
Signed-off-by: Gautham R Shenoy e...@in.ibm.com
---
qemu-thread.c | 23 +++
qemu-thread.h |9 +
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/qemu-thread.c b/qemu-thread.c
index 3923db7..7c445b6 100644
--- a/qemu-thread.c
+++ b/qemu-thread.c
@@ -161,3 +161,26 @@ int qemu_thread_equal(QemuThread *thread1, QemuThread
*thread2)
return pthread_equal(thread1-thread, thread2-thread);
}
+void qemu_barrier_init(QemuBarrier *barr1, int nr_threads)
+{
+int err;
+
+err = pthread_barrier_init(barr1-barr, NULL, nr_threads);
+
+if (err) {
+error_exit(err, __func__);
+}
+}
+
+int qemu_barrier_wait(QemuBarrier *barr1)
+{
+int ret;
+
+ret = pthread_barrier_wait(barr1-barr);
+
+if (ret != 0 ret != PTHREAD_BARRIER_SERIAL_THREAD) {
+error_exit(ret, __func__);
+}
+
+return ret;
+}
diff --git a/qemu-thread.h b/qemu-thread.h
index 5ef4a3a..b3d36e0 100644
--- a/qemu-thread.h
+++ b/qemu-thread.h
@@ -15,9 +15,14 @@ struct QemuThread {
pthread_t thread;
};
+struct QemuBarrier {
+pthread_barrier_t barr;
+};
+
typedef struct QemuMutex QemuMutex;
typedef struct QemuCond QemuCond;
typedef struct QemuThread QemuThread;
+typedef struct QemuBarrier QemuBarrier;
void qemu_mutex_init(QemuMutex *mutex);
void qemu_mutex_lock(QemuMutex *mutex);
@@ -31,10 +36,14 @@ void qemu_cond_broadcast(QemuCond *cond);
void qemu_cond_wait(QemuCond *cond, QemuMutex *mutex);
int qemu_cond_timedwait(QemuCond *cond, QemuMutex *mutex, uint64_t msecs);
+void qemu_barrier_init(QemuBarrier *barr, int nr_threads);
+
void qemu_thread_create(QemuThread *thread,
void *(*start_routine)(void*),
void *arg);
void qemu_thread_signal(QemuThread *thread, int sig);
void qemu_thread_self(QemuThread *thread);
int qemu_thread_equal(QemuThread *thread1, QemuThread *thread2);
+
+int qemu_barrier_wait(QemuBarrier *barr);
#endif
[Qemu-devel] [PATCH V4 0/3] qemu: Threadlets: A generic task offloading framework
Hi, This is the v4 of the patch-series to have a generic asynchronous task offloading framework (called threadlets) within qemu. V3 can be found here: http://lists.gnu.org/archive/html/qemu-devel/2010-06/index.html Changes from V3: = - Did away with the qemu-thread wrappers for handling pthread_attr_t type following review comments for V3. - Added qemu-thread wrappers for pthread_barrier_init() and pthread_barrier_wait(). - Added a flush_threadlet_queue() helper which allows the caller to wait till all the queued tasks have finished processing. - Added a global queue that can be used by most subsystems to offload tasks. The flexibility to allow individual subsystems to create their private queue with associated thread-pool has been retained. - Fixed the Copyrights in the newly introduced file to reflect the copyrights of the borrowed code. - Renamed the helper functions to reflect their use. Added comments for each of the helpers. - Typedef'd the structs to adhere to the qemu coding style. Description = This patch series decouples the asynchronous threading framework implementation from posix-aio-compat.c to implement a generic asynchronous task offloading threading framework called threadlets which can be used by other subsystems within QEMU. Currently within QEMU, the AIO subsystem (paio) creates a bunch of asynchronous threads to offload any blocking operations so that the vcpu threads and the IO thread can go back to servicing any other guest requests. This offloading framework can be used by subsystems such as virtio-9p, Asynchronous encoding for vnc-server, so that the vcpu thread can offload blocking operations on to the asynchronous threads and resume servicing any other guest requests. The asynchronous threads, after finishing the blocking operations can then transfer the control over to the IO thread so that the latter can handle the post_blocking_operation(). The patch series passed fsstress test without any issues. Could it be considered for inclusion ? --- Aneesh Kumar K.V (1): qemu: Generic task offloading framework: threadlets Gautham R Shenoy (2): qemu: Add qemu-barrier support to qemu-thread framework. qemu: Convert AIO code to use threadlets. Makefile.objs |3 + async-work.c | 186 async-work.h | 69 +++ posix-aio-compat.c | 152 +++--- qemu-thread.c | 23 ++ qemu-thread.h |9 +++ 6 files changed, 313 insertions(+), 129 deletions(-) create mode 100644 async-work.c create mode 100644 async-work.h -- Thanks and Regards gautham.
[Qemu-devel] [PATCH V4 2/3] qemu: Generic task offloading framework: threadlets
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
This patch creates a generic asynchronous-task-offloading infrastructure named
threadlets. The core idea has been borrowed from the threading framework that
is being used by paio.
The reason for creating this generic infrastructure is so that other subsystems,
such as virtio-9p could make use of it for offloading tasks that could block.
The patch creates a global queue on-to which subsystems can queue their tasks to
be executed asynchronously.
The patch also provides API's that allow a subsystem to create a private queue.
API's that allow a subsystem to wait till all the earlier queued tasks have been
executed, is also provided.
[...@in.ibm.com: Facelift of the code, cancel_threadlet,
flush_threadlet_queue and other minor helpers.]
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Gautham R Shenoy e...@in.ibm.com
---
Makefile.objs |3 +
async-work.c | 186 +
async-work.h | 69 +
3 files changed, 257 insertions(+), 1 deletions(-)
create mode 100644 async-work.c
create mode 100644 async-work.h
diff --git a/Makefile.objs b/Makefile.objs
index 1a942e5..019646f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -9,6 +9,8 @@ qobject-obj-y += qerror.o
block-obj-y = cutils.o cache-utils.o qemu-malloc.o qemu-option.o module.o
block-obj-y += nbd.o block.o aio.o aes.o osdep.o qemu-config.o
+block-obj-y += qemu-thread.o
+block-obj-y += async-work.o
block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -109,7 +111,6 @@ common-obj-y += iov.o
common-obj-$(CONFIG_VNC_TLS) += vnc-tls.o vnc-auth-vencrypt.o
common-obj-$(CONFIG_VNC_SASL) += vnc-auth-sasl.o
common-obj-$(CONFIG_COCOA) += cocoa.o
-common-obj-$(CONFIG_IOTHREAD) += qemu-thread.o
common-obj-y += notify.o event_notifier.o
common-obj-y += qemu-timer.o
diff --git a/async-work.c b/async-work.c
new file mode 100644
index 000..50e39ce
--- /dev/null
+++ b/async-work.c
@@ -0,0 +1,186 @@
+/*
+ * Threadlet support for offloading tasks to be executed asynchronously
+ * Generalization based on posix-aio emulation code.
+ *
+ * Copyright IBM, Corp. 2008
+ * Copyright IBM, Corp. 2010
+ *
+ * Authors:
+ * Anthony Liguori aligu...@us.ibm.com
+ * Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
+ * Gautham R Shenoy e...@in.ibm.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include stdio.h
+#include errno.h
+#include string.h
+#include stdlib.h
+#include signal.h
+#include async-work.h
+#include osdep.h
+
+#define MAX_GLOBAL_THREADS 64
+#define MIN_GLOBAL_THREADS 64
+ThreadletQueue globalqueue;
+static int globalqueue_init;
+
+static void *threadlet_worker(void *data)
+{
+ThreadletQueue *queue = data;
+
+while (1) {
+ThreadletWork *work;
+int ret = 0;
+qemu_mutex_lock((queue-lock));
+
+while (QTAILQ_EMPTY((queue-request_list))
+ (ret != ETIMEDOUT)) {
+ret = qemu_cond_timedwait((queue-cond),
+(queue-lock), 10*10);
+}
+
+if (QTAILQ_EMPTY((queue-request_list)))
+goto check_exit;
+
+work = QTAILQ_FIRST((queue-request_list));
+QTAILQ_REMOVE((queue-request_list), work, node);
+queue-idle_threads--;
+qemu_mutex_unlock((queue-lock));
+
+/* execute the work function */
+work-func(work);
+
+qemu_mutex_lock((queue-lock));
+queue-idle_threads++;
+
+check_exit:
+if (queue-exit || ((queue-idle_threads 0)
+(queue-cur_threads queue-min_threads))) {
+/* We exit the queue or we retain minimum number of threads */
+break;
+}
+qemu_mutex_unlock((queue-lock));
+}
+
+queue-idle_threads--;
+queue-cur_threads--;
+if (queue-exit) {
+qemu_mutex_unlock((queue-lock));
+qemu_barrier_wait(queue-barr);
+} else
+qemu_mutex_unlock(queue-lock);
+
+return NULL;
+}
+
+static void spawn_threadlet(ThreadletQueue *queue)
+{
+QemuThread thread;
+
+queue-cur_threads++;
+queue-idle_threads++;
+
+qemu_thread_create(thread, threadlet_worker, queue);
+}
+
+/**
+ * threadlet_submit: Submit a new task to be executed asynchronously.
+ * @queue: Queue to which the new task needs to be submitted.
+ * @work: Contains information about the task that needs to be submitted.
+ */
+void threadlet_submit(ThreadletQueue *queue, ThreadletWork *work)
+{
+qemu_mutex_lock((queue-lock));
+if (queue-idle_threads == 0 queue-cur_threads queue-max_threads) {
+spawn_threadlet(queue);
+}
+QTAILQ_INSERT_TAIL((queue-request_list), work, node);
+qemu_mutex_unlock((queue-lock));
+qemu_cond_signal((queue-cond));
+}
+
+/**
+ * threadlet_submit_common: Submit to the
[Qemu-devel] [PATCH V4 3/3] qemu: Convert AIO code to use threadlets.
This patch makes the paio subsystem use the threadlet framework thereby
decoupling asynchronous threading framework portion out of posix-aio-compat.c
The patch has been tested with fstress.
Signed-off-by: Gautham R Shenoy e...@in.ibm.com
---
posix-aio-compat.c | 152
1 files changed, 24 insertions(+), 128 deletions(-)
diff --git a/posix-aio-compat.c b/posix-aio-compat.c
index b43c531..f9307fb 100644
--- a/posix-aio-compat.c
+++ b/posix-aio-compat.c
@@ -28,6 +28,7 @@
#include block_int.h
#include block/raw-posix-aio.h
+#include async-work.h
struct qemu_paiocb {
@@ -50,6 +51,7 @@ struct qemu_paiocb {
struct qemu_paiocb *next;
int async_context_id;
+ThreadletWork work;
};
typedef struct PosixAioState {
@@ -57,16 +59,6 @@ typedef struct PosixAioState {
struct qemu_paiocb *first_aio;
} PosixAioState;
-
-static pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
-static pthread_t thread_id;
-static pthread_attr_t attr;
-static int max_threads = 64;
-static int cur_threads = 0;
-static int idle_threads = 0;
-static QTAILQ_HEAD(, qemu_paiocb) request_list;
-
#ifdef CONFIG_PREADV
static int preadv_present = 1;
#else
@@ -84,39 +76,6 @@ static void die(const char *what)
die2(errno, what);
}
-static void mutex_lock(pthread_mutex_t *mutex)
-{
-int ret = pthread_mutex_lock(mutex);
-if (ret) die2(ret, pthread_mutex_lock);
-}
-
-static void mutex_unlock(pthread_mutex_t *mutex)
-{
-int ret = pthread_mutex_unlock(mutex);
-if (ret) die2(ret, pthread_mutex_unlock);
-}
-
-static int cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
- struct timespec *ts)
-{
-int ret = pthread_cond_timedwait(cond, mutex, ts);
-if (ret ret != ETIMEDOUT) die2(ret, pthread_cond_timedwait);
-return ret;
-}
-
-static void cond_signal(pthread_cond_t *cond)
-{
-int ret = pthread_cond_signal(cond);
-if (ret) die2(ret, pthread_cond_signal);
-}
-
-static void thread_create(pthread_t *thread, pthread_attr_t *attr,
- void *(*start_routine)(void*), void *arg)
-{
-int ret = pthread_create(thread, attr, start_routine, arg);
-if (ret) die2(ret, pthread_create);
-}
-
static ssize_t handle_aiocb_ioctl(struct qemu_paiocb *aiocb)
{
int ret;
@@ -300,47 +259,27 @@ static ssize_t handle_aiocb_rw(struct qemu_paiocb *aiocb)
return nbytes;
}
-static void *aio_thread(void *unused)
+static void aio_thread(ThreadletWork *work)
{
-pid_t pid;
-
-pid = getpid();
-
-while (1) {
-struct qemu_paiocb *aiocb;
-ssize_t ret = 0;
-qemu_timeval tv;
-struct timespec ts;
-
-qemu_gettimeofday(tv);
-ts.tv_sec = tv.tv_sec + 10;
-ts.tv_nsec = 0;
-mutex_lock(lock);
+pid_t pid;
-while (QTAILQ_EMPTY(request_list)
- !(ret == ETIMEDOUT)) {
-ret = cond_timedwait(cond, lock, ts);
-}
+struct qemu_paiocb *aiocb = container_of(work, struct qemu_paiocb, work);
+ssize_t ret = 0;
-if (QTAILQ_EMPTY(request_list))
-break;
+pid = getpid();
-aiocb = QTAILQ_FIRST(request_list);
-QTAILQ_REMOVE(request_list, aiocb, node);
-aiocb-active = 1;
-idle_threads--;
-mutex_unlock(lock);
+aiocb-active = 1;
-switch (aiocb-aio_type QEMU_AIO_TYPE_MASK) {
-case QEMU_AIO_READ:
-case QEMU_AIO_WRITE:
+switch (aiocb-aio_type QEMU_AIO_TYPE_MASK) {
+case QEMU_AIO_READ:
+case QEMU_AIO_WRITE:
ret = handle_aiocb_rw(aiocb);
break;
-case QEMU_AIO_FLUSH:
-ret = handle_aiocb_flush(aiocb);
-break;
-case QEMU_AIO_IOCTL:
+case QEMU_AIO_FLUSH:
+ret = handle_aiocb_flush(aiocb);
+break;
+case QEMU_AIO_IOCTL:
ret = handle_aiocb_ioctl(aiocb);
break;
default:
@@ -349,57 +288,25 @@ static void *aio_thread(void *unused)
break;
}
-mutex_lock(lock);
-aiocb-ret = ret;
-idle_threads++;
-mutex_unlock(lock);
+aiocb-ret = ret;
-if (kill(pid, aiocb-ev_signo)) die(kill failed);
-}
-
-idle_threads--;
-cur_threads--;
-mutex_unlock(lock);
-
-return NULL;
-}
-
-static void spawn_thread(void)
-{
-sigset_t set, oldset;
-
-cur_threads++;
-idle_threads++;
-
-/* block all signals */
-if (sigfillset(set)) die(sigfillset);
-if (sigprocmask(SIG_SETMASK, set, oldset)) die(sigprocmask);
-
-thread_create(thread_id, attr, aio_thread, NULL);
-
-if (sigprocmask(SIG_SETMASK, oldset, NULL)) die(sigprocmask restore);
+if (kill(pid, aiocb-ev_signo)) die(kill failed);
}
static void qemu_paio_submit(struct qemu_paiocb *aiocb)
{
aiocb-ret = -EINPROGRESS;
aiocb-active = 0;
-
[Qemu-devel] Re: RFC qdev path semantics
Paul Brook wrote: Markus Armbruster wrote: A number of changes to qdev paths have been proposed in various threads. It's becoming harder to keep track of them, so let me sum them up in one place. Please correct me if I misrepresent your ideas. I'm going to describe the current state of things, and the proposed changes (marked with ###). The device tree has the main system bus as root. A child of a bus is a device. A child of a device is a bus. A qdev path consists of qdev path components separated by '/'. It resolves to a node in the device tree, either bus or device. The qdev path / resolves to the root, i.e. the main system bus. Another aspect: A path may start with an arbitrary bus name, not only the system bus. Although this is ambiguous, we need to keep it for addressing the bus itself due to existing client use. But, IMO, we should at least start deprecating this for addressing elements below that bus (e.g. pci.0/e1000). I think this would be better served by adding explicit aliases/IDs for those use-cases. i.e. define the global ID pci.0 to be an alias for /i440FX-pcihost/pci Makes sense. We could attach this ID to the BusState (corresponding to DeviceState:id) and manually set it during machine init. qbus_find_recursive would then look for a matching ID instead of a name. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
[Qemu-devel] [PATCH,APPLIED] Strace mprotect flags.
Teach strace code about linux specific mprotect flags.
Signed-off-by: Paul Brook p...@codesourcery.com
---
linux-user/strace.c |3 +++
linux-user/syscall_defs.h |6 ++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index d77053b..bf9a0d9 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -385,6 +385,9 @@ UNUSED static struct flags mmap_prot_flags[] = {
FLAG_GENERIC(PROT_EXEC),
FLAG_GENERIC(PROT_READ),
FLAG_GENERIC(PROT_WRITE),
+FLAG_TARGET(PROT_SEM),
+FLAG_GENERIC(PROT_GROWSDOWN),
+FLAG_GENERIC(PROT_GROWSUP),
FLAG_END,
};
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 681021c..46cb05e 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -946,6 +946,12 @@ struct target_winsize {
#include termbits.h
+#if defined(TARGET_MIPS)
+#define TARGET_PROT_SEM 0x10
+#else
+#define TARGET_PROT_SEM 0x08
+#endif
+
/* Common */
#define TARGET_MAP_SHARED 0x01/* Share changes */
#define TARGET_MAP_PRIVATE 0x02/* Changes are private */
--
1.7.1
[Qemu-devel] [PATCH,APPLIED] GDB exit status for semihosting
Report exit status to GDB when a semihosted application exits.
Signed-off-by: Paul Brook p...@codesourcery.com
---
arm-semi.c |1 +
gdbstub.c | 34 --
gdbstub.h |2 +-
m68k-semi.c |1 +
4 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/arm-semi.c b/arm-semi.c
index 5239ffc..0687b03 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -459,6 +459,7 @@ uint32_t do_arm_semihosting(CPUState *env)
return 0;
}
case SYS_EXIT:
+gdb_exit(env, 0);
exit(0);
default:
fprintf(stderr, qemu: Unsupported SemiHosting SWI 0x%02x\n, nr);
diff --git a/gdbstub.c b/gdbstub.c
index 474ed8a..c1852c2 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -2373,6 +2373,26 @@ static void gdb_read_byte(GDBState *s, int ch)
}
}
+/* Tell the remote gdb that the process has exited. */
+void gdb_exit(CPUState *env, int code)
+{
+ GDBState *s;
+ char buf[4];
+
+ s = gdbserver_state;
+ if (!s) {
+ return;
+ }
+#ifdef CONFIG_USER_ONLY
+ if (gdbserver_fd 0 || s-fd 0) {
+ return;
+ }
+#endif
+
+ snprintf(buf, sizeof(buf), W%02x, (uint8_t)code);
+ put_packet(s, buf);
+}
+
#ifdef CONFIG_USER_ONLY
int
gdb_queuesig (void)
@@ -2436,20 +2456,6 @@ gdb_handlesig (CPUState *env, int sig)
return sig;
}
-/* Tell the remote gdb that the process has exited. */
-void gdb_exit(CPUState *env, int code)
-{
- GDBState *s;
- char buf[4];
-
- s = gdbserver_state;
- if (gdbserver_fd 0 || s-fd 0)
-return;
-
- snprintf(buf, sizeof(buf), W%02x, code);
- put_packet(s, buf);
-}
-
/* Tell the remote gdb that the process has exited due to SIG. */
void gdb_signalled(CPUState *env, int sig)
{
diff --git a/gdbstub.h b/gdbstub.h
index 54d753c..219abda 100644
--- a/gdbstub.h
+++ b/gdbstub.h
@@ -17,10 +17,10 @@ typedef void (*gdb_syscall_complete_cb)(CPUState *env,
void gdb_do_syscall(gdb_syscall_complete_cb cb, const char *fmt, ...);
int use_gdb_syscalls(void);
void gdb_set_stop_cpu(CPUState *env);
+void gdb_exit(CPUState *, int);
#ifdef CONFIG_USER_ONLY
int gdb_queuesig (void);
int gdb_handlesig (CPUState *, int);
-void gdb_exit(CPUState *, int);
void gdb_signalled(CPUState *, int);
void gdbserver_fork(CPUState *);
#endif
diff --git a/m68k-semi.c b/m68k-semi.c
index 48e3bd3..d16bc67 100644
--- a/m68k-semi.c
+++ b/m68k-semi.c
@@ -172,6 +172,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
args = env-dregs[1];
switch (nr) {
case HOSTED_EXIT:
+gdb_exit(env, env-dregs[0]);
exit(env-dregs[0]);
case HOSTED_OPEN:
if (use_gdb_syscalls()) {
--
1.7.1
[Qemu-devel] [PATCH,APPLIED] Usermode exec-stack fix
When loading a shared library that requires an executable stack,
glibc uses the mprotext PROT_GROWSDOWN flag to achieve this.
We don't support PROT_GROWSDOWN.
Add a special case to handle changing the stack permissions in this way.
Signed-off-by: Paul Brook p...@codesourcery.com
---
linux-user/elfload.c |1 +
linux-user/flatload.c |1 +
linux-user/qemu.h |1 +
linux-user/syscall.c | 11 +++
4 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2d920f2..accb44d 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1018,6 +1018,7 @@ static abi_ulong setup_arg_pages(abi_ulong p, struct
linux_binprm *bprm,
/* we reserve one extra page at the top of the stack as guard */
target_mprotect(error + size, qemu_host_page_size, PROT_NONE);
+info-stack_limit = error;
stack_base = error + size - MAX_ARG_PAGES*TARGET_PAGE_SIZE;
p += stack_base;
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 914de1f..8ad130a 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -802,6 +802,7 @@ int load_flt_binary(struct linux_binprm * bprm, struct
target_pt_regs * regs,
info-end_data = libinfo[0].end_data;
info-start_brk = libinfo[0].start_brk;
info-start_stack = sp;
+info-stack_limit = libinfo[0].start_brk;
info-entry = start_addr;
info-code_offset = info-start_code;
info-data_offset = info-start_data - libinfo[0].text_len;
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index dab3597..1878d5a 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -42,6 +42,7 @@ struct image_info {
abi_ulong mmap;
abi_ulong rss;
abi_ulong start_stack;
+abi_ulong stack_limit;
abi_ulong entry;
abi_ulong code_offset;
abi_ulong data_offset;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e94f1ee..0ebe7e1 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5400,6 +5400,17 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
arg1,
ret = get_errno(target_munmap(arg1, arg2));
break;
case TARGET_NR_mprotect:
+{
+TaskState *ts = ((CPUState *)cpu_env)-opaque;
+/* Special hack to detect libc making the stack executable. */
+if ((arg3 PROT_GROWSDOWN)
+ arg1 = ts-info-stack_limit
+ arg1 = ts-info-start_stack) {
+arg3 = ~PROT_GROWSDOWN;
+arg2 = arg2 + arg1 - ts-info-stack_limit;
+arg1 = ts-info-stack_limit;
+}
+}
ret = get_errno(target_mprotect(arg1, arg2, arg3));
break;
#ifdef TARGET_NR_mremap
--
1.7.1
[Qemu-devel] [PATCH 0/2 v5] Release usb devices on shutdown and usb_del
v5: Fix to Gerd Hoffmann comments on v4. Shahar Havivi (2): Return usb device to host on usb_del command Return usb device to host on exit usb-linux.c | 15 +++ 1 files changed, 15 insertions(+), 0 deletions(-)
Re: [Qemu-devel] Re: [PATCH 1/2] [scsi-bus]: Add PR-OUT and PR-IN case for SCSIRequest xfer and xfer_mode setup
Am 04.06.2010 16:06, schrieb Kevin Wolf: Am 31.05.2010 03:43, schrieb Nicholas A. Bellinger: From: Nicholas Bellinger n...@linux-iscsi.org This patch updates hw/scsi-bus.c to add PERSISTENT_RESERVE_OUT and PERSISTENT_RESERVE_IN case in scsi_req_length() to extra the incoming buffer length into SCSIRequest-cmd.xfer, and adds a second PERSISTENT_RESERVE_OUT case in scsi_req_xfer_mode() in order to properly set SCSI_XFER_TO_DEV for WRITE data. Tested with Linux KVM guests and Megasas 8708EM2 HBA emulation and TCM_Loop target ports. Signed-off-by: Nicholas A. Bellinger n...@linux-iscsi.org --- hw/scsi-bus.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c index b8e4b71..75ec74e 100644 --- a/hw/scsi-bus.c +++ b/hw/scsi-bus.c @@ -325,6 +325,10 @@ static int scsi_req_length(SCSIRequest *req, uint8_t *cmd) case INQUIRY: req-cmd.xfer = cmd[4] | (cmd[3] 8); break; +case PERSISTENT_RESERVE_OUT: +case PERSISTENT_RESERVE_IN: +req-cmd.xfer = cmd[8] | (cmd[7] 8); Maybe I'm missing something, but isn't exactly the same value set in the switch block above? (for cmd[0] 5 == 2) Nicholas? This isn't applied yet because I'm waiting for your answer. Is there a reason why it makes sense to do it explicitly here instead using the generic code a few lines above? I think the same applied to patch 2/2. Kevin
[Qemu-devel] [PATCH 2/2] Return usb device to host on exit
Signed-off-by: Shahar Havivi shah...@redhat.com
---
usb-linux.c | 14 ++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/usb-linux.c b/usb-linux.c
index 22a85e3..c3c38ec 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -33,6 +33,7 @@
#include qemu-common.h
#include qemu-timer.h
#include monitor.h
+#include sysemu.h
#include dirent.h
#include sys/ioctl.h
@@ -132,6 +133,7 @@ typedef struct USBHostDevice {
int configuration;
int ninterfaces;
int closing;
+Notifier exit;
struct ctrl_struct ctrl;
struct endp_data endp_table[MAX_ENDPOINTS];
@@ -404,6 +406,7 @@ static void usb_host_handle_destroy(USBDevice *dev)
usb_host_close(s);
QTAILQ_REMOVE(hostdevs, s, next);
+qemu_remove_exit_notifier(s-exit);
}
static int usb_linux_update_endp_table(USBHostDevice *s);
@@ -997,6 +1000,15 @@ static int usb_host_close(USBHostDevice *dev)
return 0;
}
+static void usb_host_exit_notifier(struct Notifier* n)
+{
+USBHostDevice *s = container_of(n, USBHostDevice, exit);
+
+if (s-fd != -1) {
+ioctl(s-fd, USBDEVFS_RESET);
+}
+}
+
static int usb_host_initfn(USBDevice *dev)
{
USBHostDevice *s = DO_UPCAST(USBHostDevice, dev, dev);
@@ -1004,6 +1016,8 @@ static int usb_host_initfn(USBDevice *dev)
dev-auto_attach = 0;
s-fd = -1;
QTAILQ_INSERT_TAIL(hostdevs, s, next);
+s-exit.notify = usb_host_exit_notifier;
+qemu_add_exit_notifier(s-exit);
usb_host_auto_check(NULL);
return 0;
}
--
1.7.0.4
Re: [Qemu-devel] Re: [PATCH RFC] Mark a device as non-migratable
On 06/16/2010 12:05 AM, Cam Macdonell wrote: On Tue, Jun 15, 2010 at 4:33 PM, Anthony Liguorianth...@codemonkey.ws wrote: On 06/15/2010 05:26 PM, Cam Macdonell wrote: On Tue, Jun 15, 2010 at 10:32 AM, Anthony Liguorianth...@codemonkey.ws wrote: On 06/15/2010 11:16 AM, Cam Macdonell wrote: How does this look for marking the device as non-migratable? It adds a field 'no_migrate' to the SaveStateEntry and tests for it in vmstate_save. This would replace anything that touches memory. Cam --- hw/hw.h |1 + savevm.c | 32 +--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/hw/hw.h b/hw/hw.h index d78d814..7c93f08 100644 --- a/hw/hw.h +++ b/hw/hw.h @@ -263,6 +263,7 @@ int register_savevm_live(const char *idstr, void *opaque); void unregister_savevm(const char *idstr, void *opaque); +void mark_no_migrate(const char *idstr, void *opaque); I'm not thrilled with the name but the functionality is spot on. I lack the creativity to offer a better name suggestion :-) Regards, Anthony Liguori Hmmm, in working on this it seems that the memory (from qemu_ram_map()) is still attached even when the device is removed (which causes migration to fail because there is an unexpected memory). Is something like cpu_unregister_physical_memory()/qemu_ram_free() needed? Yes. You need to unregister any memory that you have registered upon device removal. Is there an established way to achieve this? I can't seem find another device that unregisters memory registered with cpu_register_physical_memory(). Is something like cpu_unregister_physical_memory() needed? cpu_register_physical_memory(IO_MEM_UNASSIGNED). If you look at pci.c, you'll see that it automatically unregisters any mapped io regions on remove. Regards, Anthony Liguori Thanks, Cam Regards, Anthony Liguori Cam
Re: [Qemu-devel] [PATCH] Fix comparison which always returned false
On 06/15/2010 07:27 PM, Richard Henderson wrote: On 06/15/2010 02:28 PM, Anthony Liguori wrote: On 06/15/2010 04:03 PM, Stefan Weil wrote: Comparing an 8 bit value with ~0 does not work as expected. Replace ~0 by UINT8_MAX in comparison and also in assignment (and fix coding style, too). Because when the uint8_t gets promoted, it doesn't get zero filled. I'd rather something a bit more obvious like HPET_INVALID_COUNT. Er, yes it does. The problem is that it *did* get zero-extended, but ~0 is 0x, so the comparison fails. Typo on my part. I meant one filled obviously. Regards, Anthony Liguori But I really agree with Jan Kiszka down-thread -- why do we need to signal this as a special case at all? r~
[Qemu-devel] Re: [PATCH V4 2/3] qemu: Generic task offloading framework: threadlets
+block-obj-y += qemu-thread.o
+block-obj-y += async-work.o
These should be (at least for now) block-obj-$(CONFIG_POSIX).
+while (QTAILQ_EMPTY((queue-request_list))
+ (ret != ETIMEDOUT)) {
+ret = qemu_cond_timedwait((queue-cond),
+ (queue-lock), 10*10);
+}
Using qemu_cond_timedwait is a hack for not properly broadcasting the
condvar in flush_threadlet_queue.
+if (QTAILQ_EMPTY((queue-request_list)))
+goto check_exit;
What's the reason for the goto? {...} works just as well.
+/**
+ * flush_threadlet_queue: Wait till completion of all the submitted tasks
+ * @queue: Queue containing the tasks we're waiting on.
+ */
+void flush_threadlet_queue(ThreadletQueue *queue)
+{
+qemu_mutex_lock(queue-lock);
+queue-exit = 1;
+
+qemu_barrier_init(queue-barr, queue-cur_threads + 1);
+qemu_mutex_unlock(queue-lock);
+
+qemu_barrier_wait(queue-barr);
Can be implemented just as well with queue-cond and a loop waiting for
queue-cur_threads == 0. This would remove the need to implement
barriers in qemu-threads (especially for Win32). Anyway whoever will
contribute Win32 qemu-threads can do it, since it's not hard.
+int cancel_threadlet_common(ThreadletWork *work)
+{
+return cancel_threadlet(globalqueue, work);
+}
I would prefer *_threadlet to be the globalqueue function (and
flush_threadlets) and queue_*_threadlet to be the special-queue
function. I should have spoken earlier probably, but please consider
this if there will be a v5.
+ * Generalization based on posix-aio emulation code.
No need to specify these as long as the original authors are attributed
properly.
+static inline void threadlet_queue_init(ThreadletQueue *queue,
+ int max_threads, int min_threads)
+{
+queue-cur_threads = 0;
+queue-idle_threads = 0;
+queue-exit = 0;
+queue-max_threads = max_threads;
+queue-min_threads = min_threads;
+QTAILQ_INIT((queue-request_list));
+QTAILQ_INIT((queue-threadlet_work_pool));
+qemu_mutex_init((queue-lock));
+qemu_cond_init((queue-cond));
+}
No need to make this inline.
+extern void threadlet_submit(ThreadletQueue *queue,
+ ThreadletWork *work);
+
+extern void threadlet_submit_common(ThreadletWork *work);
+
+extern int cancel_threadlet(ThreadletQueue *queue, ThreadletWork *work);
+extern int cancel_threadlet_common(ThreadletWork *work);
+
+
+extern void flush_threadlet_queue(ThreadletQueue *queue);
+extern void flush_common_threadlet_queue(void);
Please make the position of the verb consistent (e.g. submit_threadlet).
Paolo
[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.
On Wed, Jun 16, 2010 at 08:38:18PM +0900, Isaku Yamahata wrote:
On Wed, Jun 16, 2010 at 02:19:44PM +0300, Michael S. Tsirkin wrote:
Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
on the other hand pbc_pci_host_init() sets the register
to PCI_HEADER_TYPE_NORMAL.
To be honest I don't know why it does so, but that is what Blue wants.
BTW I think it would be prettier to have is_bridge instead of
header_type
as a qdev property. Agree?
The spec version 3.0 defines three header types.
0:normal device, 1:pci-to-pci bridge, 2:card bus bridge
So I'd like the name a bit more generic than is_bridge.
Any suggestion?
Could we just have functions that set up header for
each type, such as
pci_init_normal_header()
pci_init_p2p_bridge_header()
pci_init_cardbus_header()
I see. You mean device specific initialization function should
call one of them. Then header_type property will be dropped.
I'll split pci p2p bridge related functions into a file
at first.
Then introduce helper functions.
Just to clarify what I meant:
the common pci spec implementation should be in pci.c,
any platform that supports pci will need it.
What I think we want to move to pc_pci_bridge.c or such
is this:
static PCIDeviceInfo bridge_info = {
.qdev.name= pci-bridge,
.qdev.size= sizeof(PCIBridge),
.init = pci_bridge_initfn,
.exit = pci_bridge_exitfn,
.config_write = pci_bridge_write_config,
.header_type = PCI_HEADER_TYPE_BRIDGE,
.qdev.props = (Property[]) {
DEFINE_PROP_HEX32(vendorid, PCIBridge, vid, 0),
DEFINE_PROP_HEX32(deviceid, PCIBridge, did, 0),
DEFINE_PROP_END_OF_LIST(),
}
};
Because if I understand correctly, this is not the bridge,
it's just a pci bridge that PC has, but it is currently
instanciated even on platforms where it's unused.
This way we can avoid linking it on these platforms.
But I think the bridge header setup is common
so it should be implemented in a set of
common functions and stay in pci.c, then all bridges
can call these functions.
--
yamahata
[Qemu-devel] Re: [PATCH 5/5] linux fbdev display driver.
On Tue, 15 Jun 2010, Gerd Hoffmann wrote: Display works, requires truecolor framebuffer with 16 or 32 bpp on the host. 32bpp is recommended. The framebuffer is used as-is, qemu doesn't try to switch modes. With LCD displays mode switching is pretty pointless IMHO, also it wouldn't work anyway with the most common fbdev drivers (vesafb, KMS). Guest display is centered on the host screen. Mouse works, uses /dev/input/mice. Keyboard works. Guest screen has whatever keymap you load inside the guest. Text windows (monitor, serial, ...) have a simple en-us keymap. Good enougth to type monitor commands. Not goot enougth to work seriously on a serial terminal. But the qemu terminal emulation isn't good enougth for that anyway ;) Hot keys: Ctrl-Alt-Fnr - host console switching. Ctrl-Alt-nr - qemu console switching. Ctrl-Alt-ESC- exit qemu. Special feature: Sane console switching. Switching away stops screen updates. Switching back redraws the screen. When started from the linux console qemu uses the vt you've started it from (requires just read/write access to /dev/fb0). When starting from somewhere else qemu tries to open a unused virtual terminal and switch to it (usually requires root privileges to open /dev/ttynr). For some strange reason console switching from X11 to qemu doesn't work. Anything else (including X11 - text console - qemu) works fine. To be investigated ... Can be enabled/disabled via monitor, use change fbdev [ on | off ] the patch still doesn't use the display allocator interface, but it shouldn't be difficult to implement support for it on top of this patch, so it is fine by me.
[Qemu-devel] Re: RFC v2: blockdev_add friends, brief rationale, QMP docs
Kevin Wolf kw...@redhat.com writes:
Am 15.06.2010 15:44, schrieb Avi Kivity:
On 06/10/2010 08:45 PM, Markus Armbruster wrote:
* Our config file format is in INI syntax. QemuOpts correspond to
INI sections. Sections can't be nested, so recursive QemuOpts
don't translate.
git (and probably others) use
[a b]
c = d
for
a.b.c=d
Examples:
* Single protocol:
-blockdev id=blk1,format=raw,protocol=[file,file=fedora.img]
Requires suitable syntactic sugar to get the simple form (*).
* blkdebug
-blockdev id=blk2,format=qcow2,\
protocol=[blkdebug,config=test.blkdebug,\
protocol=[file,file=test.qcow2]]
* Avi's mirror:
-blockdev id=blk3,format=raw,\
protocol=[mirror,\
[file,file=local.img],\
[nbd,domain=unix,sockert=nbd-sock]]
2. We already have a syntax to specify trees, namely JSON, so use it
If -blockdev's argument starts with '{', it's a JSON object suitable
as argument of blockdev_add in QMP.
We still provide ordinary QemuOpts syntax for the cases that can be
expressed with it, i.e. single protocol.
I figure we'd want syntactic sugar for blkdebug, to permit its use
from the command line without having to resort to JSON.
Might be nice as a general extension to QemuOpts.
I agree.
3. Stack protocols through named references
The first protocol is inlined into -blockdev. Any further
protocols need to be referenced by name.
Best explained by example:
* Single protocol:
-blockdev id=blk1,format=raw,protocol=file,file=fedora.img
To get the simple form (*), make protocol optional with a suitable
default.
* blkdebug
-blockdev
id=blk2,format=qcow2,protocol=blkdebug,config=test.blkdebug,\
base=blk2-base
-blockproto id=blk2-base,protocol=file,file=test.qcow2
* Avi's mirror:
-blockdev id=blk3,format=raw,protocol=mirror,\
base=blk3-base1,base=blk3=base2
-blockproto id=blk3-base1,protocol=file,file=local.img
-blockproto id=blk3-base2,protocol=nbd,domain=unix,sockert=nbd-sock
Anything but a single protocol becomes pretty verbose. Syntactic
sugar for the blkdebug case would be possible; not sure it's worth
it.
No QemuOpts syntax changes. INI can handle this just fine.
Looks like the least painful option as no new infrastructure is needed.
I'd go with this.
But it's painful to type for the user. After all -blockdev on the
command line is for the user, as tools should use QMP. Also note that
this syntax mixes format and protocol options on one line which I
consider confusing at best.
As I told Markus already in private before he posted this, I prefer the
bracket solution for its clarity and simplicity, even though it comes at
the cost of having additional characters that need to be escaped.
I dont't think 1. is less painful than 3. Let's compare the two:
* Single protocol: identical with suitable syntactical sugar, namely
-blockdev id=blk1,file=fedora.img
Unsugared it's
-blockdev id=blk1,format=raw,protocol=[file,file=fedora.img]
vs.
-blockdev id=blk1,format=raw,protocol=file,file=fedora.img
I sure prefer the latter. The brackets look like noise. You need to
understand protocol stacking for them to make any sense.
Regarding confusion caused by mixing format and protocol options: yes,
the brackets force you to distinguish between protocol options and
other options. But I doubt that'll reduce confusion here. Either you
understand protocols. Then I doubt you need brackets to unconfuse
you. Or you don't understand protocols. Then whether to put an
option inside or outside the brackets is voodoo.
* blkdebug:
-blockdev id=blk2,format=qcow2,\
protocol=[blkdebug,config=test.blkdebug,\
protocol=[file,file=test.qcow2]]
vs.
-blockdev id=blk2,format=qcow2,protocol=blkdebug,config=test.blkdebug,\
base=blk2-base
-blockproto id=blk2-base,protocol=file,file=test.qcow2
Both look equally painfull to me. It's slightly shorter with
brackets, but I can't recognize clarity and simplicity there.
To really reduce pain, we'd have to invent special-purpose syntactic
sugar in either case.
* Avi's mirror:
-blockdev id=blk3,format=raw,\
protocol=[mirror,\
[file,file=local.img],\
[nbd,domain=unix,sockert=nbd-sock]]
vs.
-blockdev id=blk3,format=raw,protocol=mirror,\
base=blk3-base1,base=blk3=base2
-blockproto id=blk3-base1,protocol=file,file=local.img
-blockproto id=blk3-base2,protocol=nbd,domain=unix,sockert=nbd-sock
Same as for blkdebug.
Both with brackets and with -blockproto the protocol is clearly
separated. Only the topmost protocol isn't with -blockproto. I keep
that in -blockdev merely for brevity. It could live in its own
-blockproto.
Brackets
[Qemu-devel] [PATCH 3/3] Toggle tracepoint state
This patch adds support for dynamically enabling/disabling of tracepoints.
This is done by internally maintaining each tracepoint's state, and
permitting logging of data from a tracepoint only if it is in an
'active' state.
Monitor commands added :
1) info tracepoints : to view all available tracepoints and
their state.
2) tracepoint NAME on|off : to enable/disable data logging from a
given tracepoint.
Eg, tracepoint paio_submit off
disables logging of data when
paio_submit is hit.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
monitor.c | 16 ++
qemu-monitor.hx | 18
simpletrace.c | 63 +++
tracetool | 30 +++---
vl.c|6 +
5 files changed, 129 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index 8b60830..238bdf0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -548,6 +548,15 @@ static void do_commit(Monitor *mon, const QDict *qdict)
}
}
+#ifdef CONFIG_SIMPLE_TRACE
+static void do_change_tracepoint_state(Monitor *mon, const QDict *qdict)
+{
+const char *tp_name = qdict_get_str(qdict, name);
+bool new_state = qdict_get_bool(qdict, option);
+change_tracepoint_state(tp_name, new_state);
+}
+#endif
+
static void user_monitor_complete(void *opaque, QObject *ret_data)
{
MonitorCompletionData *data = (MonitorCompletionData *)opaque;
@@ -2791,6 +2800,13 @@ static const mon_cmd_t info_cmds[] = {
.help = show current contents of trace buffer,
.mhandler.info = do_info_trace,
},
+{
+.name = tracepoints,
+.args_type = ,
+.params = ,
+.help = show available tracepoints their state,
+.mhandler.info = do_info_all_tracepoints,
+},
#endif
{
.name = NULL,
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 766c30f..8540b8f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -117,6 +117,8 @@ show device tree
#ifdef CONFIG_SIMPLE_TRACE
@item info trace
show contents of trace buffer
+...@item info tracepoints
+show available tracepoints and their state
#endif
@end table
ETEXI
@@ -225,6 +227,22 @@ STEXI
@item logfile @var{filename}
@findex logfile
Output logs to @var{filename}.
+#ifdef CONFIG_SIMPLE_TRACE
+ETEXI
+
+{
+.name = tracepoint,
+.args_type = name:s,option:b,
+.params = name on|off,
+.help = changes status of a specific tracepoint,
+.mhandler.cmd = do_change_tracepoint_state,
+},
+
+STEXI
+...@item tracepoint
+...@findex tracepoint
+changes status of a tracepoint
+#endif
ETEXI
{
diff --git a/simpletrace.c b/simpletrace.c
index 239ae3f..4221a8f 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -3,6 +3,12 @@
#include trace.h
typedef struct {
+char *tp_name;
+bool state;
+unsigned int hash;
+} Tracepoint;
+
+typedef struct {
unsigned long event;
unsigned long x1;
unsigned long x2;
@@ -18,11 +24,29 @@ enum {
static TraceRecord trace_buf[TRACE_BUF_LEN];
static unsigned int trace_idx;
static FILE *trace_fp;
+static Tracepoint trace_list[NR_TRACEPOINTS];
+
+void init_tracepoint(const char *tname, TraceEvent tevent)
+{
+if (!tname || tevent NR_TRACEPOINTS) {
+return;
+}
+
+trace_list[tevent].tp_name = (char*)qemu_malloc(strlen(tname)+1);
+strncpy(trace_list[tevent].tp_name, tname, strlen(tname));
+trace_list[tevent].hash = qemu_hash(tname);
+trace_list[tevent].state = 1; /* Enable all by default */
+}
static void trace(TraceEvent event, unsigned long x1,
unsigned long x2, unsigned long x3,
unsigned long x4, unsigned long x5) {
TraceRecord *rec = trace_buf[trace_idx];
+
+if (!trace_list[event].state) {
+return;
+}
+
rec-event = event;
rec-x1 = x1;
rec-x2 = x2;
@@ -75,3 +99,42 @@ void do_info_trace(Monitor *mon)
trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
}
}
+
+void do_info_all_tracepoints(Monitor *mon)
+{
+unsigned int i;
+
+for (i=0; iNR_TRACEPOINTS; i++) {
+monitor_printf(mon, %s [Event ID %u] : state %u\n,
+trace_list[i].tp_name, i, trace_list[i].state);
+}
+}
+
+static Tracepoint* find_tracepoint_by_name(const char *tname)
+{
+unsigned int i, name_hash;
+
+if (!tname) {
+return NULL;
+}
+
+name_hash = qemu_hash(tname);
+
+for (i=0; iNR_TRACEPOINTS; i++) {
+if (trace_list[i].hash == name_hash
+ !strncmp(trace_list[i].tp_name, tname, strlen(tname))) {
+return trace_list[i];
+}
+}
+return NULL; /* indicates
[Qemu-devel] [PATCH 2/3] Monitor command 'info trace'
Monitor command 'info trace' to display contents of trace buffer
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
configure |3 +++
monitor.c | 12
qemu-monitor.hx |4
simpletrace.c | 13 +
tracetool |2 ++
5 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/configure b/configure
index 675d0fc..56af8dd 100755
--- a/configure
+++ b/configure
@@ -2302,6 +2302,9 @@ bsd)
esac
echo TRACE_BACKEND=$trace_backend $config_host_mak
+if test $trace_backend = simple; then
+ echo CONFIG_SIMPLE_TRACE=y $config_host_mak
+fi
if test $trace_backend = ust; then
LIBS=-lust $LIBS
fi
diff --git a/monitor.c b/monitor.c
index ad50f12..8b60830 100644
--- a/monitor.c
+++ b/monitor.c
@@ -55,6 +55,9 @@
#include json-streamer.h
#include json-parser.h
#include osdep.h
+#ifdef CONFIG_SIMPLE_TRACE
+#include trace.h
+#endif
//#define DEBUG
//#define DEBUG_COMPLETION
@@ -2780,6 +2783,15 @@ static const mon_cmd_t info_cmds[] = {
.help = show roms,
.mhandler.info = do_info_roms,
},
+#if defined(CONFIG_SIMPLE_TRACE)
+{
+.name = trace,
+.args_type = ,
+.params = ,
+.help = show current contents of trace buffer,
+.mhandler.info = do_info_trace,
+},
+#endif
{
.name = NULL,
},
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index b6e3467..766c30f 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -114,6 +114,10 @@ show migration status
show balloon information
@item info qtree
show device tree
+#ifdef CONFIG_SIMPLE_TRACE
+...@item info trace
+show contents of trace buffer
+#endif
@end table
ETEXI
diff --git a/simpletrace.c b/simpletrace.c
index 2fec4d3..239ae3f 100644
--- a/simpletrace.c
+++ b/simpletrace.c
@@ -62,3 +62,16 @@ void trace4(TraceEvent event, unsigned long x1, unsigned
long x2, unsigned long
void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned
long x3, unsigned long x4, unsigned long x5) {
trace(event, x1, x2, x3, x4, x5);
}
+
+void do_info_trace(Monitor *mon)
+{
+unsigned int i, max_idx;
+
+max_idx = trace_idx ? trace_idx : TRACE_BUF_LEN;
+
+for (i=0; imax_idx ;i++) {
+monitor_printf(mon, Event %ld : %ld %ld %ld %ld %ld\n,
+ trace_buf[i].event, trace_buf[i].x1, trace_buf[i].x2,
+trace_buf[i].x3, trace_buf[i].x4, trace_buf[i].x5);
+}
+}
diff --git a/tracetool b/tracetool
index 9ea9c08..2c73bab 100755
--- a/tracetool
+++ b/tracetool
@@ -130,6 +130,7 @@ void trace2(TraceEvent event, unsigned long x1, unsigned
long x2);
void trace3(TraceEvent event, unsigned long x1, unsigned long x2, unsigned
long x3);
void trace4(TraceEvent event, unsigned long x1, unsigned long x2, unsigned
long x3, unsigned long x4);
void trace5(TraceEvent event, unsigned long x1, unsigned long x2, unsigned
long x3, unsigned long x4, unsigned long x5);
+void do_info_trace(Monitor *mon);
EOF
simple_event_num=0
@@ -289,6 +290,7 @@ tracetoh()
#define TRACE_H
#include qemu-common.h
+#include monitor.h
EOF
convert h
echo #endif /* TRACE_H */
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
[Qemu-devel] [PATCH 1/3] Export hash function
Rename tdb_hash() to qemu_hash().
Move definition from qdict.c to a new file qemu-misc.c for use by tracing
infrastructure.
Signed-off-by: Prerna Saxena pre...@linux.vnet.ibm.com
---
Makefile.objs |2 +-
qdict.c | 24
qemu-common.h |3 +++
qemu-misc.c | 24
4 files changed, 32 insertions(+), 21 deletions(-)
create mode 100644 qemu-misc.c
diff --git a/Makefile.objs b/Makefile.objs
index 7cb40ac..53e3a65 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
###
# QObject
-qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y = qint.o qstring.o qdict.o qemu-misc.o qlist.o qfloat.o qbool.o
qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
qobject-obj-y += qerror.o
diff --git a/qdict.c b/qdict.c
index 175bc17..d4588ef 100644
--- a/qdict.c
+++ b/qdict.c
@@ -53,22 +53,6 @@ QDict *qobject_to_qdict(const QObject *obj)
}
/**
- * tdb_hash(): based on the hash agorithm from gdbm, via tdb
- * (from module-init-tools)
- */
-static unsigned int tdb_hash(const char *name)
-{
-unsigned value;/* Used to compute the hash value. */
-unsigned i; /* Used to cycle through random values. */
-
-/* Set the initial value from the key size. */
-for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++)
-value = (value + (((const unsigned char *)name)[i] (i*5 % 24)));
-
-return (1103515243 * value + 12345);
-}
-
-/**
* alloc_entry(): allocate a new QDictEntry
*/
static QDictEntry *alloc_entry(const char *key, QObject *value)
@@ -113,7 +97,7 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject
*value)
unsigned int hash;
QDictEntry *entry;
-hash = tdb_hash(key) % QDICT_HASH_SIZE;
+hash = qemu_hash(key) % QDICT_HASH_SIZE;
entry = qdict_find(qdict, key, hash);
if (entry) {
/* replace key's value */
@@ -137,7 +121,7 @@ QObject *qdict_get(const QDict *qdict, const char *key)
{
QDictEntry *entry;
-entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+entry = qdict_find(qdict, key, qemu_hash(key) % QDICT_HASH_SIZE);
return (entry == NULL ? NULL : entry-value);
}
@@ -148,7 +132,7 @@ QObject *qdict_get(const QDict *qdict, const char *key)
*/
int qdict_haskey(const QDict *qdict, const char *key)
{
-unsigned int hash = tdb_hash(key) % QDICT_HASH_SIZE;
+unsigned int hash = qemu_hash(key) % QDICT_HASH_SIZE;
return (qdict_find(qdict, key, hash) == NULL ? 0 : 1);
}
@@ -347,7 +331,7 @@ void qdict_del(QDict *qdict, const char *key)
{
QDictEntry *entry;
-entry = qdict_find(qdict, key, tdb_hash(key) % QDICT_HASH_SIZE);
+entry = qdict_find(qdict, key, qemu_hash(key) % QDICT_HASH_SIZE);
if (entry) {
QLIST_REMOVE(entry, next);
qentry_destroy(entry);
diff --git a/qemu-common.h b/qemu-common.h
index a4888e5..d225e45 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -17,6 +17,9 @@ typedef struct QEMUTimer QEMUTimer;
typedef struct QEMUFile QEMUFile;
typedef struct QEMUBH QEMUBH;
+/* Hash function definition */
+unsigned int qemu_hash(const char *name);
+
/* Hack around the mess dyngen-exec.h causes: We need QEMU_NORETURN in files
that
cannot include the following headers without conflicts. This condition has
to be removed once dyngen is gone. */
diff --git a/qemu-misc.c b/qemu-misc.c
new file mode 100644
index 000..a69196a
--- /dev/null
+++ b/qemu-misc.c
@@ -0,0 +1,24 @@
+/*
+ * Definition of tdb_hash() moved here, from qdict.c. Renamed to qemu_hash()
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+#include qemu-common.h
+
+/**
+ * tdb_hash(): based on the hash agorithm from gdbm, via tdb
+ * (from module-init-tools). Renamed to qemu_hash().
+ */
+unsigned int qemu_hash(const char *name)
+{
+unsigned value; /* Used to compute the hash value. */
+unsigned i; /* Used to cycle through random values. */
+
+/* Set the initial value from the key size. */
+for (value = 0x238F13AF * strlen(name), i=0; name[i]; i++)
+value = (value + (((const unsigned char *)name)[i] (i*5 % 24)));
+
+return (1103515243 * value + 12345);
+}
--
Prerna Saxena
Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: [...] The format I will propose is global-ID|/absolute/path, no more /path/global-ID as this comes with the risk of ambiguity (ID may shadow bus-local name of a device). Doesn't this break existing usage? Please name one. It could only be weird corner cases like device_add driver,bus=bus1/ID/bus2 or bus=ID/bus. But given that we always allowed to address bus2 directly (and this is something I cannot and will not change), does this really matter? Maybe allowing paths to start with an ID is something worth considering, will think about this again. I checked with Dan: libvirt doesn't care. So I don't either. [...]
[Qemu-devel] Re: [PATCH] monitor: Really show snapshot information about all devices
Am 16.06.2010 03:53, schrieb Miguel Di Ciurcio Filho: The 'info snapshots' monitor command does not show snapshot information from all available block devices. Usage example: $ qemu -hda disk1.qcow2 -hdb disk2.qcow2 (qemu) info snapshots Snapshot devices: ide0-hd0 Snapshot list (from ide0-hd0): IDTAG VM SIZEDATE VM CLOCK 11.5M 2010-05-26 21:51:02 00:00:03.263 21.5M 2010-05-26 21:51:09 00:00:08.844 31.5M 2010-05-26 21:51:24 00:00:23.274 41.5M 2010-05-26 21:53:17 00:00:03.595 In the above case, disk2.qcow2 has snapshot information, but it is not being shown. Only the first device is always shown. This patch updates the do_info_snapshots() function do correctly show snapshot information about all available block devices. New output: (qemu) info snapshots Snapshot list from ide0-hd0: IDTAG VM SIZEDATE VM CLOCK 11.5M 2010-05-26 21:51:02 00:00:03.263 21.5M 2010-05-26 21:51:09 00:00:08.844 31.5M 2010-05-26 21:51:24 00:00:23.274 41.5M 2010-05-26 21:53:17 00:00:03.595 Snapshot list from ide0-hd1: IDTAG VM SIZEDATE VM CLOCK 1 0 2010-05-26 21:51:02 00:00:03.263 2 0 2010-05-26 21:51:09 00:00:08.844 3 0 2010-05-26 21:51:24 00:00:23.274 4 0 2010-05-26 21:53:17 00:00:03.595 Signed-off-by: Miguel Di Ciurcio Filho miguel.fi...@gmail.com If the human monitor was exactly what its name says, I'd happily apply this one (though I think it should be made clear from which image the VM state would be loaded). However, it isn't and I'm not sure if this wouldn't break libvirt. Dan, can you help? If we can't change the info snapshots output, a possible alternative would be to introduce another info command for this. Kevin
