Re: [Qemu-devel] [PATCH 1/2] audio: honor QEMU_AUDIO_TIMER_PERIOD instead of waking up every *nano* second
On 9 Oct 2013, at 20:42, Hans de Goede wrote:
> Now that we no longer have MIN_REARM_TIMER_NS a bug in the audio subsys has
> clearly shown it self by trying to make a timer fire every nano second.
>
> Note we have a similar problem in 1.6, 1.5 and older but there
> MIN_REARM_TIMER_NS limits the wakeups caused by audio being active to
> 4000 times / second. This still causes a host cpu load of 50 % for simply
> playing audio, where as with this patch git master is at 13%, so we should
> backport this to 1.5 and 1.6 too.
>
> Note this will not apply to 1.5 and 1.6 as is.
>
> Cc: qemu-sta...@nongnu.org
> Signed-off-by: Hans de Goede
> ---
> audio/audio.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index af4cdf6..b3db679 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1124,7 +1124,8 @@ static int audio_is_timer_needed (void)
> static void audio_reset_timer (AudioState *s)
> {
> if (audio_is_timer_needed ()) {
> -timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
> +timer_mod (s->ts,
> +qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
This assumes conf.period.ticks is in nanoseconds. That seems wrong.
Suggest multiplying by SCALE_US or SCALE_MS.
Alex
> }
> else {
> timer_del (s->ts);
> --
> 1.8.3.1
>
>
>
--
Alex Bligh
Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually
Il 09/10/2013 23:26, Paolo Bonzini ha scritto:
> Il 09/10/2013 21:41, Marcelo Tosatti ha scritto:
How was that tested? For BUS_MCEERR_AO it can work, but BUS_MCEERR_AR
calls force_sig_info which does this:
ignored = action->sa.sa_handler == SIG_IGN;
blocked = sigismember(&t->blocked, sig);
if (blocked || ignored) {
action->sa.sa_handler = SIG_DFL;
if (blocked) {
sigdelset(&t->blocked, sig);
recalc_sigpending_and_wake(t);
}
if (action->sa.sa_handler == SIG_DFL)
t->signal->flags &= ~SIGNAL_UNKILLABLE;
and kills the process (because that's the default action of SIG_DFL).
>> For vcpu context its not blocked?
>
> It causes KVM to exit back to userspace, but as soon as KVM exits it
> should be blocked.
... but it's been queued and this bypasses the checks in force_sig_info.
So in guest mode it is accepted, in QEMU mode it causes a SIGBUS.
Paolo
Re: [Qemu-devel] [PATCH V3 2/7] qemu-nbd: support internal snapshot export
于 2013/10/10 14:00, Wenchao Xia 写道:
于 2013/10/2 0:08, Paolo Bonzini 写道:
Il 26/09/2013 02:16, Wenchao Xia ha scritto:
Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.
Signed-off-by: Wenchao Xia
---
block/snapshot.c | 18 ++
include/block/snapshot.h |6 ++
qemu-nbd.c | 35 ++-
3 files changed, 58 insertions(+), 1 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 2ae3099..b371c27 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,24 @@
#include "block/snapshot.h"
#include "block/block_int.h"
+QemuOptsList internal_snapshot_opts = {
+.name = "snapshot",
+.head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
+.desc = {
+{
+.name = SNAPSHOT_OPT_ID,
Why not just use "id" and "name"?
Later it is used by code:
qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
The macro is used to avoid type it twice in the codes, shouldn't it be
used?
Another reason not using "id" is because string "id" is treated as
special case
in opts_parse() so I choosed string "snapshot.id".
+.type = QEMU_OPT_STRING,
+.help = "snapshot id"
+},{
+.name = SNAPSHOT_OPT_NAME,
+.type = QEMU_OPT_STRING,
+.help = "snapshot name"
+},{
+/* end of list */
+}
+},
+};
+
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo
*sn_info,
const char *name)
{
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d05bea7..c524a49 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -27,6 +27,12 @@
#include "qemu-common.h"
#include "qapi/error.h"
+#include "qemu/option.h"
+
+#define SNAPSHOT_OPT_ID "snapshot.id"
+#define SNAPSHOT_OPT_NAME "snapshot.name"
+
+extern QemuOptsList internal_snapshot_opts;
typedef struct QEMUSnapshotInfo {
char id_str[128]; /* unique snapshot id */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..6588a1f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@
#include "block/block.h"
#include "block/nbd.h"
#include "qemu/main-loop.h"
+#include "block/snapshot.h"
#include
#include
@@ -315,7 +316,9 @@ int main(int argc, char **argv)
char *device = NULL;
int port = NBD_DEFAULT_PORT;
off_t fd_size;
-const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+QemuOpts *sn_opts = NULL;
+const char *sn_id_or_name = NULL;
+const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:L:";
struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
@@ -328,6 +331,8 @@ int main(int argc, char **argv)
{ "connect", 1, NULL, 'c' },
{ "disconnect", 0, NULL, 'd' },
{ "snapshot", 0, NULL, 's' },
+{ "load-snapshot", 1, NULL, 'l' },
Just omit the long option here...
+{ "load-snapshot1", 1, NULL, 'L' },
... and call this "load-snapshot".
Paolo
OK, I will change as:
{ NULL, 1, NULL, 'l' },
{ "load-snapshot", 1, NULL, 'L' },
From Eric's suggestion, I think simply one item:
{ "load-snapshot", 1, NULL, 'l' }
would be engough to handle both cases.
{ "nocache", 0, NULL, 'n' },
{ "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
#ifdef CONFIG_LINUX_AIO
@@ -428,6 +433,14 @@ int main(int argc, char **argv)
errx(EXIT_FAILURE, "Offset must be positive `%s'",
optarg);
}
break;
+case 'l':
+sn_id_or_name = optarg;
+nbdflags |= NBD_FLAG_READ_ONLY;
+flags&= ~BDRV_O_RDWR;
+break;
+case 'L':
+sn_opts = qemu_opts_parse(&internal_snapshot_opts,
optarg, 0);
+/* fall through */
case 'r':
nbdflags |= NBD_FLAG_READ_ONLY;
flags&= ~BDRV_O_RDWR;
@@ -581,6 +594,22 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
}
+if (sn_opts) {
+ret = bdrv_snapshot_load_tmp(bs,
+ qemu_opt_get(sn_opts,
SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts,
SNAPSHOT_OPT_NAME),
+&local_err);
+} else if (sn_id_or_name) {
+ret = bdrv_snapshot_load_tmp_by_id_or_name(bs, sn_id_or_name,
+&local_err);
+}
+if (ret< 0) {
+errno = -ret;
+err(EXIT_FAILURE,
+"Failed to load snapshot: %s",
+error_get_pretty(local_err));
+}
+
fd_size = bdrv_getlength(bs);
if (partition != -1) {
@@ -641,6 +670,10 @@ int main(int argc, char **argv)
unlink(sockpath);
}
+if (sn_opts) {
+qemu_opts_del(sn_opts);
+}
+
if (device) {
void *ret;
pthread_join(client_thread,&ret);
Re: [Qemu-devel] [PATCH V3 7/7] qemu-iotests: add test for snapshot in qemu-img convert
于 2013/10/1 22:57, Eric Blake 写道: On 09/25/2013 06:16 PM, Wenchao Xia wrote: Signed-off-by: Wenchao Xia --- @@ -53,6 +55,7 @@ _cleanup() kill $NBD_SNAPSHOT_PID fi _cleanup_test_img +rm -f $converted_image Indentation is off. will fix.
Re: [Qemu-devel] [PATCH V3 6/7] qemu-img: add doc for param -L in convert
于 2013/10/1 22:56, Eric Blake 写道: On 09/25/2013 06:16 PM, Wenchao Xia wrote: Also renamed snapshot_name to snapshot_id_or_name to tip better. s/to tip better/as a better hint of what it does/ Signed-off-by: Wenchao Xia --- qemu-img-cmds.hx |2 +- qemu-img.c |2 ++ qemu-img.texi|7 +-- 3 files changed, 8 insertions(+), 3 deletions(-) Squash this into 5/7. OK. + " 'snapshot_param' is param used for internal snapshot, format is\n" + "'snapshot.id=[ID],snapshot.name=[NAME]'\n" Again, can you reuse the existing -s, instead of having to add -L, by There may be compatiability issue for existing user, I think add -l and deprecate old -s, would be better. making the command line parser smarter about whether it is seeing a single name vs. a string starting with 'snapshot.'?
Re: [Qemu-devel] [PATCH V3 5/7] qemu-img: add -L for snapshot in convert
于 2013/10/2 0:07, Paolo Bonzini 写道:
Il 26/09/2013 02:16, Wenchao Xia ha scritto:
+c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnL:");
if (c == -1) {
break;
}
@@ -1183,6 +1184,9 @@ static int img_convert(int argc, char **argv)
case 's':
snapshot_name = optarg;
break;
+case 'L':
+sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+break;
case 'S':
Should qemu-img introduce -l too, and deprecate -s (continue to accept
it silently, but not document it)?
Paolo
OK, will document both but mark it deprecated.
Re: [Qemu-devel] [PATCH V3 4/7] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
于 2013/10/1 22:53, Eric Blake 写道:
On 09/25/2013 06:16 PM, Wenchao Xia wrote:
Signed-off-by: Wenchao Xia
---
+_export_nbd_snapshot()
+{
+eval "$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port $TEST_IMG -l $1&"
Uggh. Why do you need an eval here? Especially given that there was
recently a patch to properly quote $TEST_IMG in case the tests are run
inside a directory whose absolute name included a space. What's wrong
with just directly:
$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 $
Just a copy and paste for "eval", will remove it.
+NBD_SNAPSHOT_PID=$!
+sleep 1
+}
+
+_export_nbd_snapshot1()
+{
+eval "$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port $TEST_IMG -L
snapshot.name=$1&"
Likewise; and given my complaint on 2-3/7, it would be nicer to support
this with only one option name spelling.
+_cleanup()
+{
+if [ -n "$NBD_SNAPSHOT_PID" ]; then
+kill $NBD_SNAPSHOT_PID
+fi
+ _cleanup_test_img
Kill the TAB, fix the indentation.
Will fix.
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Any format supporting intenal snapshots
s/intenal/internal/
will fix.
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
Is this test truly Linux-only?
I think it is generic, will remove it.
Re: [Qemu-devel] [PATCH V3 3/7] qemu-nbd: add doc for internal snapshot export
于 2013/10/1 22:49, Eric Blake 写道: On 09/25/2013 06:16 PM, Wenchao Xia wrote: Signed-off-by: Wenchao Xia --- qemu-nbd.c| 11 ++- qemu-nbd.texi | 11 ++- 2 files changed, 20 insertions(+), 2 deletions(-) This should be squashed into 2/7. When adding new options, the documentation should be added at the same time. OK. +" the temporary one\n" +" -l, --load-snapshot=SNAPSHOT_ID_OR_NAME\n" +" load an internal snapshot inside FILE and export it\n" +" as an read-only device\n" +" -L, --load-snapshot1=SNAPSHOT_PARAM\n" +" load an internal snapshot inside FILE and export it\n" +" as an read-only device, SNAPSHOT_PARAM format is\n" +" 'snapshot.id=[ID],snapshot.name=[NAME]'\n" Why can't ONE option be good enough? In other words, make the command line parser smart enough so that: --load-snapshot=name tries SNAPSHOT_ID_OR_NAME, while --load-snapshot=snapshot.id=xyz,snapshot.name=name tries the SNAPSHOT_PARAM form. In other words, if the optarg begins with 'snapshot.', assume the SNAPSHOT_PARAM form, otherwise use the SNAPSHOT_ID_OR_NAME form. Then you only burn one short option letter, and avoid the problem with ambiguous abbreviation that I complained about in 2/7. I split the option as two item since want to keep capatiability for "-s snapshot.id=xyz" in qemu-img convert, it is possible some one already named a snapshot as "snapshot.id=xyz". But from the comments of Paolo, I think add a new option in qemu-img convert and deprecate -s, can solve the problem, so I will use your format in next version, thanks for tipping that.
Re: [Qemu-devel] [PATCH V3 2/7] qemu-nbd: support internal snapshot export
于 2013/10/2 0:08, Paolo Bonzini 写道:
Il 26/09/2013 02:16, Wenchao Xia ha scritto:
Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.
Signed-off-by: Wenchao Xia
---
block/snapshot.c | 18 ++
include/block/snapshot.h |6 ++
qemu-nbd.c | 35 ++-
3 files changed, 58 insertions(+), 1 deletions(-)
diff --git a/block/snapshot.c b/block/snapshot.c
index 2ae3099..b371c27 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,24 @@
#include "block/snapshot.h"
#include "block/block_int.h"
+QemuOptsList internal_snapshot_opts = {
+.name = "snapshot",
+.head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
+.desc = {
+{
+.name = SNAPSHOT_OPT_ID,
Why not just use "id" and "name"?
Later it is used by code:
qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
The macro is used to avoid type it twice in the codes, shouldn't it be used?
Another reason not using "id" is because string "id" is treated as
special case
in opts_parse() so I choosed string "snapshot.id".
+.type = QEMU_OPT_STRING,
+.help = "snapshot id"
+},{
+.name = SNAPSHOT_OPT_NAME,
+.type = QEMU_OPT_STRING,
+.help = "snapshot name"
+},{
+/* end of list */
+}
+},
+};
+
int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
const char *name)
{
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d05bea7..c524a49 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -27,6 +27,12 @@
#include "qemu-common.h"
#include "qapi/error.h"
+#include "qemu/option.h"
+
+#define SNAPSHOT_OPT_ID "snapshot.id"
+#define SNAPSHOT_OPT_NAME "snapshot.name"
+
+extern QemuOptsList internal_snapshot_opts;
typedef struct QEMUSnapshotInfo {
char id_str[128]; /* unique snapshot id */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..6588a1f 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@
#include "block/block.h"
#include "block/nbd.h"
#include "qemu/main-loop.h"
+#include "block/snapshot.h"
#include
#include
@@ -315,7 +316,9 @@ int main(int argc, char **argv)
char *device = NULL;
int port = NBD_DEFAULT_PORT;
off_t fd_size;
-const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+QemuOpts *sn_opts = NULL;
+const char *sn_id_or_name = NULL;
+const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:L:";
struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
@@ -328,6 +331,8 @@ int main(int argc, char **argv)
{ "connect", 1, NULL, 'c' },
{ "disconnect", 0, NULL, 'd' },
{ "snapshot", 0, NULL, 's' },
+{ "load-snapshot", 1, NULL, 'l' },
Just omit the long option here...
+{ "load-snapshot1", 1, NULL, 'L' },
... and call this "load-snapshot".
Paolo
OK, I will change as:
{ NULL, 1, NULL, 'l' },
{ "load-snapshot", 1, NULL, 'L' },
{ "nocache", 0, NULL, 'n' },
{ "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
#ifdef CONFIG_LINUX_AIO
@@ -428,6 +433,14 @@ int main(int argc, char **argv)
errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
}
break;
+case 'l':
+sn_id_or_name = optarg;
+nbdflags |= NBD_FLAG_READ_ONLY;
+flags&= ~BDRV_O_RDWR;
+break;
+case 'L':
+sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+/* fall through */
case 'r':
nbdflags |= NBD_FLAG_READ_ONLY;
flags&= ~BDRV_O_RDWR;
@@ -581,6 +594,22 @@ int main(int argc, char **argv)
error_get_pretty(local_err));
}
+if (sn_opts) {
+ret = bdrv_snapshot_load_tmp(bs,
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+ qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+&local_err);
+} else if (sn_id_or_name) {
+ret = bdrv_snapshot_load_tmp_by_id_or_name(bs, sn_id_or_name,
+&local_err);
+}
+if (ret< 0) {
+errno = -ret;
+err(EXIT_FAILURE,
+"Failed to load snapshot: %s",
+error_get_pretty(local_err));
+}
+
fd_size = bdrv_getlength(bs);
if (partition != -1) {
@@ -641,6 +670,10 @@ int main(int argc, char **argv)
unlink(sockpath);
}
+if (sn_opts) {
+qemu_opts_del(sn_opts);
+}
+
if (device) {
void *ret;
pthread_join(client_thread,&ret);
Re: [Qemu-devel] [PATCH V3 1/7] snapshot: distinguish id and name in load_tmp
于 2013/10/1 22:35, Eric Blake 写道: On 09/25/2013 06:16 PM, Wenchao Xia wrote: Since later this function will be used so improve it. The only caller of it now is qemu-img, and it is not impacted by introduce function bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp() twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return int to let caller know the errno, and errno will be used later. Signed-off-by: Wenchao Xia --- block/qcow2-snapshot.c| 16 +++- block/qcow2.h |5 +++- block/snapshot.c | 58 +++- include/block/block_int.h |4 ++- include/block/snapshot.h |7 - qemu-img.c|8 - 6 files changed, 89 insertions(+), 9 deletions(-) + * + * If both @snapshot_id and @name are specified, load the first one with + * id @snapshot_id and name @name. + * If only @snapshot_id is specified, load the first one with id + * @snapshot_id. + * If only @name is specified, load the first one with name @name. + * if none is specified, return -ENINVAL. s/ENINVAL/EINVAL/ + * + * Returns: 0 on success, -errno on fail. If @bs is not inserted, return + * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support + * internal snapshot, return -ENOTSUP. If qemu can't find one matching @id and + * @name, return -ENOENT. If @bs do not support parameter @snapshot_id or s/do/does/ will fix the grammar, thanks.
Re: [Qemu-devel] savevm/loadvm
On 10/09/2013 06:47 PM, Paolo Bonzini wrote: > Il 09/10/2013 09:15, Alexey Kardashevskiy ha scritto: >> Sorry for my ignorance (I never ever touched this part of qemu) but how can >> you possibly avoid block.c while doing savevm? The qcow2 driver must not >> use posix read()/write(), right? So no matter how, all writes end up in >> bdrv_co_do_writev() which changes blocks number. Or use >> raw_aio_readv()/raw_aio_writev() API directly? Please give some more hints. >> Thanks. > > I think Kevin was suggesting using qcow_aio_writev directly, or > something like that. But it is not trivial, especially because > save_vm_state takes byte offsets instead of sectors. So for now I'd > still go for the more hacky solution. I failed to find qcow_aio_writev() or anything like that. qcow2_co_writev() uses block.c. And I tried this: diff --git a/block/qcow2.c b/block/qcow2.c index 4a9888c..17faf8b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1837,10 +1837,16 @@ static int qcow2_save_vmstate(BlockDriverState *bs, QEMUIOVector *qiov, BDRVQcowState *s = bs->opaque; int growable = bs->growable; int ret; +int64_t total_sectors = bs->total_sectors; BLKDBG_EVENT(bs->file, BLKDBG_VMSTATE_SAVE); bs->growable = 1; ret = bdrv_pwritev(bs, qcow2_vm_state_offset(s) + pos, qiov); +/* + * Setting @growable may cause underlying bdrv_co_do_writev() + * to increase bs->total_sectors and we do not want this to happen. + */ +bs->total_sectors = total_sectors; bs->growable = growable; return ret; It breaks loadvm in a different (weird) way, the error is something like "ram" or "spapr/htab" (streams registered with register_savevm_live()) chunk cannot be read. Need to debug more... -- Alexey
Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.
On Wed, 2013-10-09 at 06:02 -0600, Eric Blake wrote:
> [your emailer munged the reply, making it a bit hard to read. Are you
> set for plain-text-only mail to the list?]
Thanks VERY much for remind me that, I'm using another client now.
> On 10/09/2013 12:49 AM, junqing.w...@cs2c.com.cn wrote:
>
> > >> +++ b/hmp.c
> >>> @@ -1213,10 +1213,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> >>> int detach = qdict_get_try_bool(qdict, "detach", 0);
> >>> int blk = qdict_get_try_bool(qdict, "blk", 0);
> >>> int inc = qdict_get_try_bool(qdict, "inc", 0);
> >>> + int ft = qdict_get_try_bool(qdict, "ft", 0);
> >>
> >> Why two spaces?
> >
> > To align the '=', I will remove them if you like.
>
> It's not a problem with me either way, other than we have a lot of code
> that doesn't care about alignment and consistently uses one space, and a
> fair amount of code where everything in a block of code is consistently
> aligned. But your patch was neither, in the context of the block it
> lives within - if you're going to align, then line up everything with
> the longest line 'int detach' (including blk and inc).
>
oh, I got it, you are right, I missed the longest line 'int detach ...'
I'm not going to align them.
> >
> > >
> >>> +++ b/qapi-schema.json
> >>> @@ -2420,7 +2420,8 @@
> >>> # Since: 0.14.0
> >>> ##
> >>> { 'command': 'migrate',
> >>> - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach':
> >>> 'bool' } }
> >>> + 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach':
> >>> 'bool',
> >>> + '*ft': 'bool' } }
> >>
> >> Missing documentation, including mention that the new option was only
> >> made available in 1.7. We still don't have introspection; is there some
> >> other means by which libvirt and other management apps can tell whether
> >> this feature is available?
> >
> > I'm not clear about how to do that, could you pls give me some hints, where
> > to
> > add code and documentation.
>
> As for the documentation, qapi-schema.json has plenty of examples (look
> for a field with "(since 1.7)" as a hint for how to document an optional
> field added in a later release than the main struct).
I see. Thanks.
>
> As for the introspection, Amos Kong was most recently working on trying
> to add that (but missed the 1.6 deadline, and I haven't seen work on it
> since). Introspection is not a hard requirement, but it makes it harder
> for libvirt to know if it can use 'ft':true if there is no other
> 'query-*' command that it can call first that would give it a hint that
> this is a new enough qemu to support 'ft' during migration. Maybe even
> having something listed under query-migrate-capabilities would be
> sufficient (ie. modify the 'MigrationCapability' enum to advertise a new
> capability).
Adding a new migration capability is a work-around method. we turn on ft
by using the -f option instead of setting fault-tolerant-capability to
true. I hesitate to add it.
What about adding a query for the options of migration similar to
@query-command-line-options?
[Qemu-devel] [PATCH] block: improve error message for read-only whitelisted driver
Supplement of 7780d47, with message reworded and format probe case
included: print an easy to understand message, when user tries to open a
read-only format as read-write.
Signed-off-by: Fam Zheng
---
block.c| 8 +++-
blockdev.c | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/block.c b/block.c
index 93e113a..240c28e 100644
--- a/block.c
+++ b/block.c
@@ -769,7 +769,13 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
bs->read_only = !(open_flags & BDRV_O_RDWR);
if (use_bdrv_whitelist && !bdrv_is_whitelisted(drv, bs->read_only)) {
-error_setg(errp, "Driver '%s' is not whitelisted", drv->format_name);
+if (!bs->read_only && bdrv_is_whitelisted(drv, true)) {
+error_setg(errp, "format '%s' is only supported read-only",
+ drv->format_name);
+} else {
+error_setg(errp, "Driver '%s' is not whitelisted",
+ drv->format_name);
+}
return -ENOTSUP;
}
diff --git a/blockdev.c b/blockdev.c
index 8aa66a9..c39fd9d 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -471,7 +471,7 @@ static DriveInfo *blockdev_init(QemuOpts *all_opts,
drv = bdrv_find_whitelisted_format(buf, ro);
if (!drv) {
if (!ro && bdrv_find_whitelisted_format(buf, !ro)) {
-error_report("'%s' can be only used as read-only device.",
buf);
+error_report("format '%s' is only supported read-only", buf);
} else {
error_report("'%s' invalid format", buf);
}
--
1.8.3.1
Re: [Qemu-devel] problems with 1G hugepages and linux 3.12-rc3
Hi Andy,
> On Sun, Oct 06, 2013 at 02:47:41AM +0200, andy123 wrote:
> > Hi,
> >
> > as the subject states, I have some problems with 1G hugepages with
> > qemu(-vfio-git) on Linux 3.12-rc3.
> >
> > I start qemu like this, for example:
> > "/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -mem-path /dev/hugepages
> > -drive file=/files/vm/arch.img,if=virtio,media=disk -monitor stdio"
> > where /dev/hugepages is "hugetlbfs on /dev/hugepages type hugetlbfs
> > (rw,relatime,mode=1770,gid=78,pagesize=1G,pagesize=1G)"
> > and the kernel is booted with "hugepagesz=1G hugepages=4".
> > This result in lots of error message in dmesg, as seen here:
> > https://gist.github.com/ajs124/6842823 (starting at 18:04:28)
> >
> > After starting and stopping multiple virtual machines, the hugepages seem
> > to "fill up" and qemu outputs
> > "file_ram_alloc: can't mmap RAM pages: Cannot allocate memory", but works
> > anyways.
> > With fill up, I mean that I can start qemu 2 time with "-m 2048" and 4
> > times with "-m 1024", before it fails to mmap.
> >
Thanks for discovering and reporting this problem. Could you test the
below patch?
> I can reproduce huge page leak, but not oops, but they can be related.
> Can you revert 11feeb498086a3a5907b8148bdf1786a9b18fc55 and retry?
Agreed, that it was the problematic commit. I believe it's more
correct if gigantic hugepages won't keep the reserved bit set in the
tail pages, this way we can retain the optimization. It was unexpected
that the gigapages initialization code was leaving some flag like
PG_reserved uninitialized. I put this just after the other
__SetPage... so that we load the cacheline just once, so it should be
zero cost to initialize PG_reserved properly.
==
>From 952d474fae6dc42ece4b05ce1f1489c86da2a268 Mon Sep 17 00:00:00 2001
From: Andrea Arcangeli
Date: Thu, 10 Oct 2013 01:55:45 +0200
Subject: [PATCH] hugetlb: initialize PG_reserved for tail pages of gigantig
compound pages
11feeb498086a3a5907b8148bdf1786a9b18fc55 introduced a memory leak when
KVM is run on gigantic compound pages.
11feeb498086a3a5907b8148bdf1786a9b18fc55 depends on the assumption
that PG_reserved is identical for all head and tail pages of a
compound page. So that if get_user_pages returns a tail page, we don't
need to check the head page in order to know if we deal with a
reserved page that requires different refcounting.
The assumption that PG_reserved is the same for head and tail pages is
certainly correct for THP and regular hugepages, but gigantic
hugepages allocated through bootmem don't clear the PG_reserved on the
tail pages (the clearing of PG_reserved is done later only if the
gigantic hugepage is freed).
This patch corrects the gigantic compound page initialization so that
we can retain the optimization in
11feeb498086a3a5907b8148bdf1786a9b18fc55. The cacheline was already
modified in order to set PG_tail so this won't affect the boot time of
large memory systems.
Reported-by: andy123
Signed-off-by: Andrea Arcangeli
---
mm/hugetlb.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b49579c..315450e 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -695,8 +695,24 @@ static void prep_compound_gigantic_page(struct page *page,
unsigned long order)
/* we rely on prep_new_huge_page to set the destructor */
set_compound_order(page, order);
__SetPageHead(page);
+ __ClearPageReserved(page);
for (i = 1; i < nr_pages; i++, p = mem_map_next(p, page, i)) {
__SetPageTail(p);
+ /*
+* For gigantic hugepages allocated through bootmem at
+* boot, it's safer to be consistent with the
+* not-gigantic hugepages and to clear the PG_reserved
+* bit from all tail pages too. Otherwse drivers using
+* get_user_pages() to access tail pages, may get the
+* reference counting wrong if they see the
+* PG_reserved bitflag set on a tail page (despite the
+* head page didn't have PG_reserved set). Enforcing
+* this consistency between head and tail pages,
+* allows drivers to optimize away a check on the head
+* page when they need know if put_page is needed after
+* get_user_pages() or not.
+*/
+ __ClearPageReserved(p);
set_page_count(p, 0);
p->first_page = page;
}
@@ -1329,9 +1345,9 @@ static void __init gather_bootmem_prealloc(void)
#else
page = virt_to_page(m);
#endif
- __ClearPageReserved(page);
WARN_ON(page_count(page) != 1);
prep_compound_huge_page(page, h->order);
+ WARN_ON(PageReserved(page));
prep_new_huge_page(h, page, page_to_nid(page));
/*
* If we had gigantic hu
Re: [Qemu-devel] [PATCHv3 3/3] seccomp: general fixes
On Tuesday, October 08, 2013 09:42:26 PM Eduardo Otubo wrote:
> 1) On qemu-seccomp.c:255, the variable ctx was being used
> uninitialized; now it's initialized with NULL and it's being checked at
> the end of the function.
>
> 2) Changed the name of the command line option from "enable" to
> "sandbox" for a better understanding from user side.
>
> Signed-off-by: Eduardo Otubo
> ---
> qemu-seccomp.c | 4 ++--
> vl.c | 6 +++---
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/qemu-seccomp.c b/qemu-seccomp.c
> index 84a42bc..fdd0de3 100644
> --- a/qemu-seccomp.c
> +++ b/qemu-seccomp.c
> @@ -258,7 +258,7 @@ seccomp_return:
> int seccomp_start(int list_type)
> {
> int rc = 0;
> -scmp_filter_ctx ctx;
> +scmp_filter_ctx ctx = NULL;
>
> switch (list_type) {
> case WHITELIST:
> @@ -285,7 +285,7 @@ int seccomp_start(int list_type)
>
> rc = seccomp_load(ctx);
>
> - seccomp_return:
> +seccomp_return:
> if (ctx)
> seccomp_release(ctx);
> return rc;
Any particular reason these changes weren't folded into patch 1/3?
--
paul moore
security and virtualization @ redhat
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On Tuesday, October 08, 2013 09:42:24 PM Eduardo Otubo wrote: > v3: The "-netdev tap" option is checked in the vl.c file during the > process of the command line argument list. It sets tap_enabled to true > or false according to the configuration found. Later at the seccomp > filter installation, this value is checked wheter to install or not this > feature. I like the idea of slowly making the QEMU syscall filter dependent on the runtime configuration. With that in mind, I wonder if we should have a more general purpose API in include/sysemu/seccomp.h that allows QEMU to indicate to the the QEMU/seccomp code that a particular feature is enabled. Maybe something like this: #define SCMP_FEAT_TAP ... int seccomp_feature_enable(int feature); One more comment below. > Adding a system call blacklist right before the vcpus starts. This > filter is composed by the system calls that can't be executed after the > guests are up. This list should be refined as whitelist is, with as much > testing as we can do using virt-test. > > Signed-off-by: Eduardo Otubo > --- > include/sysemu/seccomp.h | 6 - > qemu-seccomp.c | 64 > +++- vl.c | > 21 +++- > 3 files changed, 77 insertions(+), 14 deletions(-) > > diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h > index 1189fa2..9dc7e52 100644 > --- a/include/sysemu/seccomp.h > +++ b/include/sysemu/seccomp.h > @@ -15,8 +15,12 @@ > #ifndef QEMU_SECCOMP_H > #define QEMU_SECCOMP_H > > +#define WHITELIST 0 > +#define BLACKLIST 1 Should these #defines be namespaced in some way, e.g. SCMP_LIST_BLACKLIST? > #include > #include "qemu/osdep.h" > > -int seccomp_start(void); > +int seccomp_start(int list_type); > + > #endif -- paul moore security and virtualization @ redhat
Re: [Qemu-devel] [patch 0/2] force -mem-path RAM allocation
Il 09/10/2013 22:05, Marcelo Tosatti ha scritto: >>> > > Can the new option format for memory be created incrementally on >>> > > top of -mem-path-force? (agree its a good thing, it avoids proliferation >>> > > of new options). >> > >> > If you do it on top, it won't avoid proliferation, or am I missing >> > something? > Right. But in fact, the new option is not necessary. > > So please consider only patch 2 for inclusion. Do you mean only patch 1? Paolo
Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually
Il 09/10/2013 21:41, Marcelo Tosatti ha scritto:
>> > How was that tested? For BUS_MCEERR_AO it can work, but BUS_MCEERR_AR
>> > calls force_sig_info which does this:
>> >
>> > ignored = action->sa.sa_handler == SIG_IGN;
>> > blocked = sigismember(&t->blocked, sig);
>> > if (blocked || ignored) {
>> > action->sa.sa_handler = SIG_DFL;
>> > if (blocked) {
>> > sigdelset(&t->blocked, sig);
>> > recalc_sigpending_and_wake(t);
>> > }
>> >
>> > if (action->sa.sa_handler == SIG_DFL)
>> > t->signal->flags &= ~SIGNAL_UNKILLABLE;
>> >
>> > and kills the process (because that's the default action of SIG_DFL).
> For vcpu context its not blocked?
It causes KVM to exit back to userspace, but as soon as KVM exits it
should be blocked. Thus a SIGBUS with BUS_MCEERR_AR will never be
returned by sigtimedwait.
Paolo
[Qemu-devel] [Bug 1237625] [NEW] Cannot read serial from /sys/bus/usb/devices/
Public bug reported: After an update to qemu 1.6 I can't start any of my images. Qemu always crashs. I tried it with root and as a normal user... Here are some log entries I get: Type: Warning Num: 85 Date: 2013.10.09 23:48:46 549 Sender: bool System_Info::Scan_USB_Sys( QList &list ) Message: Cannot read serial from /sys/bus/usb/devices/ Type: Debug Num: 86 Date: 2013.10.09 23:48:46 553 Sender: void Virtual_Machine::QEMU_Started() Message: QEMU Start Type: Debug Num: 87 Date: 2013.10.09 23:48:46 554 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: Begin Type: Debug Num: 88 Date: 2013.10.09 23:48:46 554 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: End Type: Debug Num: 89 Date: 2013.10.09 23:48:46 575 Sender: void Virtual_Machine::QEMU_Started() Message: emit Loading_Complete() Type: Debug Num: 90 Date: 2013.10.09 23:48:47 470 Sender: void Virtual_Machine::QEMU_Finished( int exitCode, QProcess::ExitStatus exitStatus ) Message: QEMU Finished Type: Debug Num: 91 Date: 2013.10.09 23:48:47 470 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: Begin Type: Debug Num: 92 Date: 2013.10.09 23:48:47 470 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: End Type: Error Num: 93 Date: 2013.10.09 23:48:47 498 Sender: QEMU Crashed! Message: ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1237625 Title: Cannot read serial from /sys/bus/usb/devices/ Status in QEMU: New Bug description: After an update to qemu 1.6 I can't start any of my images. Qemu always crashs. I tried it with root and as a normal user... Here are some log entries I get: Type: Warning Num: 85 Date: 2013.10.09 23:48:46 549 Sender: bool System_Info::Scan_USB_Sys( QList &list ) Message: Cannot read serial from /sys/bus/usb/devices/ Type: Debug Num: 86 Date: 2013.10.09 23:48:46 553 Sender: void Virtual_Machine::QEMU_Started() Message: QEMU Start Type: Debug Num: 87 Date: 2013.10.09 23:48:46 554 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: Begin Type: Debug Num: 88 Date: 2013.10.09 23:48:46 554 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: End Type: Debug Num: 89 Date: 2013.10.09 23:48:46 575 Sender: void Virtual_Machine::QEMU_Started() Message: emit Loading_Complete() Type: Debug Num: 90 Date: 2013.10.09 23:48:47 470 Sender: void Virtual_Machine::QEMU_Finished( int exitCode, QProcess::ExitStatus exitStatus ) Message: QEMU Finished Type: Debug Num: 91 Date: 2013.10.09 23:48:47 470 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: Begin Type: Debug Num: 92 Date: 2013.10.09 23:48:47 470 Sender: bool Virtual_Machine::operator==( const Virtual_Machine &vm ) const Message: End Type: Error Num: 93 Date: 2013.10.09 23:48:47 498 Sender: QEMU Crashed! Message: To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1237625/+subscriptions
Re: [Qemu-devel] [PATCH 12/13] Add xxspltw
On 10/04/2013 06:26 AM, Tom Musta wrote:
> +case 0: {
> +tcg_gen_mov_i64(b, cpu_vsrh(xB(ctx->opcode)));
> +tcg_gen_andi_i64(b, b, 0xul);
> +tcg_gen_shri_i64(b, b, 32);
> +break;
...
> +case 2: {
> +tcg_gen_mov_i64(b, cpu_vsrl(xB(ctx->opcode)));
> +tcg_gen_andi_i64(b, b, 0xul);
> +tcg_gen_shri_i64(b, b, 32);
> +break;
No need for the and.
Perhaps better as
TCGv_i64 vsr = (uim & 2 ? cpu_vrsl(xb) : cpu_vrsh(xb));
if (uim & 1) {
tcg_gen_ext32u_i64(b, vsr);
} else {
tcg_gen_shri_i32(b, vsr, 32);
}
> +tcg_gen_shli_i64(b2, b, 32);
> +tcg_gen_or_i64(b, b, b2);
deposit.
r~
Re: [Qemu-devel] [PATCH 11/13] Add xxsel
On 10/04/2013 06:24 AM, Tom Musta wrote: > +tcg_gen_and_i64(b, b, c); > +tcg_gen_not_i64(c, c); > +tcg_gen_and_i64(a, a, c); tcg_gen_andc_i64. > +#define GEN_XXSEL() \ > +GEN_XXSEL_ROW(0x00) \ > +GEN_XXSEL_ROW(0x01) \ Why bother with defining GEN_XXSEL when its only used once? Surely just put the rows there. OTOH, this does suggest that we could do with a better way to decode the instructions, because this is ugly... r~
Re: [Qemu-devel] [PATCH 10/13] Add xxmrgh/xxmrgl
On 10/04/2013 06:23 AM, Tom Musta wrote: > +tcg_gen_andi_i64(a0, a0, 0xul); \ > +tcg_gen_shli_i64(a1, a1, 32); \ > +tcg_gen_shri_i64(b0, b0, 32); \ > +tcg_gen_andi_i64(b0, b0, 0xul); \ > +tcg_gen_andi_i64(b1, b1, 0xul); \ > +tcg_gen_or_i64(a0, a0, b0); \ > +tcg_gen_or_i64(a1, a1, b1); \ > +tcg_gen_mov_i64(cpu_vsrh(xT(ctx->opcode)), a0); \ > +tcg_gen_mov_i64(cpu_vsrl(xT(ctx->opcode)), a1); \ Two deposit operations. r~
Re: [Qemu-devel] [patch 0/2] force -mem-path RAM allocation
On Wed, Oct 09, 2013 at 08:23:26AM +0200, Markus Armbruster wrote: > Marcelo Tosatti writes: > > > On Tue, Oct 08, 2013 at 10:02:26AM +0200, Paolo Bonzini wrote: > >> Il 08/10/2013 09:32, Markus Armbruster ha scritto: > >> > We have > >> > > >> > -mem-path FILE provide backing storage for guest RAM > >> > -mem-prealloc preallocate guest memory (use with -mem-path) > >> > > >> > PATCH 2/2 adds > >> > > >> > -mem-path-force fail if unable to allocate RAM as specified by > >> > -mem-path > >> > > >> > Looks like it's time to consolidate the options related to guest memory > >> > into a single, QemuOpts-style -memory NAME=VALUE,... What do you guys > >> > think? > >> > >> Yes, we can use "-numa memory" (or "-numa mem") that Wanlong Gao is > >> adding. We can add path=, preallocate= and force= options there. > >> > >> Paolo > > > > It would be important for the new option to be backportable > > independently. Therefore mixing it with -numa is not an option. > > > > Also due to backportability supporting a new style of command line > > for -mem-path is problematic (management must be changed accordingly). > > We've converted -FOO ARG options to QemuOpts-style -FOO > NAME=VALUE,... before. You can use QemuOptsList member implied_opt_name > to get bare ARG accepted. Works except for ARGs containing '=' or ','. > > Management still has to detect whether -FOO is old or new. QMP command > query-command-line-options should do. > > > Can the new option format for memory be created incrementally on > > top of -mem-path-force? (agree its a good thing, it avoids proliferation > > of new options). > > If you do it on top, it won't avoid proliferation, or am I missing > something? Right. But in fact, the new option is not necessary. So please consider only patch 2 for inclusion.
Re: [Qemu-devel] [PATCH 27/28] memory: cpu_physical_memory_set_dirty_range() now uses bitmap operations
On 10/09/2013 05:28 AM, Juan Quintela wrote: > We were setting a range of bits, so use bitmap_set(). > > Note: xen has always been wrong, and should have used start insntead s/insntead/instead/ > of addr from the beggining. s/beggining/beginning/ > > Signed-off-by: Juan Quintela > --- > include/exec/memory-internal.h | 19 +++ > 1 file changed, 7 insertions(+), 12 deletions(-) > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 04/13] Add lxvw4x
On 10/04/2013 06:16 AM, Tom Musta wrote: > +tcg_gen_shli_tl(xth, xth, 32); > +tcg_gen_addi_tl(EA, EA, 4); > +gen_qemu_ld32u(ctx, tmp, EA); > +tcg_gen_or_tl(xth, xth, tmp); Better with deposit_i64. r~
Re: [Qemu-devel] [PATCH 26/28] memory: use find_next_bit() to find dirty bits
On 10/09/2013 05:28 AM, Juan Quintela wrote: > This operation is way faster that doing it bit by bit. s/that/than/ > > Signed-off-by: Juan Quintela > --- > include/exec/memory-internal.h | 15 ++- > 1 file changed, 6 insertions(+), 9 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [ANNOUNCE] QEMU 1.6.1 Stable released
Hi everyone, I am pleased to announce that the QEMU v1.6.1 stable release is now available at: http://wiki.qemu.org/download/qemu-1.6.1.tar.bz2 v1.6.1 is now tagged in the official qemu.git repository, and the stable-1.6 branch has been updated accordingly: http://git.qemu.org/?p=qemu.git;a=shortlog;h=refs/heads/stable-1.6 This release contains 48 build/bug fixes, including an important security fix for CVE-2013-4344 involving SCSI disk emulation: http://www.openwall.com/lists/oss-security/2013/10/02/2 Fixed by: scsi: Allocate SCSITargetReq r->buf dynamically Thank you to everyone involved! 62ecc3a: Update VERSION for 1.6.1 release (Michael Roth) fdcbe7d: scsi: Allocate SCSITargetReq r->buf dynamically (Asias He) 1b5f770: qemu: Add qemu xen logic for Xen HVM S3 resume (Liu, Jinsong) bc05a48: qemu: Adjust qemu wakeup (Liu, Jinsong) ba20326: coroutine: add ./configure --disable-coroutine-pool (Stefan Hajnoczi) ae00a27: piix4: disable io on reset (Michael S. Tsirkin) 61fbeb6: vmdk: fix cluster size check for flat extents (Fam Zheng) fc06b43: rbd: avoid qemu_rbd_snap_list() memory leaks (Stefan Hajnoczi) 6bbb9d8: tap: Use numbered tap/tun devices on all *BSD OS's (Brad Smith) b314120: iov: avoid "orig_len may be used unitialized" warning (Michael Tokarev) dc6fbaa: xhci: emulate intr endpoint intervals correctly (Gerd Hoffmann) c8adc0d: virtio-blk: do not relay a previous driver's WCE configuration to the current (Paolo Bonzini) aeab582: blockdev: do not default cache.no-flush to true (Paolo Bonzini) 5c20c1f: tci: Fix qemu-alpha on 32 bit hosts (wrong assertions) (Stefan Weil) 5d2de77: kvmvapic: Clear also physical ROM address when entering INACTIVE state (Jan Kiszka) 7ea8a3c: kvmvapic: Enter inactive state on hardware reset (Jan Kiszka) 50b31e8: kvmvapic: Catch invalid ROM size (Jan Kiszka) 4b5b472: chardev: fix pty_chr_timer (Gerd Hoffmann) 76f6989: pcnet-pci: mark I/O and MMIO as LITTLE_ENDIAN (Aurelien Jarno) 8b4b3a7: qapi-types.py: Fix enum struct sizes on i686 (Cole Robinson) 41900b0: pc_q35: Initialize Xen. (Anthony PERARD) 755ec4c: pc: Initializing ram_memory under Xen. (Anthony PERARD) dc0973b: qxl: fix local renderer (Gerd Hoffmann) b6d163f: ehci: save device pointer in EHCIState (Gerd Hoffmann) a1991d0: ne2000: mark I/O as LITTLE_ENDIAN (Aurelien Jarno) 1110014: exec: check offset_within_address_space for register subpage (Hu Tao) 2a93d3d: Revert "memory: Return -1 again on reads from unsigned regions" (Jan Kiszka) 7ab1044: memory: Provide separate handling of unassigned io ports accesses (Jan Kiszka) e8601a4: w32: Fix access to host devices (regression) (Stefan Weil) 96b14d0: usb: parallelize usb3 streams (Gerd Hoffmann) 9dbfbb8: xhci: reset port when disabling slot (Gerd Hoffmann) 57ea2d2: exec: always use MADV_DONTFORK (Andrea Arcangeli) 1cd7138: virtio_pci: fix level interrupts with irqfd (Michael S. Tsirkin) 9fab8e1: exec: fix writing to MMIO area with non-power-of-two length (Paolo Bonzini) 2ffbe03: adlib: sort offsets in portio registration (Hervé Poussineau) f9fd82e: target-i386: fix disassembly with PAE=1, PG=0 (Paolo Bonzini) da4e203: block: expect errors from bdrv_co_is_allocated (Paolo Bonzini) c09a463: Revert "usb-hub: report status changes only once" (Gerd Hoffmann) c0a5eb8: xhci: fix endpoint interval calculation (Gerd Hoffmann) 358bb0d: virtio: virtqueue_get_avail_bytes: fix desc_pa when loop over the indirect descriptor table (yinyin) 3fe494e: pseries: Fix stalls on hypervisor virtual console (Anton Blanchard) a73c74f: pc: fix regression for 64 bit PCI memory (Michael S. Tsirkin) 964e0d4: scsi: Fix scsi_bus_legacy_add_drive() scsi-generic with serial (Markus Armbruster) 11b0ab7: usb/dev-hid: Modified usb-tablet category from Misc to Input (Marcel Apfelbaum) d6dcfd6: scripts/qapi.py: Avoid syntax not supported by Python 2.4 (Peter Maydell) 2607906: rdma: silly ipv6 bugfix (Michael R. Hines) 52f99b0: target-ppc: fix bit extraction for FPBF and FPL (Aurelien Jarno) c0c080c: gdbstub: Fix gdb_register_coprocessor() register counting (Andreas Färber) 670599a: block: ensure bdrv_drain_all() works during bdrv_delete() (Stefan Hajnoczi)
Re: [Qemu-devel] [PATCH 19/28] memory: split dirty bitmap into three
On 10/09/2013 05:28 AM, Juan Quintela wrote:
> After all the previous patches, spliting the bitmap gets direct.
>
> ToDo: Why can't i include "exec/memory.h" into cpu-all.h? This is the
> reason that I have duplicated DIRTY_MEMORY_NUM.
>
> ToDo2: current bitmaps have one int as index, this limit us to 8TB RAM
>guest, Should we move to longs?
>
> Signed-off-by: Juan Quintela
> ---
> +for(i = 0; i < DIRTY_MEMORY_NUM; i++) {
Not your typical formatting.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 1/2] audio: honor QEMU_AUDIO_TIMER_PERIOD instead of waking up every *nano* second
Now that we no longer have MIN_REARM_TIMER_NS a bug in the audio subsys has
clearly shown it self by trying to make a timer fire every nano second.
Note we have a similar problem in 1.6, 1.5 and older but there
MIN_REARM_TIMER_NS limits the wakeups caused by audio being active to
4000 times / second. This still causes a host cpu load of 50 % for simply
playing audio, where as with this patch git master is at 13%, so we should
backport this to 1.5 and 1.6 too.
Note this will not apply to 1.5 and 1.6 as is.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Hans de Goede
---
audio/audio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/audio/audio.c b/audio/audio.c
index af4cdf6..b3db679 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1124,7 +1124,8 @@ static int audio_is_timer_needed (void)
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
-timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
+timer_mod (s->ts,
+qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + conf.period.ticks);
}
else {
timer_del (s->ts);
--
1.8.3.1
[Qemu-devel] [PATCH 2/2] audio: Lower default wakeup rate to 100 times / second
This is more then plenty to keep audio card fifos filles / emptied.
This drops host cpu-load for audio playback inside a linux vm from
13% to 9%.
Signed-off-by: Hans de Goede
---
audio/audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/audio.c b/audio/audio.c
index b3db679..fc77511 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -95,7 +95,7 @@ static struct {
}
},
-.period = { .hertz = 250 },
+.period = { .hertz = 100 },
.plive = 0,
.log_to_monitor = 0,
.try_poll_in = 1,
--
1.8.3.1
Re: [Qemu-devel] [patch 1/2] qemu: mempath: prefault pages manually
On Wed, Oct 09, 2013 at 10:05:44AM +0200, Paolo Bonzini wrote:
> Il 08/10/2013 23:51, Marcelo Tosatti ha scritto:
> > On Tue, Oct 08, 2013 at 10:03:48AM +0200, Paolo Bonzini wrote:
> >> Il 08/10/2013 02:41, Marcelo Tosatti ha scritto:
> >>> +/* unblock SIGBUS */
> >>> +pthread_sigmask(SIG_BLOCK, NULL, &oldset);
> >>> +sigemptyset(&set);
> >>> +sigaddset(&set, SIGBUS);
> >>> +pthread_sigmask(SIG_UNBLOCK, &set, NULL);
> >>
> >> Please instead modify qemu-thread-posix.c to unblock all per-thread
> >> signals (SIGBUS, SIGSEGV, SIGILL, SIGFPE and SIGSYS). There is no need
> >> to keep those blocked.
> >
> > main-loop.c handles SIGBUS via signalfd to emulate MCEs (associated
> > commits). Therefore it must be blocked.
>
> How was that tested? For BUS_MCEERR_AO it can work, but BUS_MCEERR_AR
> calls force_sig_info which does this:
>
> ignored = action->sa.sa_handler == SIG_IGN;
> blocked = sigismember(&t->blocked, sig);
> if (blocked || ignored) {
> action->sa.sa_handler = SIG_DFL;
> if (blocked) {
> sigdelset(&t->blocked, sig);
> recalc_sigpending_and_wake(t);
> }
>
> if (action->sa.sa_handler == SIG_DFL)
> t->signal->flags &= ~SIGNAL_UNKILLABLE;
>
> and kills the process (because that's the default action of SIG_DFL).
For vcpu context its not blocked?
> > Note that what this patch does it to maintain the signal handling state
> > (it saves the previous state, modifies state, restores previous state) so
> > that its unchanged.
>
> Yes, understood. I was missing the part about MCE (I knew it used
> SIGBUS, but forgot about signalfd). So this patch is good, but the
> above point about BUS_MCEERR_AR needs to be checked sooner or later.
>
> Paolo
Re: [Qemu-devel] [PATCH 13/28] memory: use DIRTY_MEMORY_* instead of *_DIRTY_FLAG
On 10/09/2013 05:28 AM, Juan Quintela wrote: > Instead of the bitmap, we use the bitmap number. Once done this, we s/done this/this is done/ > change all names from dirty_flag to memory regions naming of client. > > Signed-off-by: Juan Quintela > --- -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 14/28] memory: use bit 2 for migration
On 10/09/2013 05:28 AM, Juan Quintela wrote: > For historical reasons it was bit 3. One there create a constant to s/One there/Once there,/ > know the number of clients. > > Signed-off-by: Juan Quintela > --- -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 08/28] exec: simplify notdirty_mem_write()
On 10/09/2013 01:10 PM, Eric Blake wrote: > On 10/09/2013 05:28 AM, Juan Quintela wrote: >> We don't need to make special things for CODE, just set the other two bits >> >> Signed-off-by: Juan Quintela >> --- >> exec.c | 8 ++-- >> 1 file changed, 2 insertions(+), 6 deletions(-) >> > >> -dirty_flags |= (0xff & ~CODE_DIRTY_FLAG); >> -cpu_physical_memory_set_dirty_flags(ram_addr, dirty_flags); >> +cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG); >> +cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG); > > Worth writing as a single call: > > cpu_physical_memory_set_dirty_flag(ram_addr, > MIGRATION_DIRTY_FLAG|VGA_DIRTY_FLAG); > > or will that just get in the way of refactoring later in the series? Answering myself - it gets in the way. Doing things explicitly one flag at a time makes it easier to redirect flags to separate tables. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 12/28] memory: cpu_physical_memory_mask_dirty_range() allways clear a single flag
On 10/09/2013 05:28 AM, Juan Quintela wrote: > Document it s/allways/always/ in the subject -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi, On 10/09/2013 09:03 PM, Paolo Bonzini wrote: Il 09/10/2013 20:49, Hans de Goede ha scritto: I wonder whether it's meant to be 1 millisecond or 1 microsecond? Maybe once it was 1 ms, this code just exists to keep the buffers of a soundcard filled / emptied in time. 100 times / second is more then plenty for that, so that is what I'm going to use in the patch I'm about to submit. It was 1 ms when that was the resolution of the "alarm tick" (which was based on /dev/rtc or /dev/hpet), then it became 250 us with "dynamic ticks", then we know how it became 0 (1 ns after timer_mod is effectively 0 ns after select). Yep, and the weird thing is, there is a cmdline option to set the wakeup frequency in hz, and parsing code for it, and code to convert it to ticks, it is just not used. So I'm throwing away my initial patch for this and writing a new one. Regards, Hans
Re: [Qemu-devel] [PATCH 08/28] exec: simplify notdirty_mem_write()
On 10/09/2013 05:28 AM, Juan Quintela wrote: > We don't need to make special things for CODE, just set the other two bits > > Signed-off-by: Juan Quintela > --- > exec.c | 8 ++-- > 1 file changed, 2 insertions(+), 6 deletions(-) > > -dirty_flags |= (0xff & ~CODE_DIRTY_FLAG); > -cpu_physical_memory_set_dirty_flags(ram_addr, dirty_flags); > +cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG); > +cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG); Worth writing as a single call: cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG|VGA_DIRTY_FLAG); or will that just get in the way of refactoring later in the series? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Il 09/10/2013 20:49, Hans de Goede ha scritto: >> >> I wonder whether it's meant to be 1 millisecond or 1 microsecond? > > Maybe once it was 1 ms, this code just exists to keep the buffers > of a soundcard filled / emptied in time. 100 times / second is more > then plenty for that, so that is what I'm going to use in the patch I'm > about to submit. It was 1 ms when that was the resolution of the "alarm tick" (which was based on /dev/rtc or /dev/hpet), then it became 250 us with "dynamic ticks", then we know how it became 0 (1 ns after timer_mod is effectively 0 ns after select). Paolo
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi,
On 10/09/2013 08:36 PM, Alex Bligh wrote:
On 9 Oct 2013, at 19:28, Alex Bligh wrote:
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
}
else {
timer_del (s->ts);
}
}
static void audio_timer (void *opaque)
{
audio_run ("timer");
audio_reset_timer (opaque);
}
Note how that is using a timer which expires every freaking nano second,
I think it is very likely that is the culprit.
Indeed. I am hoping that it is not my automated perl conversion code that
did that, because if it is, it may have broken other stuff :-/
Thanks for finding this - let me see whether the bug existed before
my automated changes commit.
I think this was broken prior to my changes. Here's audio/audio.c before
my changes:
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
qemu_mod_timer (s->ts, qemu_get_clock_ns (vm_clock) + 1);
}
else {
qemu_del_timer (s->ts);
}
}
Now qemu_get_clock_ns can only return something in nanoseconds, so it's
adding 1 nanosecond. This is thus either broken because:
1. ts->scale is in nanoseconds, in which case that timer expires in
one nano-second's time; or
2. ts->scale is not in nanoseconds, in which case nanosecond VM clock
is going to be a huge time in the future, and it's never going
to expire.
I wonder whether it's meant to be 1 millisecond or 1 microsecond?
Maybe once it was 1 ms, this code just exists to keep the buffers
of a soundcard filled / emptied in time. 100 times / second is more
then plenty for that, so that is what I'm going to use in the patch I'm
about to submit.
Regards,
Hans
p.s.
I still think we should unlock the io-thread in the main_loop_wait when
called with nonblocking == 0, even if there are expired timers. Any
suggestions on how to best do that ?
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
On 9 Oct 2013, at 19:28, Alex Bligh wrote:
>>
>> static void audio_reset_timer (AudioState *s)
>> {
>> if (audio_is_timer_needed ()) {
>> timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
>> }
>> else {
>> timer_del (s->ts);
>> }
>> }
>>
>> static void audio_timer (void *opaque)
>> {
>> audio_run ("timer");
>> audio_reset_timer (opaque);
>> }
>>
>> Note how that is using a timer which expires every freaking nano second,
>> I think it is very likely that is the culprit.
>
> Indeed. I am hoping that it is not my automated perl conversion code that
> did that, because if it is, it may have broken other stuff :-/
>
> Thanks for finding this - let me see whether the bug existed before
> my automated changes commit.
I think this was broken prior to my changes. Here's audio/audio.c before
my changes:
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
qemu_mod_timer (s->ts, qemu_get_clock_ns (vm_clock) + 1);
}
else {
qemu_del_timer (s->ts);
}
}
Now qemu_get_clock_ns can only return something in nanoseconds, so it's
adding 1 nanosecond. This is thus either broken because:
1. ts->scale is in nanoseconds, in which case that timer expires in
one nano-second's time; or
2. ts->scale is not in nanoseconds, in which case nanosecond VM clock
is going to be a huge time in the future, and it's never going
to expire.
I wonder whether it's meant to be 1 millisecond or 1 microsecond?
Whatever, it looks like this is suitable for an individual patch to
audio/audio.c
--
Alex Bligh
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
On 9 Oct 2013, at 19:03, Hans de Goede wrote:
> static void audio_reset_timer (AudioState *s)
> {
>if (audio_is_timer_needed ()) {
>timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
>}
>else {
>timer_del (s->ts);
>}
> }
>
> static void audio_timer (void *opaque)
> {
>audio_run ("timer");
>audio_reset_timer (opaque);
> }
>
> Note how that is using a timer which expires every freaking nano second,
> I think it is very likely that is the culprit.
Indeed. I am hoping that it is not my automated perl conversion code that
did that, because if it is, it may have broken other stuff :-/
Thanks for finding this - let me see whether the bug existed before
my automated changes commit.
--
Alex Bligh
Re: [Qemu-devel] [PATCH 28/28] memory: cpu_physical_memory_clear_dirty_range() now uses bitmap operations
On 10/09/2013 04:28 AM, Juan Quintela wrote:
> We were clearing a range of bits, so use bitmap_set().
Comment is slightly wrong. ;-)
>
> Signed-off-by: Juan Quintela
> ---
> include/exec/memory-internal.h | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
> index 2f704e8..d46570e 100644
> --- a/include/exec/memory-internal.h
> +++ b/include/exec/memory-internal.h
> @@ -95,13 +95,11 @@ static inline void
> cpu_physical_memory_clear_dirty_range(ram_addr_t start,
> ram_addr_t length,
> unsigned client)
> {
> -ram_addr_t addr, end;
> +unsigned long end, page;
>
> -end = TARGET_PAGE_ALIGN(start + length);
> -start &= TARGET_PAGE_MASK;
> -for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
> -clear_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
> -}
> +end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
> +page = start >> TARGET_PAGE_BITS;
> +bitmap_clear(ram_list.dirty_memory[client], page, end - page);
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi,
On 10/09/2013 08:03 PM, Hans de Goede wrote:
So I started looking for suspecious timers under audio/*.c and immediately
found this in audio/audio.c :
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
}
else {
timer_del (s->ts);
}
}
static void audio_timer (void *opaque)
{
audio_run ("timer");
audio_reset_timer (opaque);
}
Note how that is using a timer which expires every freaking nano second,
I think it is very likely that is the culprit.
Yep, this is the culprit, patch coming up.
Regards,
Hans
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi, On 10/09/2013 04:37 PM, Paolo Bonzini wrote: Il 09/10/2013 14:58, Hans de Goede ha scritto: I still think we should add my patch in some form, since the lock starvation caused by timers set to expire in the past could still happen in other cases, esp for timer users who take a time stamp once and then add incremental values to trigger the next run, these can get behind quite a bit if there are latency spikes, and we don't wont to run without ever releasing the lock while these are catching up. I agree. Do you also agree that the equivalent workaround, before Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)? I think I agree :) Regards, Hans
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi,
On 10/09/2013 03:18 PM, Alex Bligh wrote:
qemu_mod_timer(timer->timer, qemu_get_clock_ms(rt_clock) + ms);
qemu_mod_timer does not exist in master.
This line is now:
timer_mod(timer->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + ms);
Ah yes my bad, I was accidentally looking at a stable-1.5 checkout.
This suggests to me either timer->scale is not set to 100 (i.e.
it isn't in milliseconds),
The timer always gets created like this:
timer->timer = timer_new_ms(QEMU_CLOCK_REALTIME, func, opaque);
OR ms is zero/negative.
Nope I've added a debug printf to the spice timer code and it all works
as expected.
So I started looking for suspecious timers under audio/*.c and immediately
found this in audio/audio.c :
static void audio_reset_timer (AudioState *s)
{
if (audio_is_timer_needed ()) {
timer_mod (s->ts, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 1);
}
else {
timer_del (s->ts);
}
}
static void audio_timer (void *opaque)
{
audio_run ("timer");
audio_reset_timer (opaque);
}
Note how that is using a timer which expires every freaking nano second,
I think it is very likely that is the culprit.
Regards,
Hans
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi, On 10/09/2013 06:26 PM, Paolo Bonzini wrote: Il 09/10/2013 18:19, Alex Bligh ha scritto: Do you also agree that the equivalent workaround, before Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)? I don't think this was the case, as if it's a timer constantly expiring we'd have seen select() exit as soon as it was entered by the fd poked by the signal. The signal itself was clamped to be at least 250 microseconds... That might be far more frequent. ... it's true though that it could have been less than 250 microseconds (more precisely, 250 microseconds minus the time from qemu_mod_timer_ns to select). Since the CPU usage with Hans's patch is 100% and used to be 50%, it was also more than 1 ns that Hans's patch is using. I think the equivalent would be something like: if the 'zero' timeout comes from the deadline calculation (and not nonblocking=true) then release the lock anyway. I think that would be a reasonable approach. I would however like to get to the bottom of what's causing this as even pre my changes playing sound was apparently taking 50% CPU, which is not good. I am completely packed until the weekend but I propose producing a timer debug patch which will instrument what is expiring constantly (unless the problem with spice is obvious to someone). I think Hans already debugged it to the (supposedly) 33 Hz timer that spice audio uses. Correction, I've been looking at timers in the spice audio path which could potentially cause this, and this one stood out. The real problem could be a completely different timer! If it turns out the bug is in the QEMU part of spice, I think it makes sense _not_ to include this patch at all. As you said yourself before in a previous mail, at a minimum we should ensure we always unlock the iolock when called in blocking mode, to give other threads a chance to aquire it, so we need some form of my patch, or some other patch which achieves the unlocking, I welcome other proposals which have the same end result :) Regards, Hans
Re: [Qemu-devel] [RFC] map 64-bit PCI devices after all possible RAM
On Wed, 09 Oct 2013 15:12:08 +0200
Gerd Hoffmann wrote:
> On Mi, 2013-10-09 at 14:23 +0200, Igor Mammedov wrote:
> > I'm posting it to get an oppinion on one of possible approaches
> > on where to map a hotplug memory.
> >
> > This patch assumes that a space for hotplug memory is located right
> > after RamSizeOver4G region and QEMU will provide romfile to specify
> > where it ends so that BIOS could know from what base to start
> > 64-bit PCI devices mapping.
>
> We should think about both pci hotplug and memory hotplug while being at
> it.
>
> Today the 64bit pci window is mapped right above high memory and is
> sized (in acpi tables) according to what is needed to map the devices
> present at boot.
>
> Effect is that there is no extra address space for 64bit bars of
> hotplugged pci devices. And the window is also in the way when it comes
> to memory hotplug.
>
> Given that some windows versions don't like the large 64bit windows we
> should make the window size configurable.
So far from QEMU side it's partially (only memory region mapping and not ACPI
window) configurable via {i440FX-pcihost|q35-pcihost}.pci-hole64-size property
>
> The window location can either be made configurable too, or we simply
> place it at the top of the address space, with "address space" being
> what the cpu can address according to cpuinfo.
An earlier attempt by Michael to push complete PCI window placement info
via "etc/pci-info" romfile to Seabios was rejected in favor of letting Seabios
to program windows at hardcoded(32-bit/behind high mem) locations with a
64-bit window size (in ACPI) that covers all present devices but doesn't
account for future PCI hotplug either.
That behavior maintained in his "ACPI in QEMU" series, see:
http://patchwork.ozlabs.org/patch/281032/
acpi_get_pci_info()->i440fx_pcihost_get_pci_hole64_end()->pci_bus_get_w64_range()
which is then embedded in ACPI table. So end result stays the same as
before (no usable 64-bit PCI window for hotlug).
But 64-bit PCI window size, which is capped by QEMU to insane legacy 62 bits
(memory region size), is a bit of orthogonal to freeing space for memory
hotplug before it.
>
> Current qemu reports this by default:
>
> $ cat /proc/cpuinfo
> model name : QEMU Virtual CPU version 1.5.3
> address sizes : 40 bits physical, 48 bits virtual
>
> 40 address lines allow 1TB, so we would place the window just below 1TB.
>
> Comments?
More to the point if OS supports/enforces 1Tb physical address space,the RAM
and 64-bit PCI hole are going to contend for it, QEMU could abort on startup
if they both do not fit in CPU supported address space but I don't see what
else it could do.
Proposed patch favors RAM vs 64-bit PCI hole and moves the hole behind the
possible RAM, which in present state of QEMU potentially leaves the rest of
address space up to 62 bits for hole.
It has drawback that one can't get a working VM if QEMU is started in
memory hotlug mode with old BIOS + PCI devices that require 64-bit bars,
otherwise it's backward compatible.
PS:
As for remedying BSODs because of huge CRS sizes of particular RAM device/PCI
window, it might be solved by splitting one big chunk in several smaller, at
least it works for RAM device.
>
> Gerd
>
>
>
--
Regards,
Igor
Re: [Qemu-devel] [PATCH v5] Extend qemu-ga's 'guest-info' command to expose flag 'success-response'
Quoting Mark Wu (2013-10-08 21:37:26)
> Now we have several qemu-ga commands not returning response on success.
> It has been documented in qga/qapi-schema.json already. This patch exposes
> the 'success-response' flag by extending 'guest-info' command. With this
> change, the clients can handle the command response more flexibly.
>
> Signed-off-by: Mark Wu
Thanks, applied to qga tree:
https://github.com/mdroth/qemu/commits/qga
> ---
> v5:
> Fix a tab indent and rebase
> v4:
> Add signature of qmp_has_success_response per Michael.
> v3:
> 1. treat cmd->options as a bitmask instead of single option (per
> Eric)
> 2. rebase on the patch " Add interface to traverse the qmp command
> list
> by QmpCommand" to avoid the O(n2) problem (per Eric and Michael)
> v2:
> add the notation 'since 1.7' to the option 'success-response'
> (per Eric Blake's comments)
>
> include/qapi/qmp/dispatch.h | 1 +
> qapi/qmp-registry.c | 5 +
> qga/commands.c | 1 +
> qga/qapi-schema.json| 5 -
> 4 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 7d759ef..cea3818 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -49,6 +49,7 @@ void qmp_disable_command(const char *name);
> void qmp_enable_command(const char *name);
> bool qmp_command_is_enabled(const QmpCommand *cmd);
> const char *qmp_command_name(const QmpCommand *cmd);
> +bool qmp_has_success_response(const QmpCommand *cmd);
> QObject *qmp_build_error_object(Error *errp);
> typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
> void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 5e26710..3e4498a 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -76,6 +76,11 @@ const char *qmp_command_name(const QmpCommand *cmd)
> return cmd->name;
> }
>
> +bool qmp_has_success_response(const QmpCommand *cmd)
> +{
> +return !(cmd->options & QCO_NO_SUCCESS_RESP);
> +}
> +
> void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
> {
> QmpCommand *cmd;
> diff --git a/qga/commands.c b/qga/commands.c
> index 063b22b..7f089ba 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -54,6 +54,7 @@ static void qmp_command_info(QmpCommand *cmd, void *opaque)
> cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> cmd_info->name = g_strdup(cmd->name);
> cmd_info->enabled = qmp_command_is_enabled(cmd);
> +cmd_info->success_response = qmp_has_success_response(cmd);
>
> cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> cmd_info_list->value = cmd_info;
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 7155b7a..245f968 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -141,10 +141,13 @@
> #
> # @enabled: whether command is currently enabled by guest admin
> #
> +# @success-response: whether command returns a response on success
> +#(since 1.7)
> +#
> # Since 1.1.0
> ##
> { 'type': 'GuestAgentCommandInfo',
> - 'data': { 'name': 'str', 'enabled': 'bool' } }
> + 'data': { 'name': 'str', 'enabled': 'bool', 'success-response': 'bool' } }
>
> ##
> # @GuestAgentInfo
> --
> 1.8.3.1
Re: [Qemu-devel] [PATCH v4] Add interface to traverse the qmp command list by QmpCommand
Quoting Mark Wu (2013-10-08 22:25:07)
> In the original code, qmp_get_command_list is used to construct
> a list of all commands' name. To get the information of all qga
> commands, it traverses the name list and search the command info
> with its name. So it can cause O(n^2) in the number of commands.
>
> This patch adds an interface to traverse the qmp command list by
> QmpCommand to replace qmp_get_command_list. It can decrease the
> complexity from O(n^2) to O(n).
>
> Signed-off-by: Mark Wu
Thanks, applied to qga tree:
https://github.com/mdroth/qemu/commits/qga
> ---
> v4:
> Add the missing change from cmd->namd to qmp_command_name(cmd)
> v3:
> Add an accessor for cmd->name to avoid exposing internals of QmpCommand
> v2:
> 1. Keep the signature of qmp_command_is_enabled (per Eric and Michael)
> 2. Remove the unnecessary pointer castings (per Eric)
> include/qapi/qmp/dispatch.h | 6 ++--
> qapi/qmp-registry.c | 30 +-
> qga/commands.c | 38 +--
> qga/main.c | 75
> ++---
> 4 files changed, 57 insertions(+), 92 deletions(-)
>
> diff --git a/include/qapi/qmp/dispatch.h b/include/qapi/qmp/dispatch.h
> index 1ce11f5..7d759ef 100644
> --- a/include/qapi/qmp/dispatch.h
> +++ b/include/qapi/qmp/dispatch.h
> @@ -47,9 +47,11 @@ QmpCommand *qmp_find_command(const char *name);
> QObject *qmp_dispatch(QObject *request);
> void qmp_disable_command(const char *name);
> void qmp_enable_command(const char *name);
> -bool qmp_command_is_enabled(const char *name);
> -char **qmp_get_command_list(void);
> +bool qmp_command_is_enabled(const QmpCommand *cmd);
> +const char *qmp_command_name(const QmpCommand *cmd);
> QObject *qmp_build_error_object(Error *errp);
> +typedef void (*qmp_cmd_callback_fn)(QmpCommand *cmd, void *opaque);
> +void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque);
>
> #endif
>
> diff --git a/qapi/qmp-registry.c b/qapi/qmp-registry.c
> index 28bbbe8..5e26710 100644
> --- a/qapi/qmp-registry.c
> +++ b/qapi/qmp-registry.c
> @@ -66,35 +66,21 @@ void qmp_enable_command(const char *name)
> qmp_toggle_command(name, true);
> }
>
> -bool qmp_command_is_enabled(const char *name)
> +bool qmp_command_is_enabled(const QmpCommand *cmd)
> {
> -QmpCommand *cmd;
> -
> -QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -if (strcmp(cmd->name, name) == 0) {
> -return cmd->enabled;
> -}
> -}
> +return cmd->enabled;
> +}
>
> -return false;
> +const char *qmp_command_name(const QmpCommand *cmd)
> +{
> +return cmd->name;
> }
>
> -char **qmp_get_command_list(void)
> +void qmp_for_each_command(qmp_cmd_callback_fn fn, void *opaque)
> {
> QmpCommand *cmd;
> -int count = 1;
> -char **list_head, **list;
> -
> -QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -count++;
> -}
> -
> -list_head = list = g_malloc0(count * sizeof(char *));
>
> QTAILQ_FOREACH(cmd, &qmp_commands, node) {
> -*list = g_strdup(cmd->name);
> -list++;
> +fn(cmd, opaque);
> }
> -
> -return list_head;
> }
> diff --git a/qga/commands.c b/qga/commands.c
> index 528b082..e87cbf8 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -45,35 +45,27 @@ void qmp_guest_ping(Error **err)
> slog("guest-ping called");
> }
>
> -struct GuestAgentInfo *qmp_guest_info(Error **err)
> +static void qmp_command_info(QmpCommand *cmd, void *opaque)
> {
> -GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
> +GuestAgentInfo *info = opaque;
> GuestAgentCommandInfo *cmd_info;
> GuestAgentCommandInfoList *cmd_info_list;
> -char **cmd_list_head, **cmd_list;
> -
> -info->version = g_strdup(QEMU_VERSION);
> -
> -cmd_list_head = cmd_list = qmp_get_command_list();
> -if (*cmd_list_head == NULL) {
> -goto out;
> -}
>
> -while (*cmd_list) {
> -cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> -cmd_info->name = g_strdup(*cmd_list);
> -cmd_info->enabled = qmp_command_is_enabled(cmd_info->name);
> +cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo));
> +cmd_info->name = g_strdup(qmp_command_name(cmd));
> +cmd_info->enabled = qmp_command_is_enabled(cmd);
>
> -cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> -cmd_info_list->value = cmd_info;
> -cmd_info_list->next = info->supported_commands;
> -info->supported_commands = cmd_info_list;
> +cmd_info_list = g_malloc0(sizeof(GuestAgentCommandInfoList));
> +cmd_info_list->value = cmd_info;
> +cmd_info_list->next = info->supported_commands;
> +info->supported_commands = cmd_info_list;
> +}
>
> -g_free(*cmd_list);
> -cmd_list++;
> -}
> +struct GuestAgentInfo *qmp_guest_info(Error **err)
> +{
> +GuestAgentInfo *info = g_malloc0(sizeof(GuestAgentInfo));
>
> -out:
> -g_f
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
On 9 Oct 2013, at 17:26, Paolo Bonzini wrote: > I think Hans already debugged it to the (supposedly) 33 Hz timer that > spice audio uses. > > If it turns out the bug is in the QEMU part of spice, I think it makes > sense _not_ to include this patch at all. > > If it turns out to be in spice itself, then we can include it as a > workaround, but still it would be nice to tune a bit the limit and not > burn even more CPU time than before. Or we could simply clamp the value passed to timer_mod in spice.c which should have the same effect. -- Alex Bligh
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Il 09/10/2013 18:19, Alex Bligh ha scritto: >> Do you also agree that the equivalent workaround, before >> > Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)? > I don't think this was the case, as if it's a timer constantly > expiring we'd have seen select() exit as soon as it was entered > by the fd poked by the signal. The signal itself was clamped to be at least 250 microseconds... > That might be far more frequent. ... it's true though that it could have been less than 250 microseconds (more precisely, 250 microseconds minus the time from qemu_mod_timer_ns to select). Since the CPU usage with Hans's patch is 100% and used to be 50%, it was also more than 1 ns that Hans's patch is using. > I think the equivalent would be something like: if the 'zero' > timeout comes from the deadline calculation (and not > nonblocking=true) then release the lock anyway. I think > that would be a reasonable approach. > > I would however like to get to the bottom of what's causing > this as even pre my changes playing sound was apparently taking > 50% CPU, which is not good. I am completely packed until the > weekend but I propose producing a timer debug patch which > will instrument what is expiring constantly (unless the > problem with spice is obvious to someone). I think Hans already debugged it to the (supposedly) 33 Hz timer that spice audio uses. If it turns out the bug is in the QEMU part of spice, I think it makes sense _not_ to include this patch at all. If it turns out to be in spice itself, then we can include it as a workaround, but still it would be nice to tune a bit the limit and not burn even more CPU time than before. Paolo
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
On 9 Oct 2013, at 15:37, Paolo Bonzini wrote: > > I agree. Do you also agree that the equivalent workaround, before > Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)? I don't think this was the case, as if it's a timer constantly expiring we'd have seen select() exit as soon as it was entered by the fd poked by the signal. That might be far more frequent. I think the equivalent would be something like: if the 'zero' timeout comes from the deadline calculation (and not nonblocking=true) then release the lock anyway. I think that would be a reasonable approach. I would however like to get to the bottom of what's causing this as even pre my changes playing sound was apparently taking 50% CPU, which is not good. I am completely packed until the weekend but I propose producing a timer debug patch which will instrument what is expiring constantly (unless the problem with spice is obvious to someone). -- Alex Bligh
Re: [Qemu-devel] [PATCH 0/4] qdev: switch reset to post-order, clean up PCI reset
Il 06/10/2013 22:34, Paolo Bonzini ha scritto: > Il 06/10/2013 20:28, Michael S. Tsirkin ha scritto: > For each PCI device I tried creating a VM with an instance of it (a few > devices at a time), and did VM resets. Earlier versions were tested by > the guy who reported the SCSI problems. x86 kvm only? >>> >>> Yes. >> >> Hmm, I'm not sure that's enough for this kind of change. > > I'll do more tests though, from looking at the source code, I'm not sure > what could happen depending on the host bridge. Did more tests, PPC g3beige and PPC64 mac99 both work. I also tested resetting the secondary bus of a PCI bridge (via setpci), and it also works as expected. Finally, I looked more at the history of the code to justify patch 2. Initially, zeroing the irq_state was added in commit 6eaa684 (Add pci_bus_reset() function., 2009-06-17) to deal with this issue: >> Shouldn't each device's reset function bring its line low, thus zeroing >> the irq_state naturally? >> >> If not, we have a bug somewhere. Note we have exactly the same issue >> with save/restore. >> > They should, but I haven't found one that does. More registers were then cleared by pci_device_reset in your commit c0b1905 (qemu/pci: reset device registers on bus reset, 2009-09-16). Deasserting interrupts explicitly came in later as part of PCI bus and FLR support in commit 4c92325 (pci: deassert intx on reset., 2011-01-20). That should be the point where the code starts following the invariant of patch 2. Paolo
Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier
Hi,
On 10/09/2013 03:35 PM, Gerd Hoffmann wrote:
Hi,
Assuming we have a device with multiple configurations, each
configuration has a different set of interfaces, guest switches from one
config to another. Do we correctly unbind kernel / claim interfaces
then?
Yes we still have a usb_host_detach_kernel() call in the beginning
of usb_host_claim_interfaces(), which gets run on the new interfaces
immediately after the libusb_set_configuration call.
Ok, good.
Doing a detach_kernel after a set_config has always been necessary,
since the kernel will automatically bind drivers to the new interfaces
if the set_config succeeds.
Is there some way to avoid the kernel's autobind in the first place btw?
Not atm, but so far the kernel guys have been open to adding (sane) API's
for things like this, and avoiding the whole re-bind after a set_config from
userspace would probably be nice to have.
Note that this is not triggering in 99% of all cases though, as the kernel has
this bit of code in its set_config handling for usbfs:
/* SET_CONFIGURATION is often abused as a "cheap" driver reset,
* so avoid usb_set_configuration()'s kick to sysfs
*/
if (actconfig && actconfig->desc.bConfigurationValue == u)
status = usb_reset_configuration(ps->dev);
else
status = usb_set_configuration(ps->dev, u);
So it only actually does a set_config (and binds the kernel drivers to
the interfaces) if the guest is asking for another config then the host os
has chosen for the device.
Since the guest assumes the device starts unconfigured, it does not issue
a set_config(0), only a set_config(1) (usually) which the above code
turns into a light weight reset.
Note that my usbredirhost code avoids even the unclaim / claim dance around
set_config and calling into the kernel at all in this case, it has:
if (host->config &&
host->config->bConfigurationValue == config) {
goto exit;
}
A downside of this is that not even the lightweight device reset is done,
but the guest always starts with a full device-reset, immediately following
that with a lightweight reset has little value.
I think we could and should to the same in host-libusb.c.
Regards,
Hans
[Qemu-devel] [PULL 1/2] block/iscsi: reenable iscsi_co_get_block_status
From: Peter Lieven
Commit f35c934a accidently disabled iscsi_co_get_block_status for all
libiscsi versions. Its not possible to check for enumeration constants
in the C preprocessor. This patch changes the check to the preprocessor
constant LIBISCSI_FEATURE_IOVECTOR which was introduced shortly after
get_lba_status support was added to libiscsi.
Signed-off-by: Peter Lieven
Signed-off-by: Paolo Bonzini
---
block/iscsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 6152ef1..a2a961e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -811,7 +811,7 @@ iscsi_getlength(BlockDriverState *bs)
return len;
}
-#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
@@ -903,7 +903,7 @@ out:
return ret;
}
-#endif /* SCSI_PROVISIONING_TYPE_DEALLOCATED */
+#endif /* LIBISCSI_FEATURE_IOVECTOR */
static int
coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num,
@@ -1529,7 +1529,7 @@ static BlockDriver bdrv_iscsi = {
.bdrv_getlength = iscsi_getlength,
.bdrv_truncate = iscsi_truncate,
-#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
.bdrv_co_get_block_status = iscsi_co_get_block_status,
#endif
.bdrv_co_discard = iscsi_co_discard,
--
1.8.3.1
[Qemu-devel] [PULL 2/2] scsi: Allocate SCSITargetReq r->buf dynamically [CVE-2013-4344]
From: Asias He
r->buf is hardcoded to 2056 which is (256 + 1) * 8, allowing 256 luns at
most. If more than 256 luns are specified by user, we have buffer
overflow in scsi_target_emulate_report_luns.
To fix, we allocate the buffer dynamically.
Signed-off-by: Asias He
Tested-by: Michael Roth
Signed-off-by: Paolo Bonzini
---
hw/scsi/scsi-bus.c | 45 ++---
include/hw/scsi/scsi.h | 2 ++
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4d36841..24ec52f 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -11,6 +11,8 @@ static char *scsibus_get_dev_path(DeviceState *dev);
static char *scsibus_get_fw_dev_path(DeviceState *dev);
static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
static void scsi_req_dequeue(SCSIRequest *req);
+static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
+static void scsi_target_free_buf(SCSIRequest *req);
static Property scsi_props[] = {
DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
@@ -317,7 +319,8 @@ typedef struct SCSITargetReq SCSITargetReq;
struct SCSITargetReq {
SCSIRequest req;
int len;
-uint8_t buf[2056];
+uint8_t *buf;
+int buf_len;
};
static void store_lun(uint8_t *outbuf, int lun)
@@ -361,14 +364,12 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq
*r)
if (!found_lun0) {
n += 8;
}
-len = MIN(n + 8, r->req.cmd.xfer & ~7);
-if (len > sizeof(r->buf)) {
-/* TODO: > 256 LUNs? */
-return false;
-}
+scsi_target_alloc_buf(&r->req, n + 8);
+
+len = MIN(n + 8, r->req.cmd.xfer & ~7);
memset(r->buf, 0, len);
-stl_be_p(&r->buf, n);
+stl_be_p(&r->buf[0], n);
i = found_lun0 ? 8 : 16;
QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
DeviceState *qdev = kid->child;
@@ -387,6 +388,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq
*r)
static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
{
assert(r->req.dev->lun != r->req.lun);
+
+scsi_target_alloc_buf(&r->req, SCSI_INQUIRY_LEN);
+
if (r->req.cmd.buf[1] & 0x2) {
/* Command support data - optional, not implemented */
return false;
@@ -411,7 +415,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
return false;
}
/* done with EVPD */
-assert(r->len < sizeof(r->buf));
+assert(r->len < r->buf_len);
r->len = MIN(r->req.cmd.xfer, r->len);
return true;
}
@@ -422,7 +426,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
}
/* PAGE CODE == 0 */
-r->len = MIN(r->req.cmd.xfer, 36);
+r->len = MIN(r->req.cmd.xfer, SCSI_INQUIRY_LEN);
memset(r->buf, 0, r->len);
if (r->req.lun != 0) {
r->buf[0] = TYPE_NO_LUN;
@@ -455,8 +459,9 @@ static int32_t scsi_target_send_command(SCSIRequest *req,
uint8_t *buf)
}
break;
case REQUEST_SENSE:
+scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
r->len = scsi_device_get_sense(r->req.dev, r->buf,
- MIN(req->cmd.xfer, sizeof r->buf),
+ MIN(req->cmd.xfer, r->buf_len),
(req->cmd.buf[1] & 1) == 0);
if (r->req.dev->sense_is_ua) {
scsi_device_unit_attention_reported(req->dev);
@@ -501,11 +506,29 @@ static uint8_t *scsi_target_get_buf(SCSIRequest *req)
return r->buf;
}
+static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len)
+{
+SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+r->buf = g_malloc(len);
+r->buf_len = len;
+
+return r->buf;
+}
+
+static void scsi_target_free_buf(SCSIRequest *req)
+{
+SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+g_free(r->buf);
+}
+
static const struct SCSIReqOps reqops_target_command = {
.size = sizeof(SCSITargetReq),
.send_command = scsi_target_send_command,
.read_data= scsi_target_read_data,
.get_buf = scsi_target_get_buf,
+.free_req = scsi_target_free_buf,
};
@@ -1365,7 +1388,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
buf[7] = 10;
buf[12] = sense.asc;
buf[13] = sense.ascq;
-return MIN(len, 18);
+return MIN(len, SCSI_SENSE_LEN);
} else {
/* Return descriptor format sense buffer */
buf[0] = 0x72;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1b66510..76f6ac2 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -9,6 +9,8 @@
#define MAX_SCSI_DEVS 255
#define SCSI_CMD_BUF_SIZE 16
+#define SCSI_SENSE_LEN 18
+#define SCSI_INQUIRY_LEN36
typedef struct SCSIBus SCSIBus;
typedef struct SCSIBusInfo SCSIBusInfo;
--
1.8.3.1
[Qemu-devel] [PULL v2 0/2] SCSI patches for 2013-10-09
Anthony, The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging (2013-09-30 17:15:27 -0500) are available in the git repository at: git://github.com/bonzini/qemu.git scsi-next for you to fetch changes up to 846424350b292f16b732b573273a5c1f195cd7a3: scsi: Allocate SCSITargetReq r->buf dynamically [CVE-2013-4344] (2013-10-09 10:43:52 +0200) Paolo Asias He (1): scsi: Allocate SCSITargetReq r->buf dynamically [CVE-2013-4344] Peter Lieven (1): block/iscsi: reenable iscsi_co_get_block_status v1->v2: include CVE number block/iscsi.c | 6 +++--- hw/scsi/scsi-bus.c | 45 ++--- include/hw/scsi/scsi.h | 2 ++ 3 files changed, 39 insertions(+), 14 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/08/2013 08:42 PM, Eduardo Otubo wrote:
v3: The "-netdev tap" option is checked in the vl.c file during the
process of the command line argument list. It sets tap_enabled to true
or false according to the configuration found. Later at the seccomp
filter installation, this value is checked wheter to install or not this
feature.
Adding a system call blacklist right before the vcpus starts. This
filter is composed by the system calls that can't be executed after the
guests are up. This list should be refined as whitelist is, with as much
testing as we can do using virt-test.
Signed-off-by: Eduardo Otubo
---
include/sysemu/seccomp.h | 6 -
qemu-seccomp.c | 64 +++-
vl.c | 21 +++-
3 files changed, 77 insertions(+), 14 deletions(-)
diff --git a/include/sysemu/seccomp.h b/include/sysemu/seccomp.h
index 1189fa2..9dc7e52 100644
--- a/include/sysemu/seccomp.h
+++ b/include/sysemu/seccomp.h
@@ -15,8 +15,12 @@
#ifndef QEMU_SECCOMP_H
#define QEMU_SECCOMP_H
+#define WHITELIST 0
+#define BLACKLIST 1
+
#include
#include "qemu/osdep.h"
-int seccomp_start(void);
+int seccomp_start(int list_type);
+
#endif
diff --git a/qemu-seccomp.c b/qemu-seccomp.c
index 37d38f8..84a42bc 100644
--- a/qemu-seccomp.c
+++ b/qemu-seccomp.c
@@ -21,7 +21,7 @@ struct QemuSeccompSyscall {
uint8_t priority;
};
-static const struct QemuSeccompSyscall seccomp_whitelist[] = {
+static const struct QemuSeccompSyscall whitelist[] = {
{ SCMP_SYS(timer_settime), 255 },
{ SCMP_SYS(timer_gettime), 254 },
{ SCMP_SYS(futex), 253 },
@@ -221,32 +221,72 @@ static const struct QemuSeccompSyscall
seccomp_whitelist[] = {
{ SCMP_SYS(arch_prctl), 240 }
};
-int seccomp_start(void)
+/*
+ * The second list, called blacklist, basically reduces previously installed
+ * whitelist. All the syscalls configured by the previous whitelist are still
+ * allowed, except for the ones in the blacklist.
+ * */
+
+static const struct QemuSeccompSyscall blacklist[] = {
+{ SCMP_SYS(execve), 255 }
+};
+
+static int process_list(scmp_filter_ctx *ctx,
+const struct QemuSeccompSyscall *list,
+unsigned int list_size, uint32_t action)
{
int rc = 0;
unsigned int i = 0;
-scmp_filter_ctx ctx;
-ctx = seccomp_init(SCMP_ACT_KILL);
-if (ctx == NULL) {
-goto seccomp_return;
-}
+for (i = 0; i < list_size; i++) {
+rc = seccomp_rule_add(ctx, action, list[i].num, 0);
+if (rc < 0) {
+goto seccomp_return;
+}
-for (i = 0; i < ARRAY_SIZE(seccomp_whitelist); i++) {
-rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num,
0);
+rc = seccomp_syscall_priority(ctx, list[i].num,
+ list[i].priority);
if (rc < 0) {
goto seccomp_return;
}
-rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num,
- seccomp_whitelist[i].priority);
+}
+
+seccomp_return:
+return rc;
+}
+
+int seccomp_start(int list_type)
+{
+int rc = 0;
+scmp_filter_ctx ctx;
+
+switch (list_type) {
+case WHITELIST:
+ctx = seccomp_init(SCMP_ACT_KILL);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, whitelist, ARRAY_SIZE(whitelist),
SCMP_ACT_ALLOW);
if (rc < 0) {
goto seccomp_return;
}
+break;
+case BLACKLIST:
+ctx = seccomp_init(SCMP_ACT_ALLOW);
+if (ctx == NULL) {
+goto seccomp_return;
+}
+rc = process_list(ctx, blacklist, ARRAY_SIZE(blacklist),
SCMP_ACT_KILL);
+break;
+default:
+rc = -1;
+goto seccomp_return;
}
rc = seccomp_load(ctx);
seccomp_return:
-seccomp_release(ctx);
+if (ctx)
+seccomp_release(ctx);
return rc;
}
diff --git a/vl.c b/vl.c
index b4b119a..ee95674 100644
--- a/vl.c
+++ b/vl.c
@@ -179,6 +179,8 @@ int main(int argc, char **argv)
#define MAX_VIRTIO_CONSOLES 1
#define MAX_SCLP_CONSOLES 1
+static bool enable_blacklist = false;
+static bool tap_enabled = false;
static const char *data_dir[16];
static int data_dir_idx;
const char *bios_name = NULL;
@@ -1033,7 +1035,7 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
/* FIXME: change this to true for 1.3 */
if (qemu_opt_get_bool(opts, "enable", false)) {
#ifdef CONFIG_SECCOMP
-if (seccomp_start() < 0) {
+if (seccomp_start(WHITELIST) < 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
"failed to install seccomp syscall filter in the
kernel");
return -1;
@@ -1765,12 +1767,24 @@ void vm_state_notify(int running, RunState state)
}
}
+static void install_seccomp_blacklist(void)
+{
+if (enable_blacklist
Re: [Qemu-devel] [PATCH] Ensure PCIR is aligned to 4 bytes
Am 09.10.2013 03:56, schrieb Brad Smith: > On 25/09/13 7:24 PM, Brad Smith wrote: >> On 21/09/13 12:38 PM, Sebastian Herbszt wrote: >>> Brad Smith wrote: On 19/09/13 12:53 PM, Sebastian Herbszt wrote: > Brad Smith wrote: >> On 20/01/13 1:12 PM, David Woodhouse wrote: >>> The PCI Firmware Specification apparently requires that the PCI >>> Data Structure be DWORD-aligned. The implementation in OVMF also >>> requires this, so vgabios ROMs don't work there. With this fixed, >>> I can now initialise the VGA ROM from EFI, and EFI can display >>> using INT 10h services. >>> >>> --- vgabios-0.6c/vgabios.c.orig2013-01-20 >>> 11:33:36.138548472 -0600 +++ vgabios-0.6c/vgabios.c >>> 2013-01-20 11:36:26.060270163 -0600 @@ -204,6 +204,7 @@ >>> vgabios_website: .byte0x00 >>> >>> #ifdef PCIBIOS >>> +.align 4 // DWORD alignment required by PCI Firmware >>> Specification vgabios_pci_data: >>> .ascii "PCIR" >>> #ifdef CIRRUS >> >> We have had this in the OpenBSD port of QEMU for awhile now. Is it >> possible to have this reviewed and commited? > > This change was commited to upstream vgabios back in February [1]. But that has not resulted in it being brought into QEMU. >>> >>> Gerd, Anthony, care to update QEMU's vgabios repository [1] with >>> changes from upstream CVS repository [2]? >>> >>> [1] http://git.qemu.org/?p=vgabios.git >>> [2] http://cvs.savannah.gnu.org/viewvc/?root=vgabios >> >> Any comment? > > ping. Same for http://patchwork.ozlabs.org/patch/273758/ - vgabios seems to be falling through the cracks... Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PULL 0/6] VFIO updates for QEMU
On Wed, 2013-10-09 at 07:54 -0700, Anthony Liguori wrote: > Alex Williamson writes: > > > The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: > > > > Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging > > (2013-09-30 17:15:27 -0500) > > > > are available in the git repository at: > > > > > > git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-20131003.0 > > > > for you to fetch changes up to 1d5bf692e55ae22b59083741d521e27db704846d: > > > > vfio: Fix debug output for int128 values (2013-10-03 09:10:09 -0600) > > > > > > Judging from the review comments, I think this needs a v2. Yep, I've already posted the fixes for review, I'll re-tag with them appended to the end for a v2 request. Thanks, Alex > > vfio-pci updates include: > > - Forgotten MSI affinity patch posted several months ago > > - Lazy option ROM loading to delay load until after device/bus resets > > - Error reporting cleanups > > - PCI hot reset support introduced with Linux v3.12 development kernels > > - Debug build fix for int128 > > > > The lazy ROM loading and hot reset should help VGA assignment as we can > > now do a bus reset when there are multiple devices on the bus, ex. > > multi-function graphics and audio cards. The known remaining part for > > VGA is the KVM-VFIO device and matching QEMU support to properly handle > > devices that make use of No-Snoop transactions, particularly on Intel > > host systems. > > > > > > Alex Williamson (5): > > vfio-pci: Add support for MSI affinity > > vfio-pci: Test device reset capabilities > > vfio-pci: Lazy PCI option ROM loading > > vfio-pci: Cleanup error_reports > > vfio-pci: Implement PCI hot reset > > > > Alexey Kardashevskiy (1): > > vfio: Fix debug output for int128 values > > > > hw/misc/vfio.c | 621 > > +++-- > > 1 file changed, 512 insertions(+), 109 deletions(-) > -- > 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
Re: [Qemu-devel] [PULL 0/6] VFIO updates for QEMU
Alex Williamson writes: > The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: > > Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging > (2013-09-30 17:15:27 -0500) > > are available in the git repository at: > > > git://github.com/awilliam/qemu-vfio.git tags/vfio-pci-for-qemu-20131003.0 > > for you to fetch changes up to 1d5bf692e55ae22b59083741d521e27db704846d: > > vfio: Fix debug output for int128 values (2013-10-03 09:10:09 -0600) > > Judging from the review comments, I think this needs a v2. Regards, Anthony Liguori > vfio-pci updates include: > - Forgotten MSI affinity patch posted several months ago > - Lazy option ROM loading to delay load until after device/bus resets > - Error reporting cleanups > - PCI hot reset support introduced with Linux v3.12 development kernels > - Debug build fix for int128 > > The lazy ROM loading and hot reset should help VGA assignment as we can > now do a bus reset when there are multiple devices on the bus, ex. > multi-function graphics and audio cards. The known remaining part for > VGA is the KVM-VFIO device and matching QEMU support to properly handle > devices that make use of No-Snoop transactions, particularly on Intel > host systems. > > > Alex Williamson (5): > vfio-pci: Add support for MSI affinity > vfio-pci: Test device reset capabilities > vfio-pci: Lazy PCI option ROM loading > vfio-pci: Cleanup error_reports > vfio-pci: Implement PCI hot reset > > Alexey Kardashevskiy (1): > vfio: Fix debug output for int128 values > > hw/misc/vfio.c | 621 > +++-- > 1 file changed, 512 insertions(+), 109 deletions(-)
[Qemu-devel] [PULL 2/2] scsi: Allocate SCSITargetReq r->buf dynamically
From: Asias He
r->buf is hardcoded to 2056 which is (256 + 1) * 8, allowing 256 luns at
most. If more than 256 luns are specified by user, we have buffer
overflow in scsi_target_emulate_report_luns.
To fix, we allocate the buffer dynamically.
Signed-off-by: Asias He
Tested-by: Michael Roth
Signed-off-by: Paolo Bonzini
---
hw/scsi/scsi-bus.c | 45 ++---
include/hw/scsi/scsi.h | 2 ++
2 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4d36841..24ec52f 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -11,6 +11,8 @@ static char *scsibus_get_dev_path(DeviceState *dev);
static char *scsibus_get_fw_dev_path(DeviceState *dev);
static int scsi_req_parse(SCSICommand *cmd, SCSIDevice *dev, uint8_t *buf);
static void scsi_req_dequeue(SCSIRequest *req);
+static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len);
+static void scsi_target_free_buf(SCSIRequest *req);
static Property scsi_props[] = {
DEFINE_PROP_UINT32("channel", SCSIDevice, channel, 0),
@@ -317,7 +319,8 @@ typedef struct SCSITargetReq SCSITargetReq;
struct SCSITargetReq {
SCSIRequest req;
int len;
-uint8_t buf[2056];
+uint8_t *buf;
+int buf_len;
};
static void store_lun(uint8_t *outbuf, int lun)
@@ -361,14 +364,12 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq
*r)
if (!found_lun0) {
n += 8;
}
-len = MIN(n + 8, r->req.cmd.xfer & ~7);
-if (len > sizeof(r->buf)) {
-/* TODO: > 256 LUNs? */
-return false;
-}
+scsi_target_alloc_buf(&r->req, n + 8);
+
+len = MIN(n + 8, r->req.cmd.xfer & ~7);
memset(r->buf, 0, len);
-stl_be_p(&r->buf, n);
+stl_be_p(&r->buf[0], n);
i = found_lun0 ? 8 : 16;
QTAILQ_FOREACH(kid, &r->req.bus->qbus.children, sibling) {
DeviceState *qdev = kid->child;
@@ -387,6 +388,9 @@ static bool scsi_target_emulate_report_luns(SCSITargetReq
*r)
static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
{
assert(r->req.dev->lun != r->req.lun);
+
+scsi_target_alloc_buf(&r->req, SCSI_INQUIRY_LEN);
+
if (r->req.cmd.buf[1] & 0x2) {
/* Command support data - optional, not implemented */
return false;
@@ -411,7 +415,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
return false;
}
/* done with EVPD */
-assert(r->len < sizeof(r->buf));
+assert(r->len < r->buf_len);
r->len = MIN(r->req.cmd.xfer, r->len);
return true;
}
@@ -422,7 +426,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
}
/* PAGE CODE == 0 */
-r->len = MIN(r->req.cmd.xfer, 36);
+r->len = MIN(r->req.cmd.xfer, SCSI_INQUIRY_LEN);
memset(r->buf, 0, r->len);
if (r->req.lun != 0) {
r->buf[0] = TYPE_NO_LUN;
@@ -455,8 +459,9 @@ static int32_t scsi_target_send_command(SCSIRequest *req,
uint8_t *buf)
}
break;
case REQUEST_SENSE:
+scsi_target_alloc_buf(&r->req, SCSI_SENSE_LEN);
r->len = scsi_device_get_sense(r->req.dev, r->buf,
- MIN(req->cmd.xfer, sizeof r->buf),
+ MIN(req->cmd.xfer, r->buf_len),
(req->cmd.buf[1] & 1) == 0);
if (r->req.dev->sense_is_ua) {
scsi_device_unit_attention_reported(req->dev);
@@ -501,11 +506,29 @@ static uint8_t *scsi_target_get_buf(SCSIRequest *req)
return r->buf;
}
+static uint8_t *scsi_target_alloc_buf(SCSIRequest *req, size_t len)
+{
+SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+r->buf = g_malloc(len);
+r->buf_len = len;
+
+return r->buf;
+}
+
+static void scsi_target_free_buf(SCSIRequest *req)
+{
+SCSITargetReq *r = DO_UPCAST(SCSITargetReq, req, req);
+
+g_free(r->buf);
+}
+
static const struct SCSIReqOps reqops_target_command = {
.size = sizeof(SCSITargetReq),
.send_command = scsi_target_send_command,
.read_data= scsi_target_read_data,
.get_buf = scsi_target_get_buf,
+.free_req = scsi_target_free_buf,
};
@@ -1365,7 +1388,7 @@ int scsi_build_sense(uint8_t *in_buf, int in_len,
buf[7] = 10;
buf[12] = sense.asc;
buf[13] = sense.ascq;
-return MIN(len, 18);
+return MIN(len, SCSI_SENSE_LEN);
} else {
/* Return descriptor format sense buffer */
buf[0] = 0x72;
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1b66510..76f6ac2 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -9,6 +9,8 @@
#define MAX_SCSI_DEVS 255
#define SCSI_CMD_BUF_SIZE 16
+#define SCSI_SENSE_LEN 18
+#define SCSI_INQUIRY_LEN36
typedef struct SCSIBus SCSIBus;
typedef struct SCSIBusInfo SCSIBusInfo;
--
1.8.3.1
[Qemu-devel] [PULL 0/2] SCSI patches for 2013-10-09
Anthony, The following changes since commit a684f3cf9b9b9c3cb82be87aafc463de8974610c: Merge remote-tracking branch 'kraxel/seabios-1.7.3.2' into staging (2013-09-30 17:15:27 -0500) are available in the git repository at: git://github.com/bonzini/qemu.git scsi-next for you to fetch changes up to 2fdbc33b44ce46a5ba8e1bc7c17766c477061413: scsi: Allocate SCSITargetReq r->buf dynamically (2013-10-09 10:43:52 +0200) Paolo Asias He (1): scsi: Allocate SCSITargetReq r->buf dynamically Peter Lieven (1): block/iscsi: reenable iscsi_co_get_block_status block/iscsi.c | 6 +++--- hw/scsi/scsi-bus.c | 45 ++--- include/hw/scsi/scsi.h | 2 ++ 3 files changed, 39 insertions(+), 14 deletions(-) -- 1.8.3.1
[Qemu-devel] [PULL 1/2] block/iscsi: reenable iscsi_co_get_block_status
From: Peter Lieven
Commit f35c934a accidently disabled iscsi_co_get_block_status for all
libiscsi versions. Its not possible to check for enumeration constants
in the C preprocessor. This patch changes the check to the preprocessor
constant LIBISCSI_FEATURE_IOVECTOR which was introduced shortly after
get_lba_status support was added to libiscsi.
Signed-off-by: Peter Lieven
Signed-off-by: Paolo Bonzini
---
block/iscsi.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 6152ef1..a2a961e 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -811,7 +811,7 @@ iscsi_getlength(BlockDriverState *bs)
return len;
}
-#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
int64_t sector_num,
@@ -903,7 +903,7 @@ out:
return ret;
}
-#endif /* SCSI_PROVISIONING_TYPE_DEALLOCATED */
+#endif /* LIBISCSI_FEATURE_IOVECTOR */
static int
coroutine_fn iscsi_co_discard(BlockDriverState *bs, int64_t sector_num,
@@ -1529,7 +1529,7 @@ static BlockDriver bdrv_iscsi = {
.bdrv_getlength = iscsi_getlength,
.bdrv_truncate = iscsi_truncate,
-#if defined(SCSI_PROVISIONING_TYPE_DEALLOCATED)
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
.bdrv_co_get_block_status = iscsi_co_get_block_status,
#endif
.bdrv_co_discard = iscsi_co_discard,
--
1.8.3.1
Re: [Qemu-devel] [PATCHv3 2/3] seccomp: adding command line support for blacklist
On 10/08/2013 09:42 PM, Eduardo Otubo wrote:
v3: The options for blacklist in the command line also checkes the
existence of "-netdev tap", leaving a warning message in a positive
case.
New command line options for the seccomp blacklist feature:
$ qemu -sandbox on[,strict=]
The strict parameter will turn on or off the new system call blacklist
Signed-off-by: Eduardo Otubo
---
qemu-options.hx | 8 +---
vl.c| 17 -
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index d15338e..05485e1 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2978,13 +2978,15 @@ Old param mode (ARM only).
ETEXI
DEF("sandbox", HAS_ARG, QEMU_OPTION_sandbox, \
-"-sandbox Enable seccomp mode 2 system call filter (default
'off').\n",
+"-sandbox Enable seccomp mode 2 system call filter (default
'off').\n"
+"-sandbox on[,strict=]\n"
+"Enable seccomp mode 2 system call second level filter (default
'off').\n",
QEMU_ARCH_ALL)
STEXI
-@item -sandbox @var{arg}
+@item -sandbox @var{arg}[,strict=@var{value}]
@findex -sandbox
Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering
and 'off' will
-disable it. The default is 'off'.
+disable it. The default is 'off'. 'strict=on' will enable second level filter
(default is 'off').
ETEXI
DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/vl.c b/vl.c
index ee95674..ffdf460 100644
--- a/vl.c
+++ b/vl.c
@@ -330,6 +330,9 @@ static QemuOptsList qemu_sandbox_opts = {
{
.name = "enable",
.type = QEMU_OPT_BOOL,
+},{
+.name = "strict",
+.type = QEMU_OPT_STRING,
},
{ /* end of list */ }
},
@@ -1032,6 +1035,7 @@ static int bt_parse(const char *opt)
static int parse_sandbox(QemuOpts *opts, void *opaque)
{
+const char *strict_value = NULL;
/* FIXME: change this to true for 1.3 */
if (qemu_opt_get_bool(opts, "enable", false)) {
#ifdef CONFIG_SECCOMP
@@ -1040,6 +1044,17 @@ static int parse_sandbox(QemuOpts *opts, void *opaque)
"failed to install seccomp syscall filter in the
kernel");
return -1;
}
+
+strict_value = qemu_opt_get(opts, "strict");
+
+if (!tap_enabled)
.--^
Just spotted tha I erased this open brace in one of my rebases.
+ if (strict_value && !strcmp(strict_value, "on")) {
+ enable_blacklist = true;
+ }
+} else {
+fprintf(stderr, "Warning: seccomp syscall second level filter \"-sandbox
on,strict=on\" "
+"cannot work together with \"-netdev tap\". Disabling
it.\n");
+}
#else
qerror_report(ERROR_CLASS_GENERIC_ERROR,
"sandboxing request but seccomp is not compiled into this
build");
@@ -1769,7 +1784,7 @@ void vm_state_notify(int running, RunState state)
static void install_seccomp_blacklist(void)
{
-if (enable_blacklist && !tap_enabled) {
+if (enable_blacklist) {
if (seccomp_start(BLACKLIST) < 0) {
qerror_report(ERROR_CLASS_GENERIC_ERROR,
"failed to install seccomp syscall second level filter
in the kernel");
--
Eduardo Otubo
IBM Linux Technology Center
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Il 09/10/2013 14:58, Hans de Goede ha scritto: > > > I still think we should add my patch in some form, since the lock > starvation > caused by timers set to expire in the past could still happen in other > cases, > esp for timer users who take a time stamp once and then add incremental > values to trigger the next run, these can get behind quite a bit if there > are latency spikes, and we don't wont to run without ever releasing the > lock while these are catching up. I agree. Do you also agree that the equivalent workaround, before Alex's patch, was MIN_REARM_TIMER_NS (and thus 250 microseconds)? Paolo
Re: [Qemu-devel] KVM Guest keymap issue
If I could help with some further testing, please send me more information and I will be glad to help. Thanks, Matej 2013/10/8 Jan Krupa : > On 10/08/2013 05:51 PM, Andreas Färber wrote: >> Hi, >> >> Am 08.10.2013 11:07, schrieb Matej Mailing: >>> the strange thing is that all other keys and combinations work except >>> those ccaron, Ccaron, scaron and Scaron, zcaron and ZCaron don't. >> >> As mentioned on IRC, my colleague had sent a patch to add ccaron, scaron >> and zcaron support for VNC: >> >> http://patchwork.ozlabs.org/patch/270143/ >> >> Looks as if that hasn't been picked up yet, other patches in that series >> have unresolved review comments. Jan? > > Sorry I was on vacation for the past few weeks. I'm back and I'll be > sending updated patch later this week. > > I also added all missing key symbols already used in all keymaps in QEMU > which should fix these problems. I'll send this in separate patch. > > Jan > >> Regards, >> Andreas >> >> P.S. Please don't top-post, that makes it harder to understand for added >> people. >> >>> In >>> our language there are many words containing those chars and I really >>> need to have them working. >>> >>> When looking at the sl keymap file, those codes, even for all other >>> chars that I type with showkey --ascii, are different than the showkey >>> outputs, but they work (except those mentioned above). >>> >>> Now I am totally confused on how could those that work, work ... >>> >>> Thanks for any enlightenments in advance :) >>> Matej >>> >>> 2013/9/26 Matej Mailing : I am still pretty lost here, also after reading your link which shed a light to many things. Every suggestion and idea is very welcome! Thanks, Matej 2013/9/24 Markus Armbruster : > Not specific to KVM, adding qemu-devel. > > Matej Mailing writes: > >> Dear list, >> >> I have a problem with a Windows XP guest that I connect to via VNC and >> is using "sl" keymap (option -k sl). >> >> The guest is Windows XP and the problematic characters are s, c and z >> with caron... when I type them via VNC, they are not printed at all in >> virtual system... I have checked the file /usr/share/kvm/keymaps/sl >> and it seems that it contains different codes than I get when doing >> showkey --ascii on the host machine (running Ubuntu 12.04). I have >> tried to change the KVM's keymap file 'sl' with the codes I get from >> showkey, but they are still not printed in virtual system to which I >> am connected via VNC... >> >> I am totally lost with this issue, thanks for your time and ideas. > > Required reading for anyone struggling with virtual keyboards: > > https://www.berrange.com/posts/2010/07/04/more-than-you-or-i-ever-wanted-to-know-about-virtual-keyboard-handling/
Re: [Qemu-devel] [PATCH] qcow2: Fix snapshot restoration in snapshot_create
Am 09.10.2013 um 14:42 hat Max Reitz geschrieben: > If the new snapshot table could not be written in qcow2_snapshot_create, > the old snapshot table has to be restored in memory and the new one > released. This should include restoration of the old snapshot count as > well, which is added by this patch. > > Signed-off-by: Max Reitz Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier
Hi, > > Assuming we have a device with multiple configurations, each > > configuration has a different set of interfaces, guest switches from one > > config to another. Do we correctly unbind kernel / claim interfaces > > then? > > Yes we still have a usb_host_detach_kernel() call in the beginning > of usb_host_claim_interfaces(), which gets run on the new interfaces > immediately after the libusb_set_configuration call. Ok, good. > Doing a detach_kernel after a set_config has always been necessary, > since the kernel will automatically bind drivers to the new interfaces > if the set_config succeeds. Is there some way to avoid the kernel's autobind in the first place btw? cheers, Gerd
Re: [Qemu-devel] [RFC 00/28] bitmap handling optimization
Il 09/10/2013 13:28, Juan Quintela ha scritto:
> Hi
>
> This series split the dirty bitmap (8 bits per page, only three used)
> into 3 individual bitmaps. Once the conversion is done, operations
> are handled by bitmap operations, not bit by bit.
>
> - *_DIRTY_FLAG flags are gone, now we use memory.h DIRTY_MEMORY_*
>everywhere.
>
> - We set/reset each flag individually
> (set_dirty_flags(0xff&~CODE_DIRTY_FLAG)) are gone.
>
> - Rename several functions to clarify/make consistent things.
>
> - I know it dont't pass checkpatch for long lines, propper submission
> should pass it. We have to have long lines, short variable names, or
> ugly line splitting :p
>
> - DIRTY_MEMORY_NUM: how can one include exec/memory.h into cpu-all.h?
> #include it don't work, as a workaround, I have copied its value, but
> any better idea? I can always create "exec/migration-flags.h", though.
I think both files are too "central" and you get some sort of circular
dependency.
The solution could be to move RAM definitions from cpu-all.h to
memory-internal.h. For example:
diff --git a/arch_init.c b/arch_init.c
index 7545d96..8752e27 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -47,7 +47,7 @@
#include "qemu/config-file.h"
#include "qmp-commands.h"
#include "trace.h"
-#include "exec/cpu-all.h"
+#include "exec/memory-internal.h"
#include "hw/acpi/acpi.h"
#ifdef DEBUG_ARCH_INIT
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 019dc20..4cfde66 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -435,57 +435,11 @@ void cpu_watchpoint_remove_all(CPUArchState *env, int
mask);
#if !defined(CONFIG_USER_ONLY)
-/* memory API */
-
extern ram_addr_t ram_size;
-
-/* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
-#define RAM_PREALLOC_MASK (1 << 0)
-
-typedef struct RAMBlock {
-struct MemoryRegion *mr;
-uint8_t *host;
-ram_addr_t offset;
-ram_addr_t length;
-uint32_t flags;
-char idstr[256];
-/* Reads can take either the iothread or the ramlist lock.
- * Writes must take both locks.
- */
-QTAILQ_ENTRY(RAMBlock) next;
-int fd;
-} RAMBlock;
-
-#define DIRTY_MEMORY_NUM 3
-
-typedef struct RAMList {
-QemuMutex mutex;
-/* Protected by the iothread lock. */
-unsigned long *dirty_memory[DIRTY_MEMORY_NUM];
-RAMBlock *mru_block;
-/* Protected by the ramlist lock. */
-QTAILQ_HEAD(, RAMBlock) blocks;
-uint32_t version;
-} RAMList;
-extern RAMList ram_list;
-
extern const char *mem_path;
extern int mem_prealloc;
-/* Flags stored in the low bits of the TLB virtual address. These are
- defined so that fast path ram access is all zeros. */
-/* Zero if TLB entry is valid. */
-#define TLB_INVALID_MASK (1 << 3)
-/* Set if TLB entry references a clean RAM page. The iotlb entry will
- contain the page physical address. */
-#define TLB_NOTDIRTY(1 << 4)
-/* Set if TLB entry is an IO callback. */
-#define TLB_MMIO(1 << 5)
-
void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
-ram_addr_t last_ram_offset(void);
-void qemu_mutex_lock_ramlist(void);
-void qemu_mutex_unlock_ramlist(void);
#endif /* !CONFIG_USER_ONLY */
int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr,
diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h
index e21cb60..719fa27 100644
--- a/include/exec/cputlb.h
+++ b/include/exec/cputlb.h
@@ -20,6 +20,17 @@
#define CPUTLB_H
#if !defined(CONFIG_USER_ONLY)
+
+/* Flags stored in the low bits of the TLB virtual address. These are
+ defined so that fast path ram access is all zeros. */
+/* Zero if TLB entry is valid. */
+#define TLB_INVALID_MASK (1 << 3)
+/* Set if TLB entry references a clean RAM page. The iotlb entry will
+ contain the page physical address. */
+#define TLB_NOTDIRTY(1 << 4)
+/* Set if TLB entry is an IO callback. */
+#define TLB_MMIO(1 << 5)
+
/* cputlb.c */
void tlb_protect_code(ram_addr_t ram_addr);
void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index d46570e..aaf76c2 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -22,6 +22,35 @@
#ifndef CONFIG_USER_ONLY
#include "hw/xen/xen.h"
+/* RAM is pre-allocated and passed into qemu_ram_alloc_from_ptr */
+#define RAM_PREALLOC_MASK (1 << 0)
+
+typedef struct RAMBlock {
+struct MemoryRegion *mr;
+uint8_t *host;
+ram_addr_t offset;
+ram_addr_t length;
+uint32_t flags;
+char idstr[256];
+/* Reads can take either the iothread or the ramlist lock.
+ * Writes must take both locks.
+ */
+QTAILQ_ENTRY(RAMBlock) next;
+int fd;
+} RAMBlock;
+
+#define DIRTY_MEMORY_NUM 3
+
+typedef struct RAMList {
+QemuMutex mutex;
+/* Protected by the iothread lock. */
+unsigned long *dirty_memory[DIRTY_MEMORY_NUM];
+RAMBlock *mru_block;
+/* Protected by the ramlist lo
Re: [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros
On 2013-10-09 15:07, Kevin Wolf wrote:
Am 20.09.2013 um 10:37 hat Max Reitz geschrieben:
Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to
the already existing QCOW2_OL_CACHED, signifying all metadata overlap
checks that can be performed in constant time (regardless of image size
etc.) and truly all available overlap checks, respectively.
Signed-off-by: Max Reitz
---
block/qcow2.h | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 067ed2f..098c14f 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap {
QCOW2_OL_INACTIVE_L2= (1 << QCOW2_OL_INACTIVE_L2_BITNR),
} QCow2MetadataOverlap;
+/* Perform all overlap checks which can be done in constant time */
+#define QCOW2_OL_CONSTANT \
+(QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_REFCOUNT_TABLE | \
+ QCOW2_OL_SNAPSHOT_TABLE)
+
/* Perform all overlap checks which don't require disk access */
#define QCOW2_OL_CACHED \
-(QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \
- QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \
- QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1)
+(QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \
+ QCOW2_OL_SNAPSHOT_TABLE)
QCOW2_OL_INACTIVE_L1 is lost here.
Oh, right, I'll fix it. Thanks for reviewing, by the way. :)
Max
+/* Perform all overlap checks */
+#define QCOW2_OL_ALL \
+(QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2)
Kevin
Re: [Qemu-devel] [PATCH] qcow2: Add missing space in error message
Am 09.10.2013 um 14:40 hat Max Reitz geschrieben: > The error message in qcow2_downgrade about an unsupported refcount > order is missing a space. This patch adds it. > > Signed-off-by: Max Reitz Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] qcow2: Use better type for numerical snapshot ID
Am 09.10.2013 um 14:42 hat Max Reitz geschrieben: > When trying to find a new snapshot ID, the existing ones are converted > to integers using strtoul. This function returns an unsigned long, > therefore its result should be saved in an unsigned long as well. > > Signed-off-by: Max Reitz Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] qcow2: Remove wrong metadata overlap check
Am 09.10.2013 um 14:41 hat Max Reitz geschrieben: > In qcow2_write_compressed, if the compression fails, a normal cluster is > written to disk. This is done through bdrv_write on the qcow2 BDS > itself (using the guest offset), thus it is wrong to do a metadata > overlap check before. > > Signed-off-by: Max Reitz Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
On 9 Oct 2013, at 13:58, Hans de Goede wrote:
> I got an off-list email this morning from a kind soul informing me that
> he was seeing the same issue as me, but only when using audio over
> spice. I've just tried things with "-device hda -device hda-duplex"
> removed from my qemu cmdline and I can confirm this is the same for me.
>
> There seem to be more cases which trigger the timer set to expire in the
> past case, I added a printf("bingo\n"); to my code for catching this
> case and still unlocking the lock there, and it triggers a couple of
> times before the hda device becomes active too.
OK, that's useful.
> As soon as pulseaudio starts in the guest the screen really starts
> filling with bingos though, so it definitely seems a spice + audio problem,
>
> Normally the bingo's stop scrolling by after a couple of seconds (as pulse
> audio stops the audio device when there is no sound), but when I try to play
> music they keep scrolling.
>
> Also playing music in git master (with my patch) makes qemu take 100% host
> cpu,
> while doing the same using qemu-1.5.3 takes 50% host cpu, so there definetily
> is something wrong here.
Possibly in both cases!
> Looking at the spice code in question, we have the spice-server code
> setting a timer to go of every MM_TIMER_GRANULARITY_MS ms, which is every
> 33 ms, so not often at all.
>
> Spice server does this by calling ui/spice-core.c timer_start() from its
> timer function, which does:
>
> qemu_mod_timer(timer->timer, qemu_get_clock_ms(rt_clock) + ms);
qemu_mod_timer does not exist in master.
This line is now:
timer_mod(timer->timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + ms);
This suggests to me either timer->scale is not set to 100 (i.e.
it isn't in milliseconds), OR ms is zero/negative.
Or of course there might be a bug in the timer code.
> Note how it goes from the current time, not from some saved time, so even
> if we've missed a couple of timer ticks it should catch up with the
> current time after running once.
Sure. I would guess what's happening is that the timer is modified such
that it is already expired.
Alex
>
> So long story short I've no idea what is going on here, and Alex is right
> that there is a bigger underlying problem.
--
Alex Bligh
Re: [Qemu-devel] [RFC] map 64-bit PCI devices after all possible RAM
On Mi, 2013-10-09 at 14:23 +0200, Igor Mammedov wrote: > I'm posting it to get an oppinion on one of possible approaches > on where to map a hotplug memory. > > This patch assumes that a space for hotplug memory is located right > after RamSizeOver4G region and QEMU will provide romfile to specify > where it ends so that BIOS could know from what base to start > 64-bit PCI devices mapping. We should think about both pci hotplug and memory hotplug while being at it. Today the 64bit pci window is mapped right above high memory and is sized (in acpi tables) according to what is needed to map the devices present at boot. Effect is that there is no extra address space for 64bit bars of hotplugged pci devices. And the window is also in the way when it comes to memory hotplug. Given that some windows versions don't like the large 64bit windows we should make the window size configurable. The window location can either be made configurable too, or we simply place it at the top of the address space, with "address space" being what the cpu can address according to cpuinfo. Current qemu reports this by default: $ cat /proc/cpuinfo model name: QEMU Virtual CPU version 1.5.3 address sizes : 40 bits physical, 48 bits virtual 40 address lines allow 1TB, so we would place the window just below 1TB. Comments? Gerd
Re: [Qemu-devel] [PATCHv3 1/3] seccomp: adding blacklist support
On 10/08/2013 11:05 PM, Eric Blake wrote: On 10/08/2013 06:42 PM, Eduardo Otubo wrote: v3: The "-netdev tap" option is checked in the vl.c file during the process of the command line argument list. It sets tap_enabled to true or false according to the configuration found. Later at the seccomp filter installation, this value is checked wheter to install or not this s/wheter/whether/ Thank you. feature. Adding a system call blacklist right before the vcpus starts. This filter is composed by the system calls that can't be executed after the guests are up. This list should be refined as whitelist is, with as much testing as we can do using virt-test. Signed-off-by: Eduardo Otubo --- include/sysemu/seccomp.h | 6 - qemu-seccomp.c | 64 +++- vl.c | 21 +++- 3 files changed, 77 insertions(+), 14 deletions(-) No review on the actual patch, just spotting a typo. -- Eduardo Otubo IBM Linux Technology Center
Re: [Qemu-devel] [PATCH 03/13] usb-host-libusb: Detach kernel drivers earlier
Hi,
On 10/09/2013 10:55 AM, Gerd Hoffmann wrote:
On Di, 2013-10-08 at 21:58 +0200, Hans de Goede wrote:
If we detach the kernel drivers on the first set_config, then they will
be still attached when the device gets its initial reset. Causing the drivers
to re-initialize the device after the reset, dirtying the device state.
@@ -1051,7 +1055,6 @@ static void usb_host_set_config(USBHostDevice *s, int
config, USBPacket *p)
trace_usb_host_set_config(s->bus_num, s->addr, config);
usb_host_release_interfaces(s);
-usb_host_detach_kernel(s);
rc = libusb_set_configuration(s->dh, config);
if (rc != 0) {
usb_host_libusb_error("libusb_set_configuration", rc);
Sure we can safely remove the detach_kernel here?
Yes.
Assuming we have a device with multiple configurations, each
configuration has a different set of interfaces, guest switches from one
config to another. Do we correctly unbind kernel / claim interfaces
then?
Yes we still have a usb_host_detach_kernel() call in the beginning
of usb_host_claim_interfaces(), which gets run on the new interfaces
immediately after the libusb_set_configuration call.
Doing a detach_kernel after a set_config has always been necessary,
since the kernel will automatically bind drivers to the new interfaces
if the set_config succeeds.
Doing detach_kernel before set_config is necessary if kernel drivers
are attached, because the kernel will not allow a set_config if any
drivers are bound. But we already do a set_config for the current
config on usb_host_open() now, and if a new config gets installed we
immediately detach the kernel drivers again from usb_host_claim_interfaces()
so when we enter usb_host_set_config no kernel drivers, other then
usbfs (the userspace access driver) will be attached, and usbfs is
detached by releasing the interfaces (*).
Regards,
Hans
*) In recent libusb versions libusb will even disallow using
libusb_detach_kernel_driver to detach usbfs, so as to stop a user space
program from breaking anothers userspaces program claim on the interface
by completelty detaching usbfs from the interface.
Re: [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros
Am 20.09.2013 um 10:37 hat Max Reitz geschrieben:
> Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to
> the already existing QCOW2_OL_CACHED, signifying all metadata overlap
> checks that can be performed in constant time (regardless of image size
> etc.) and truly all available overlap checks, respectively.
>
> Signed-off-by: Max Reitz
> ---
> block/qcow2.h | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 067ed2f..098c14f 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap {
> QCOW2_OL_INACTIVE_L2= (1 << QCOW2_OL_INACTIVE_L2_BITNR),
> } QCow2MetadataOverlap;
>
> +/* Perform all overlap checks which can be done in constant time */
> +#define QCOW2_OL_CONSTANT \
> +(QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_REFCOUNT_TABLE | \
> + QCOW2_OL_SNAPSHOT_TABLE)
> +
> /* Perform all overlap checks which don't require disk access */
> #define QCOW2_OL_CACHED \
> -(QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \
> - QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \
> - QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1)
> +(QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \
> + QCOW2_OL_SNAPSHOT_TABLE)
QCOW2_OL_INACTIVE_L1 is lost here.
> +/* Perform all overlap checks */
> +#define QCOW2_OL_ALL \
> +(QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2)
Kevin
Re: [Qemu-devel] [PATCH] main-loop: Don't lock starve io-threads when main_loop_tlg has pending events
Hi,
On 10/08/2013 10:50 PM, Paolo Bonzini wrote:
Il 08/10/2013 22:16, Hans de Goede ha scritto:
No, it is calling main_loop_wait with nonblocking set to 0, so
normally the lock would get released. But
timerlistgroup_deadline_ns(&main_loop_tlg) is returning 0,
causing timeout_ns to be 0, and this causes the lock to not get
released.
Yes, this was my understanding of the patch as well. Before Alex's
series, this would be capped to MIN_REARM_TIMER_NS (250 us). This is
why I mentioned 250 us.
However, I agree with Alex that it looks a bit fishy and I'd like to
know what timer is it that is continuously set to expire in the past.
I got an off-list email this morning from a kind soul informing me that
he was seeing the same issue as me, but only when using audio over
spice. I've just tried things with "-device hda -device hda-duplex"
removed from my qemu cmdline and I can confirm this is the same for me.
There seem to be more cases which trigger the timer set to expire in the
past case, I added a printf("bingo\n"); to my code for catching this
case and still unlocking the lock there, and it triggers a couple of
times before the hda device becomes active too.
As soon as pulseaudio starts in the guest the screen really starts
filling with bingos though, so it definitely seems a spice + audio problem,
Normally the bingo's stop scrolling by after a couple of seconds (as pulse
audio stops the audio device when there is no sound), but when I try to play
music they keep scrolling.
Also playing music in git master (with my patch) makes qemu take 100% host cpu,
while doing the same using qemu-1.5.3 takes 50% host cpu, so there definetily
is something wrong here.
Looking at the spice code in question, we have the spice-server code
setting a timer to go of every MM_TIMER_GRANULARITY_MS ms, which is every
33 ms, so not often at all.
Spice server does this by calling ui/spice-core.c timer_start() from its
timer function, which does:
qemu_mod_timer(timer->timer, qemu_get_clock_ms(rt_clock) + ms);
Note how it goes from the current time, not from some saved time, so even
if we've missed a couple of timer ticks it should catch up with the
current time after running once.
So long story short I've no idea what is going on here, and Alex is right
that there is a bigger underlying problem.
Regards,
Hans
p.s.
I still think we should add my patch in some form, since the lock starvation
caused by timers set to expire in the past could still happen in other cases,
esp for timer users who take a time stamp once and then add incremental
values to trigger the next run, these can get behind quite a bit if there
are latency spikes, and we don't wont to run without ever releasing the
lock while these are catching up.
Re: [Qemu-devel] [PATCH v4] block: qemu-iotests for vhdx, read sample dynamic image
Am 27.09.2013 um 14:48 hat Jeff Cody geschrieben: > This adds the VHDX format to the qemu-iotests format, and adds > a read test. The test reads from an existing sample image, that > was created with Hyper-V under Windwos Server 2012. > > The image file is a 1GB dynamic image, with 32MB blocks. > > The pattern 0xa5 exists from 0MB-33MB (past a block size boundary) > > The pattern 0x96 exists from 33MB-66MB (past another block boundary, > and leaving a partial blank block) > > From 66MB-1024MB, all reads should return 0. > > Although 1GB dynamic image with 66MB of data, the bzip2'ed image > file size is only 874 bytes. > > This also adds in the IMGFMT_GENERIC flag, so r/o images can be > tested (e.g. ./check -vhdx) without failing tests that assume > r/w support. > > Signed-off-by: Jeff Cody Thanks, applied to the block branch. Kevin
[Qemu-devel] [PATCH 20/28] memory: unfold cpu_physical_memory_clear_dirty_flag() in its only user
Signed-off-by: Juan Quintela
---
include/exec/memory-internal.h | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 71f198e..d6d3537 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -86,12 +86,6 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t
addr)
cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_CODE);
}
-static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr,
- unsigned client)
-{
-clear_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
-}
-
static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
ram_addr_t length)
{
@@ -114,7 +108,7 @@ static inline void
cpu_physical_memory_mask_dirty_range(ram_addr_t start,
end = TARGET_PAGE_ALIGN(start + length);
start &= TARGET_PAGE_MASK;
for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
-cpu_physical_memory_clear_dirty_flag(addr, client);
+clear_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
}
}
--
1.8.3.1
[Qemu-devel] [PATCH 14/28] memory: use bit 2 for migration
For historical reasons it was bit 3. One there create a constant to
know the number of clients.
Signed-off-by: Juan Quintela
---
include/exec/memory.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 4bb20d0..a28c6bd 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -35,7 +35,8 @@ typedef struct MemoryRegionMmio MemoryRegionMmio;
#define DIRTY_MEMORY_VGA 0
#define DIRTY_MEMORY_CODE 1
-#define DIRTY_MEMORY_MIGRATION 3
+#define DIRTY_MEMORY_MIGRATION 2
+#define DIRTY_MEMORY_NUM 3 /* num of dirty bits */
struct MemoryRegionMmio {
CPUReadMemoryFunc *read[3];
--
1.8.3.1
[Qemu-devel] [PATCH 26/28] memory: use find_next_bit() to find dirty bits
This operation is way faster that doing it bit by bit.
Signed-off-by: Juan Quintela
---
include/exec/memory-internal.h | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 0c1dbfa..5a5bc0d 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -44,18 +44,15 @@ static inline bool cpu_physical_memory_get_dirty(ram_addr_t
start,
ram_addr_t length,
unsigned client)
{
-ram_addr_t addr, end;
+unsigned long end, page, next;
assert(client < DIRTY_MEMORY_NUM);
-end = TARGET_PAGE_ALIGN(start + length);
-start &= TARGET_PAGE_MASK;
-for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
-if (test_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]))
{
-return true;
-}
-}
-return false;
+end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
+page = start >> TARGET_PAGE_BITS;
+next = find_next_bit(ram_list.dirty_memory[client], end, page);
+
+return next < end;
}
static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
--
1.8.3.1
Re: [Qemu-devel] problems with 1G hugepages and linux 3.12-rc3
Copying Andrea, On Sun, Oct 06, 2013 at 02:47:41AM +0200, andy123 wrote: > Hi, > > as the subject states, I have some problems with 1G hugepages with > qemu(-vfio-git) on Linux 3.12-rc3. > > I start qemu like this, for example: > "/usr/bin/qemu-system-x86_64 -enable-kvm -m 1024 -mem-path /dev/hugepages > -drive file=/files/vm/arch.img,if=virtio,media=disk -monitor stdio" > where /dev/hugepages is "hugetlbfs on /dev/hugepages type hugetlbfs > (rw,relatime,mode=1770,gid=78,pagesize=1G,pagesize=1G)" > and the kernel is booted with "hugepagesz=1G hugepages=4". > This result in lots of error message in dmesg, as seen here: > https://gist.github.com/ajs124/6842823 (starting at 18:04:28) > > After starting and stopping multiple virtual machines, the hugepages seem to > "fill up" and qemu outputs > "file_ram_alloc: can't mmap RAM pages: Cannot allocate memory", but works > anyways. > With fill up, I mean that I can start qemu 2 time with "-m 2048" and 4 times > with "-m 1024", before it fails to mmap. > I can reproduce huge page leak, but not oops, but they can be related. Can you revert 11feeb498086a3a5907b8148bdf1786a9b18fc55 and retry? > This works without any problems in 3.11.x and also with 2M hugepages (umount > /dev/hugepages && mount -t hugetlbfs -o pagesize=2048k /dev/hugepages). > > I'm running this in arch linux and there has already been some discussion on > the arch forums > (https://bbs.archlinux.org/viewtopic.php?pid=1333469#p1333469) but I tried to > add everything I wrote over there in this mail. > > In case I missed something or you need more information, I'll happily supply > it in order to get this resolved. > > -- > 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 -- Gleb.
[Qemu-devel] [PATCH] qcow2: Use better type for numerical snapshot ID
When trying to find a new snapshot ID, the existing ones are converted
to integers using strtoul. This function returns an unsigned long,
therefore its result should be saved in an unsigned long as well.
Signed-off-by: Max Reitz
---
block/qcow2-snapshot.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 812dab2..5b437ce 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -292,7 +292,8 @@ static void find_new_snapshot_id(BlockDriverState *bs,
{
BDRVQcowState *s = bs->opaque;
QCowSnapshot *sn;
-int i, id, id_max = 0;
+int i;
+unsigned long id, id_max = 0;
for(i = 0; i < s->nb_snapshots; i++) {
sn = s->snapshots + i;
@@ -300,7 +301,7 @@ static void find_new_snapshot_id(BlockDriverState *bs,
if (id > id_max)
id_max = id;
}
-snprintf(id_str, id_str_size, "%d", id_max + 1);
+snprintf(id_str, id_str_size, "%lu", id_max + 1);
}
static int find_snapshot_by_id_and_name(BlockDriverState *bs,
--
1.8.3.1
[Qemu-devel] [PATCH] qcow2: Fix snapshot restoration in snapshot_create
If the new snapshot table could not be written in qcow2_snapshot_create,
the old snapshot table has to be restored in memory and the new one
released. This should include restoration of the old snapshot count as
well, which is added by this patch.
Signed-off-by: Max Reitz
---
block/qcow2-snapshot.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 812dab2..fe7e14c 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -433,6 +433,7 @@ int qcow2_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info)
if (ret < 0) {
g_free(s->snapshots);
s->snapshots = old_snapshot_list;
+s->nb_snapshots--;
goto fail;
}
--
1.8.3.1
[Qemu-devel] [PATCH] qcow2: Remove wrong metadata overlap check
In qcow2_write_compressed, if the compression fails, a normal cluster is
written to disk. This is done through bdrv_write on the qcow2 BDS
itself (using the guest offset), thus it is wrong to do a metadata
overlap check before.
Signed-off-by: Max Reitz
---
block/qcow2.c | 8
1 file changed, 8 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index d59ecbd..dc0b60e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1738,14 +1738,6 @@ static int qcow2_write_compressed(BlockDriverState *bs,
int64_t sector_num,
if (ret != Z_STREAM_END || out_len >= s->cluster_size) {
/* could not compress: write normal cluster */
-
-ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT,
-sector_num * BDRV_SECTOR_SIZE,
-s->cluster_sectors * BDRV_SECTOR_SIZE);
-if (ret < 0) {
-goto fail;
-}
-
ret = bdrv_write(bs, sector_num, buf, s->cluster_sectors);
if (ret < 0) {
goto fail;
--
1.8.3.1
[Qemu-devel] [PATCH 08/28] exec: simplify notdirty_mem_write()
We don't need to make special things for CODE, just set the other two bits
Signed-off-by: Juan Quintela
---
exec.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/exec.c b/exec.c
index 07b625f..b9f2825 100644
--- a/exec.c
+++ b/exec.c
@@ -1447,12 +1447,8 @@ found:
static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
{
-
-int dirty_flags;
-dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
if (!cpu_physical_memory_get_dirty_flag(ram_addr, CODE_DIRTY_FLAG)) {
tb_invalidate_phys_page_fast(ram_addr, size);
-dirty_flags = cpu_physical_memory_get_dirty_flags(ram_addr);
}
switch (size) {
case 1:
@@ -1467,8 +1463,8 @@ static void notdirty_mem_write(void *opaque, hwaddr
ram_addr,
default:
abort();
}
-dirty_flags |= (0xff & ~CODE_DIRTY_FLAG);
-cpu_physical_memory_set_dirty_flags(ram_addr, dirty_flags);
+cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG);
/* we remove the notdirty callback only if the code has been
flushed */
if (cpu_physical_memory_is_dirty(ram_addr)) {
--
1.8.3.1
[Qemu-devel] [PATCH 07/28] memory: make cpu_physical_memory_is_dirty return bool
Signed-off-by: Juan Quintela
---
include/exec/memory-internal.h | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 9cd2f53..eefe501 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -56,9 +56,12 @@ static inline bool
cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
}
/* read dirty bit (return 0 or 1) */
-static inline int cpu_physical_memory_is_dirty(ram_addr_t addr)
+static inline bool cpu_physical_memory_is_dirty(ram_addr_t addr)
{
-return cpu_physical_memory_get_dirty_flags(addr) == 0xff;
+bool vga = cpu_physical_memory_get_dirty_flag(addr, VGA_DIRTY_FLAG);
+bool code = cpu_physical_memory_get_dirty_flag(addr, CODE_DIRTY_FLAG);
+bool migration = cpu_physical_memory_get_dirty_flag(addr,
MIGRATION_DIRTY_FLAG);
+return vga && code && migration;
}
static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
--
1.8.3.1
[Qemu-devel] [PATCH 10/28] memory: set single dirty flags when possible
Signed-off-by: Juan Quintela
---
exec.c | 7 ---
include/exec/memory-internal.h | 4 +++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/exec.c b/exec.c
index b9f2825..0fd9c58 100644
--- a/exec.c
+++ b/exec.c
@@ -1887,7 +1887,8 @@ static void invalidate_and_set_dirty(hwaddr addr,
/* invalidate code */
tb_invalidate_phys_page_range(addr, addr + length, 0);
/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(addr, (0xff & ~CODE_DIRTY_FLAG));
+cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
}
xen_modified_memory(addr, length);
}
@@ -2467,8 +2468,8 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
/* invalidate code */
tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
/* set dirty bit */
-cpu_physical_memory_set_dirty_flags(
-addr1, (0xff & ~CODE_DIRTY_FLAG));
+cpu_physical_memory_set_dirty_flag(addr1,
MIGRATION_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr1, VGA_DIRTY_FLAG);
}
}
}
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index b72c14a..bfbfc48 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -88,7 +88,9 @@ static inline void
cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
{
-cpu_physical_memory_set_dirty_flags(addr, 0xff);
+cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr, CODE_DIRTY_FLAG);
}
static inline int cpu_physical_memory_clear_dirty_flags(ram_addr_t addr,
--
1.8.3.1
[Qemu-devel] [PATCH] qcow2: Add missing space in error message
The error message in qcow2_downgrade about an unsupported refcount
order is missing a space. This patch adds it.
Signed-off-by: Max Reitz
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index d59ecbd..1385f92 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1913,7 +1913,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int
target_version)
* support anything different than 4 anyway, there is no point in doing
* so right now; however, we should error out (if qemu supports this in
* the future and this code has not been adapted) */
-error_report("qcow2_downgrade: Image refcount orders other than 4 are"
+error_report("qcow2_downgrade: Image refcount orders other than 4 are "
"currently not supported.");
return -ENOTSUP;
}
--
1.8.3.1
[Qemu-devel] [PATCH 05/28] memory: create function to set a single dirty bit
Signed-off-by: Juan Quintela
---
cputlb.c | 2 +-
include/exec/memory-internal.h | 6 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/cputlb.c b/cputlb.c
index 19ecf60..3aaa016 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -137,7 +137,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
target_ulong vaddr)
{
-cpu_physical_memory_set_dirty_flags(ram_addr, CODE_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(ram_addr, CODE_DIRTY_FLAG);
}
static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index c71a5e6..4ebab80 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -76,6 +76,12 @@ static inline void
cpu_physical_memory_set_dirty_flags(ram_addr_t addr,
ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flags;
}
+static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
+ int dirty_flag)
+{
+ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= dirty_flag;
+}
+
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
{
cpu_physical_memory_set_dirty_flags(addr, 0xff);
--
1.8.3.1
[Qemu-devel] [PATCH 24/28] memory: cpu_physical_memory_get_dirty() is used as returning a bool
Signed-off-by: Juan Quintela
---
include/exec/memory-internal.h | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index f66d2ce..de8f279 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -40,11 +40,10 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
void qemu_ram_free(ram_addr_t addr);
void qemu_ram_free_from_ptr(ram_addr_t addr);
-static inline int cpu_physical_memory_get_dirty(ram_addr_t start,
-ram_addr_t length,
-unsigned client)
+static inline bool cpu_physical_memory_get_dirty(ram_addr_t start,
+ ram_addr_t length,
+ unsigned client)
{
-int ret = 0;
ram_addr_t addr, end;
assert(client < DIRTY_MEMORY_NUM);
@@ -52,9 +51,11 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t
start,
end = TARGET_PAGE_ALIGN(start + length);
start &= TARGET_PAGE_MASK;
for (addr = start; addr < end; addr += TARGET_PAGE_SIZE) {
-ret |= test_bit(addr >> TARGET_PAGE_BITS,
ram_list.dirty_memory[client]);
+if (test_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]))
{
+return true;
+}
}
-return ret;
+return false;
}
static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
--
1.8.3.1
[Qemu-devel] [PATCH 13/28] memory: use DIRTY_MEMORY_* instead of *_DIRTY_FLAG
Instead of the bitmap, we use the bitmap number. Once done this, we
change all names from dirty_flag to memory regions naming of client.
Signed-off-by: Juan Quintela
---
cputlb.c | 4 ++--
exec.c | 18 +-
include/exec/memory-internal.h | 38 +-
include/exec/memory.h | 3 ---
memory.c | 10 --
5 files changed, 32 insertions(+), 41 deletions(-)
diff --git a/cputlb.c b/cputlb.c
index 3aaa016..0c2ad48 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -129,7 +129,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
{
cpu_physical_memory_reset_dirty(ram_addr,
ram_addr + TARGET_PAGE_SIZE,
-CODE_DIRTY_FLAG);
+DIRTY_MEMORY_CODE);
}
/* update the TLB so that writes in physical page 'phys_addr' are no longer
@@ -137,7 +137,7 @@ void tlb_protect_code(ram_addr_t ram_addr)
void tlb_unprotect_code_phys(CPUArchState *env, ram_addr_t ram_addr,
target_ulong vaddr)
{
-cpu_physical_memory_set_dirty_flag(ram_addr, CODE_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_CODE);
}
static bool tlb_is_dirty_ram(CPUTLBEntry *tlbe)
diff --git a/exec.c b/exec.c
index f7691a0..4f8f8a2 100644
--- a/exec.c
+++ b/exec.c
@@ -678,7 +678,7 @@ static void tlb_reset_dirty_range_all(ram_addr_t start,
ram_addr_t end,
/* 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_flag)
+ unsigned client)
{
uintptr_t length;
@@ -688,7 +688,7 @@ void cpu_physical_memory_reset_dirty(ram_addr_t start,
ram_addr_t end,
length = end - start;
if (length == 0)
return;
-cpu_physical_memory_mask_dirty_range(start, length, dirty_flag);
+cpu_physical_memory_mask_dirty_range(start, length, client);
if (tcg_enabled()) {
tlb_reset_dirty_range_all(start, end, length);
@@ -1447,7 +1447,7 @@ found:
static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
uint64_t val, unsigned size)
{
-if (!cpu_physical_memory_get_dirty_flag(ram_addr, CODE_DIRTY_FLAG)) {
+if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
tb_invalidate_phys_page_fast(ram_addr, size);
}
switch (size) {
@@ -1463,8 +1463,8 @@ static void notdirty_mem_write(void *opaque, hwaddr
ram_addr,
default:
abort();
}
-cpu_physical_memory_set_dirty_flag(ram_addr, MIGRATION_DIRTY_FLAG);
-cpu_physical_memory_set_dirty_flag(ram_addr, VGA_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_MIGRATION);
+cpu_physical_memory_set_dirty_flag(ram_addr, DIRTY_MEMORY_VGA);
/* we remove the notdirty callback only if the code has been
flushed */
if (cpu_physical_memory_is_dirty(ram_addr)) {
@@ -1887,8 +1887,8 @@ static void invalidate_and_set_dirty(hwaddr addr,
/* invalidate code */
tb_invalidate_phys_page_range(addr, addr + length, 0);
/* set dirty bit */
-cpu_physical_memory_set_dirty_flag(addr, VGA_DIRTY_FLAG);
-cpu_physical_memory_set_dirty_flag(addr, MIGRATION_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_VGA);
+cpu_physical_memory_set_dirty_flag(addr, DIRTY_MEMORY_MIGRATION);
}
xen_modified_memory(addr, length);
}
@@ -2468,8 +2468,8 @@ void stl_phys_notdirty(hwaddr addr, uint32_t val)
/* invalidate code */
tb_invalidate_phys_page_range(addr1, addr1 + 4, 0);
/* set dirty bit */
-cpu_physical_memory_set_dirty_flag(addr1,
MIGRATION_DIRTY_FLAG);
-cpu_physical_memory_set_dirty_flag(addr1, VGA_DIRTY_FLAG);
+cpu_physical_memory_set_dirty_flag(addr1,
DIRTY_MEMORY_MIGRATION);
+cpu_physical_memory_set_dirty_flag(addr1, DIRTY_MEMORY_VGA);
}
}
}
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index cfe1a24..3947caa 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -40,28 +40,24 @@ void *qemu_get_ram_ptr(ram_addr_t addr);
void qemu_ram_free(ram_addr_t addr);
void qemu_ram_free_from_ptr(ram_addr_t addr);
-#define VGA_DIRTY_FLAG 0x01
-#define CODE_DIRTY_FLAG 0x02
-#define MIGRATION_DIRTY_FLAG 0x08
-
static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
- int dirty_flag)
+ unsigned client)
{
-return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & dirty_flag;
+return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & (1 << client);
}
[Qemu-devel] [RFC] map 64-bit PCI devices after all possible RAM
I'm posting it to get an oppinion on one of possible approaches
on where to map a hotplug memory.
This patch assumes that a space for hotplug memory is located right
after RamSizeOver4G region and QEMU will provide romfile to specify
where it ends so that BIOS could know from what base to start
64-bit PCI devices mapping.
Signed-off-by: Igor Mammedov
---
src/fw/pciinit.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c
index b29db99..62f8d4e 100644
--- a/src/fw/pciinit.c
+++ b/src/fw/pciinit.c
@@ -18,6 +18,8 @@
#include "paravirt.h" // RamSize
#include "string.h" // memset
#include "util.h" // pci_setup
+#include "byteorder.h" // le64_to_cpu
+#include "romfile.h" // romfile_loadint
#define PCI_DEVICE_MEM_MIN 0x1000
#define PCI_BRIDGE_IO_MIN 0x1000
@@ -764,6 +766,8 @@ static void pci_bios_map_devices(struct pci_bus *busses)
{
if (pci_bios_init_root_regions(busses)) {
struct pci_region r64_mem, r64_pref;
+u64 base64 = le64_to_cpu(romfile_loadint("etc/mem64-end",
+ 0x1ULL + RamSizeOver4G));
r64_mem.list.first = NULL;
r64_pref.list.first = NULL;
pci_region_migrate_64bit_entries(&busses[0].r[PCI_REGION_TYPE_MEM],
@@ -779,7 +783,7 @@ static void pci_bios_map_devices(struct pci_bus *busses)
u64 align_mem = pci_region_align(&r64_mem);
u64 align_pref = pci_region_align(&r64_pref);
-r64_mem.base = ALIGN(0x1LL + RamSizeOver4G, align_mem);
+r64_mem.base = ALIGN(base64, align_mem);
r64_pref.base = ALIGN(r64_mem.base + sum_mem, align_pref);
pcimem64_start = r64_mem.base;
pcimem64_end = r64_pref.base + sum_pref;
--
1.8.3.1
[Qemu-devel] [PATCH 19/28] memory: split dirty bitmap into three
After all the previous patches, spliting the bitmap gets direct.
ToDo: Why can't i include "exec/memory.h" into cpu-all.h? This is the
reason that I have duplicated DIRTY_MEMORY_NUM.
ToDo2: current bitmaps have one int as index, this limit us to 8TB RAM
guest, Should we move to longs?
Signed-off-by: Juan Quintela
---
exec.c | 9 ++---
include/exec/cpu-all.h | 4 +++-
include/exec/memory-internal.h | 11 ---
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/exec.c b/exec.c
index f037473..6fd83c9 100644
--- a/exec.c
+++ b/exec.c
@@ -1182,9 +1182,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void
*host,
new_ram_size = last_ram_offset() >> TARGET_PAGE_BITS;
if (new_ram_size > old_ram_size) {
-ram_list.phys_dirty = g_realloc(ram_list.phys_dirty, new_ram_size);
-memset(ram_list.phys_dirty + (new_block->offset >> TARGET_PAGE_BITS),
- 0, size >> TARGET_PAGE_BITS);
+int i;
+for(i = 0; i < DIRTY_MEMORY_NUM; i++) {
+ram_list.dirty_memory[i] =
+bitmap_zero_extend(ram_list.dirty_memory[i],
+ old_ram_size, new_ram_size);
+ }
}
cpu_physical_memory_set_dirty_range(new_block->offset, size);
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b6998f0..019dc20 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -456,10 +456,12 @@ typedef struct RAMBlock {
int fd;
} RAMBlock;
+#define DIRTY_MEMORY_NUM 3
+
typedef struct RAMList {
QemuMutex mutex;
/* Protected by the iothread lock. */
-uint8_t *phys_dirty;
+unsigned long *dirty_memory[DIRTY_MEMORY_NUM];
RAMBlock *mru_block;
/* Protected by the ramlist lock. */
QTAILQ_HEAD(, RAMBlock) blocks;
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 3f885a6..71f198e 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -44,7 +44,7 @@ static inline bool
cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
unsigned client)
{
assert(client < DIRTY_MEMORY_NUM);
-return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & (1 << client);
+return test_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
}
/* read dirty bit (return 0 or 1) */
@@ -75,7 +75,8 @@ static inline void
cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
unsigned client)
{
assert(client < DIRTY_MEMORY_NUM);
-ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= (1 << client);
+
+set_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
}
static inline void cpu_physical_memory_set_dirty(ram_addr_t addr)
@@ -88,11 +89,7 @@ static inline void cpu_physical_memory_set_dirty(ram_addr_t
addr)
static inline void cpu_physical_memory_clear_dirty_flag(ram_addr_t addr,
unsigned client)
{
-int mask = ~(1 << client);
-
-assert(client < DIRTY_MEMORY_NUM);
-
-ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
+clear_bit(addr >> TARGET_PAGE_BITS, ram_list.dirty_memory[client]);
}
static inline void cpu_physical_memory_set_dirty_range(ram_addr_t start,
--
1.8.3.1
Re: [Qemu-devel] [PATCH] ahci: set ahci mode on reset
Am 28.09.2013 um 23:09 hat Michael S. Tsirkin geschrieben: > ATM we set AHCI mode on 1st GHC write. > Spec says we should set it on reset. > > Signed-off-by: Michael S. Tsirkin Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH v2 2/4] Curling: cmdline interface.
[your emailer munged the reply, making it a bit hard to read. Are you
set for plain-text-only mail to the list?]
On 10/09/2013 12:49 AM, junqing.w...@cs2c.com.cn wrote:
> >> +++ b/hmp.c
>>> @@ -1213,10 +1213,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>>> int detach = qdict_get_try_bool(qdict, "detach", 0);
>>> int blk = qdict_get_try_bool(qdict, "blk", 0);
>>> int inc = qdict_get_try_bool(qdict, "inc", 0);
>>> + int ft = qdict_get_try_bool(qdict, "ft", 0);
>>
>> Why two spaces?
>
> To align the '=', I will remove them if you like.
It's not a problem with me either way, other than we have a lot of code
that doesn't care about alignment and consistently uses one space, and a
fair amount of code where everything in a block of code is consistently
aligned. But your patch was neither, in the context of the block it
lives within - if you're going to align, then line up everything with
the longest line 'int detach' (including blk and inc).
>
> >
>>> +++ b/qapi-schema.json
>>> @@ -2420,7 +2420,8 @@
>>> # Since: 0.14.0
>>> ##
>>> { 'command': 'migrate',
>>> - 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool'
>>> } }
>>> + 'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool',
>>> + '*ft': 'bool' } }
>>
>> Missing documentation, including mention that the new option was only
>> made available in 1.7. We still don't have introspection; is there some
>> other means by which libvirt and other management apps can tell whether
>> this feature is available?
>
> I'm not clear about how to do that, could you pls give me some hints, where
> to
> add code and documentation.
As for the documentation, qapi-schema.json has plenty of examples (look
for a field with "(since 1.7)" as a hint for how to document an optional
field added in a later release than the main struct).
As for the introspection, Amos Kong was most recently working on trying
to add that (but missed the 1.6 deadline, and I haven't seen work on it
since). Introspection is not a hard requirement, but it makes it harder
for libvirt to know if it can use 'ft':true if there is no other
'query-*' command that it can call first that would give it a hint that
this is a new enough qemu to support 'ft' during migration. Maybe even
having something listed under query-migrate-capabilities would be
sufficient (ie. modify the 'MigrationCapability' enum to advertise a new
capability).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH 15/28] memory: make sure that client is always inside range
Signed-off-by: Juan Quintela
---
include/exec/memory-internal.h | 4
1 file changed, 4 insertions(+)
diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h
index 3947caa..e08ac42 100644
--- a/include/exec/memory-internal.h
+++ b/include/exec/memory-internal.h
@@ -43,6 +43,7 @@ void qemu_ram_free_from_ptr(ram_addr_t addr);
static inline bool cpu_physical_memory_get_dirty_flag(ram_addr_t addr,
unsigned client)
{
+assert(client < DIRTY_MEMORY_NUM);
return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] & (1 << client);
}
@@ -73,6 +74,7 @@ static inline int cpu_physical_memory_get_dirty(ram_addr_t
start,
static inline void cpu_physical_memory_set_dirty_flag(ram_addr_t addr,
unsigned client)
{
+assert(client < DIRTY_MEMORY_NUM);
ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] |= (1 << client);
}
@@ -88,6 +90,8 @@ static inline int
cpu_physical_memory_clear_dirty_flag(ram_addr_t addr,
{
int mask = ~(1 << client);
+assert(client < DIRTY_MEMORY_NUM);
+
return ram_list.phys_dirty[addr >> TARGET_PAGE_BITS] &= mask;
}
--
1.8.3.1
