Re: [Qemu-devel] [PATCH] QEMUBH: make AioContext's bh re-entrant
Il 13/06/2013 22:55, Liu Ping Fan ha scritto:
> BH will be used outside big lock, so introduce lock to protect it.
> Note that the lock only affects the writer and bh's callback does
> not take this extra lock.
>
> Signed-off-by: Liu Ping Fan
> ---
> async.c | 10 +-
> include/block/aio.h | 2 ++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/async.c b/async.c
> index 90fe906..b1dcead 100644
> --- a/async.c
> +++ b/async.c
> @@ -47,8 +47,11 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void
> *opaque)
> bh->ctx = ctx;
> bh->cb = cb;
> bh->opaque = opaque;
> +qemu_mutex_lock(&ctx->bh_lock);
> bh->next = ctx->first_bh;
> +smp_wmb();
> ctx->first_bh = bh;
> +qemu_mutex_unlock(&ctx->bh_lock);
> return bh;
> }
>
> @@ -60,7 +63,9 @@ int aio_bh_poll(AioContext *ctx)
> ctx->walking_bh++;
>
> ret = 0;
> -for (bh = ctx->first_bh; bh; bh = next) {
> +bh = ctx->first_bh;
> +smp_rmb();
> +for (; bh; bh = next) {
> next = bh->next;
The read barrier in theory should go inside the loop. On the other
hand, it only needs to be a smp_read_barrier_depends().
Paolo
> if (!bh->deleted && bh->scheduled) {
> bh->scheduled = 0;
> @@ -75,6 +80,7 @@ int aio_bh_poll(AioContext *ctx)
>
> /* remove deleted bhs */
> if (!ctx->walking_bh) {
> +qemu_mutex_lock(&ctx->bh_lock);
> bhp = &ctx->first_bh;
> while (*bhp) {
> bh = *bhp;
> @@ -85,6 +91,7 @@ int aio_bh_poll(AioContext *ctx)
> bhp = &bh->next;
> }
> }
> +qemu_mutex_unlock(&ctx->bh_lock);
> }
>
> return ret;
> @@ -211,6 +218,7 @@ AioContext *aio_context_new(void)
> ctx = (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioContext));
> ctx->pollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD));
> ctx->thread_pool = NULL;
> +qemu_mutex_init(&ctx->bh_lock);
> event_notifier_init(&ctx->notifier, false);
> aio_set_event_notifier(ctx, &ctx->notifier,
> (EventNotifierHandler *)
> diff --git a/include/block/aio.h b/include/block/aio.h
> index 1836793..a65d16f 100644
> --- a/include/block/aio.h
> +++ b/include/block/aio.h
> @@ -17,6 +17,7 @@
> #include "qemu-common.h"
> #include "qemu/queue.h"
> #include "qemu/event_notifier.h"
> +#include "qemu/thread.h"
>
> typedef struct BlockDriverAIOCB BlockDriverAIOCB;
> typedef void BlockDriverCompletionFunc(void *opaque, int ret);
> @@ -53,6 +54,7 @@ typedef struct AioContext {
> */
> int walking_handlers;
>
> +QemuMutex bh_lock;
> /* Anchor of the list of Bottom Halves belonging to the context */
> struct QEMUBH *first_bh;
>
>
Re: [Qemu-devel] [PATCH] spapr: add yet another maintainer
On Thu, Jun 13, 2013 at 02:10:43AM +0200, Alexander Graf wrote: > > On 13.06.2013, at 02:08, Alexey Kardashevskiy wrote: > > > On 06/13/2013 12:33 AM, Alexander Graf wrote: > >> > >> On 12.06.2013, at 16:27, Alexey Kardashevskiy wrote: > >> > >>> Signed-off-by: Alexey Kardashevskiy > >>> --- > >>> MAINTAINERS |1 + > >>> 1 file changed, 1 insertion(+) > >>> > >>> diff --git a/MAINTAINERS b/MAINTAINERS > >>> index 13c0cc5..1e00bb1 100644 > >>> --- a/MAINTAINERS > >>> +++ b/MAINTAINERS > >>> @@ -430,6 +430,7 @@ F: hw/isa/pc87312.[hc] > >>> sPAPR > >>> M: David Gibson > >> > >> David should get removed here then, no? > > > > Well, at least sometime he is a qemu-ppc maintainer :) Don't know and I am > > not the one to decide. > > Yup, hence David should send a patch to remove himself and add you > instead, if that's what he sees fit. Otherwise I'd like a patch from > David removing himself only for now. I've send an updated version through to you. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgpjOcVLrKU0x.pgp Description: PGP signature
[Qemu-devel] [PATCH 3/3] pseries: Update MAINTAINERS information
I'm no longer at IBM, and therefore no long actively working on the pseries (aka sPAPR) qemu machine type. This patch replaces my information in the MAINTAINERS file with that for Alexey Kardashevskiy, who is taking over maintainership of the pseries code. While we're at it, I've added some extra file patterns for pseries specific files that weren't included in the existing pattern. Cc: Alexey Kardashevskiy Signed-off-by: David Gibson --- MAINTAINERS | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 3412b07..af28c77 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -428,11 +428,15 @@ F: hw/pci/devices/host-prep.[hc] F: hw/isa/pc87312.[hc] sPAPR -M: David Gibson +M: Alexey Kardashevskiy M: Alexander Graf L: qemu-...@nongnu.org S: Supported F: hw/*/spapr* +F: include/hw/*/spapr* +F: hw/*/xics* +F: include/hw/*/xics* +F: pc-bios/spapr-rtas/* virtex_ml507 M: Edgar E. Iglesias -- 1.8.1.4
[Qemu-devel] [PATCH 2/3] target-ppc kvm: save cr register
From: Alexey Kardashevskiy
This adds a missing code to save CR (condition register) via
kvm_arch_put_registers(). kvm_arch_get_registers() already has it.
Signed-off-by: Alexey Kardashevskiy
Signed-off-by: David Gibson
---
target-ppc/kvm.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 2bbc3b8..c89dd58 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -791,6 +791,11 @@ int kvm_arch_put_registers(CPUState *cs, int level)
for (i = 0;i < 32; i++)
regs.gpr[i] = env->gpr[i];
+regs.cr = 0;
+for (i = 0; i < 8; i++) {
+regs.cr |= (env->crf[i] & 15) << (4 * (7 - i));
+}
+
ret = kvm_vcpu_ioctl(cs, KVM_SET_REGS, ®s);
if (ret < 0)
return ret;
--
1.8.1.4
[Qemu-devel] [0/3] Accumulated ppc patches
Hi Alex, I see that you're now back from your holiday. Here, rather belatedly, is my series of ppc related patches collected while acting ppc maintainer. As you can see there aren't many at all. I'm not sure if that's because I wasn't looking in the right places for people sending patches, or there just wasn't much activity. Anyway, here they are, do with them as you see fit.
[Qemu-devel] [PATCH 1/3] target-ppc: Change default machine for 64-bit
Currently, for qemu-system-ppc64, the default machine type is 'mac99'.
The mac99 machine is not being actively maintained, and represents a
bizarre hybrid of components that never actually existed as a real system.
This patch changes the default machine to 'pseries', which is actively
maintained and works well with most modern ppc64 Linux distributions as a
guest.
Because the pseries machine type is optional (it is only built when libfdt
is available), this can result in a build with no default machine. In that
case vl.c will print a "No machine found" message. This seems reasonable,
given that as mentioned, mac99 is unlikely to be a good choice.
Signed-off-by: David Gibson
---
hw/ppc/mac_newworld.c | 3 ---
hw/ppc/spapr.c| 1 +
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index ce44e95..dafe7d2 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -458,9 +458,6 @@ static QEMUMachine core99_machine = {
.desc = "Mac99 based PowerMAC",
.init = ppc_core99_init,
.max_cpus = MAX_CPUS,
-#ifdef TARGET_PPC64
-.is_default = 1,
-#endif
DEFAULT_MACHINE_OPTIONS,
};
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 218ea23..5363c3f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -971,6 +971,7 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
static QEMUMachine spapr_machine = {
.name = "pseries",
.desc = "pSeries Logical Partition (PAPR compliant)",
+.is_default = 1,
.init = ppc_spapr_init,
.reset = ppc_spapr_reset,
.block_default_type = IF_SCSI,
--
1.8.1.4
Re: [Qemu-devel] [SeaBIOS] solaris x86 in qemu? [bisected]
On Fri, Jun 14, 2013 at 06:14:00PM +0400, Michael Tokarev wrote: > 14.06.2013 16:36, Gerd Hoffmann wrote: > > Hi, > > > >> Hmm. Speaking of the splitting. Does the current bios include the > >> the tables which were split into separate files? > > > > Yes, they are in out/ too after building seabios. Use "qemu -L > > /path/to/seabios/out" to make sure qemu picks up matching bios.bin and dsdt. > > Yes I know they're built in seabios/out, and I know one may > use -L option here. So that, for example, -bios option of > qemu makes less sense now than it was before. > > But my question was about something different. > > Can I use current seabios with old qemu which does not provide > the separate ACPI tables? For example, does current bios contain > these tables too, so they're both separate and embedded? Yes. > And the reverse, can I use old bios with new qemu which do provide > the separate tables? Yes. The only limitation is that one can not use the q35 machine type without also having a new version of seabios. > Which tables will be used in each case? SeaBIOS has the "piix4" acpi tables compiled into it, and it will use those if it doesn't receive a dsdt from qemu. -Kevin
Re: [Qemu-devel] [PATCH] make screendump an async command
On Fri, 2013-06-14 at 13:21 -0500, Anthony Liguori wrote:
> Alon Levy writes:
>
> > This fixes the broken screendump behavior for qxl devices in native mode
> > since 81fb6f1504fb9ef71f2382f44af34756668296e8.
> >
> > Note: due to QAPI not generating async commands yet I had to remove the
> > schema screendump definition.
> >
> > Related RHBZ: 973374
> > This patch is not enough to fix said bz, with the linux qxl driver you get
> > garbage still, just not out of date garbage.
> >
> > Signed-off-by: Alon Levy
>
> Async commands are badly broken with respect to error handling.
Are there any ideas on how this should work? From the user perspective
nothing changes, so this is an internal qemu design issue afaict.
>
> This needs to be done after we get the new QMP server.
Is there any ETA on this? I'm not pressuring, I'm just trying to figure
out if it is eminent or else I'll add a temporary patch to the
downstream (which is what I'm trying to avoid by sending this patch in
the first place).
>
> Why is QXL unable to do a synchronous screendump?
The qxl device needs to flush all the commands that haven't been
rendered. The rendering is done off thread in the worker thread for that
device. The communication between the threads happens via a pipe that
the io thread is listening to. That is the same thread the qmp command
is received on, so there is no option to block unless I start a new
event loop in there, which is ugly. i.e. I need some kind of bottom
half, like the async command provides, which client_migrate_info uses.
>
> Regards,
>
> Anthony Liguori
>
> > ---
> > hmp.c | 2 +-
> > hw/display/qxl-render.c | 1 +
> > hw/display/vga.c | 1 +
> > include/qapi/qmp/qerror.h | 6 +
> > include/ui/console.h | 10
> > qapi-schema.json | 13 ---
> > qmp-commands.hx | 3 ++-
> > ui/console.c | 58
> > ++-
> > 8 files changed, 78 insertions(+), 16 deletions(-)
> >
> > diff --git a/hmp.c b/hmp.c
> > index 494a9aa..2a37975 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> > const char *filename = qdict_get_str(qdict, "filename");
> > Error *err = NULL;
> >
> > -qmp_screendump(filename, &err);
> > +hmp_screen_dump_helper(filename, &err);
> > hmp_handle_error(mon, &err);
> > }
> >
> > diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> > index f511a62..d719448 100644
> > --- a/hw/display/qxl-render.c
> > +++ b/hw/display/qxl-render.c
> > @@ -139,6 +139,7 @@ static void
> > qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
> > qxl->dirty[i].bottom - qxl->dirty[i].top);
> > }
> > qxl->num_dirty_rects = 0;
> > +console_screendump_complete(vga->con);
> > }
> >
> > /*
> > diff --git a/hw/display/vga.c b/hw/display/vga.c
> > index 21a108d..1fc52eb 100644
> > --- a/hw/display/vga.c
> > +++ b/hw/display/vga.c
> > @@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
> > break;
> > }
> > }
> > +console_screendump_complete(s->con);
> > }
> >
> > /* force a full display refresh */
> > diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> > index 6c0a18d..1bd7efa 100644
> > --- a/include/qapi/qmp/qerror.h
> > +++ b/include/qapi/qmp/qerror.h
> > @@ -237,6 +237,12 @@ void assert_no_error(Error *err);
> > #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
> > ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export
> > path '%s' is mounted in the guest using mount_tag '%s'"
> >
> > +#define QERR_SCREENDUMP_NOT_AVAILABLE \
> > +ERROR_CLASS_GENERIC_ERROR, "There is no QemuConsole I can screendump
> > from"
> > +
> > +#define QERR_SCREENDUMP_IN_PROGRESS \
> > +ERROR_CLASS_GENERIC_ERROR, "Existing screendump in progress"
> > +
> > #define QERR_SOCKET_CONNECT_FAILED \
> > ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 4307b5f..643da45 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -341,4 +341,14 @@ int index_from_keycode(int code);
> > void early_gtk_display_init(void);
> > void gtk_display_init(DisplayState *ds);
> >
> > +/* hw/display/vga.c hw/display/qxl.c */
> > +void console_screendump_complete(QemuConsole *con);
> > +
> > +/* monitor.c */
> > +int qmp_screendump(Monitor *mon, const QDict *qdict,
> > + MonitorCompletion cb, void *opaque);
> > +
> > +/* hmp.c */
> > +void hmp_screen_dump_helper(const char *filename, Error **errp);
> > +
> > #endif
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 5ad6894..f5fdc2f 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3112,19 +3112,6 @@
> >'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
> >
> > ##
> > -# @screend
[Qemu-devel] [PATCH v2 1/2] hw/loader: Support ramdisk with u-boot header
Introduce 'load_ramdisk()' which can load "normal" ramdisks and ramdisks
with a u-boot header.
To enable this and leverage synergies 'load_uimage()' is refactored to
accomodate this additional use case.
Signed-off-by: Soren Brinkmann
---
v2:
- document the new 'load_ramdisk()' function
- print the image type byte in the error messages for unsupported
u-boot images
- load_uimage() and load_ramdisk() both use load_uboot_image() to load
images. Let the callers pass the expected image type to
load_uboot_image to allow error checking for wrong image types. E.g.
passing a ramdisk image to the load_uimage routine.
hw/core/loader.c| 90 ++---
include/hw/loader.h | 13
2 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7507914..5366423 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t
*src,
}
/* Load a U-Boot image. */
-int load_uimage(const char *filename, hwaddr *ep,
-hwaddr *loadaddr, int *is_linux)
+static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
+int *is_linux, uint8_t image_type)
{
int fd;
int size;
+hwaddr address;
uboot_image_header_t h;
uboot_image_header_t *hdr = &h;
uint8_t *data = NULL;
int ret = -1;
+int do_uncompress = 0;
fd = open(filename, O_RDONLY | O_BINARY);
if (fd < 0)
@@ -457,32 +459,55 @@ int load_uimage(const char *filename, hwaddr *ep,
if (hdr->ih_magic != IH_MAGIC)
goto out;
-/* TODO: Implement other image types. */
-if (hdr->ih_type != IH_TYPE_KERNEL) {
-fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
+if (hdr->ih_type != image_type) {
+fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
+image_type);
goto out;
}
-switch (hdr->ih_comp) {
-case IH_COMP_NONE:
-case IH_COMP_GZIP:
+/* TODO: Implement other image types. */
+switch (hdr->ih_type) {
+case IH_TYPE_KERNEL:
+address = hdr->ih_load;
+if (loadaddr) {
+*loadaddr = hdr->ih_load;
+}
+
+switch (hdr->ih_comp) {
+case IH_COMP_NONE:
+break;
+case IH_COMP_GZIP:
+do_uncompress = 1;
+break;
+default:
+fprintf(stderr,
+"Unable to load u-boot images with compression type %d\n",
+hdr->ih_comp);
+goto out;
+}
+
+if (ep) {
+*ep = hdr->ih_ep;
+}
+
+/* TODO: Check CPU type. */
+if (is_linux) {
+if (hdr->ih_os == IH_OS_LINUX) {
+*is_linux = 1;
+} else {
+*is_linux = 0;
+}
+}
+
+break;
+case IH_TYPE_RAMDISK:
+address = *loadaddr;
break;
default:
-fprintf(stderr,
-"Unable to load u-boot images with compression type %d\n",
-hdr->ih_comp);
+fprintf(stderr, "Unsupported u-boot image type %d\n", hdr->ih_type);
goto out;
}
-/* TODO: Check CPU type. */
-if (is_linux) {
-if (hdr->ih_os == IH_OS_LINUX)
-*is_linux = 1;
-else
-*is_linux = 0;
-}
-
-*ep = hdr->ih_ep;
data = g_malloc(hdr->ih_size);
if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
@@ -490,7 +515,7 @@ int load_uimage(const char *filename, hwaddr *ep,
goto out;
}
-if (hdr->ih_comp == IH_COMP_GZIP) {
+if (do_uncompress) {
uint8_t *compressed_data;
size_t max_bytes;
ssize_t bytes;
@@ -508,10 +533,7 @@ int load_uimage(const char *filename, hwaddr *ep,
hdr->ih_size = bytes;
}
-rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
-
-if (loadaddr)
-*loadaddr = hdr->ih_load;
+rom_add_blob_fixed(filename, data, hdr->ih_size, address);
ret = hdr->ih_size;
@@ -522,6 +544,24 @@ out:
return ret;
}
+int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
+int *is_linux)
+{
+return load_uboot_image(filename, ep, loadaddr, is_linux, IH_TYPE_KERNEL);
+}
+
+/* Load a ramdisk. */
+int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
+{
+int size = load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK);
+
+if (size < 0) {
+size = load_image_targphys(filename, addr, max_sz);
+}
+
+return size;
+}
+
/*
* Functions for reboot-persistent memory regions.
* - used for vga bios and option roms.
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 0958f06..6560e96 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -16,6 +16,19 @@ int load_aout(const char *filename, hwaddr addr, i
[Qemu-devel] [PATCH v2 0/2] Support ramdisks with U-Boot header
Updated with Peter's comments. The detailed changelog can be found in the following emails with the patches. Sören Soren Brinkmann (2): hw/loader: Support ramdisk with u-boot header hw/arm: Use 'load_ramdisk()' for loading ramdisk hw/arm/boot.c | 8 ++--- hw/core/loader.c| 90 ++--- include/hw/loader.h | 13 3 files changed, 82 insertions(+), 29 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH v2 2/2] hw/arm: Use 'load_ramdisk()' for loading ramdisk
The load_ramdisk function takes over loading traditional ramdisks images
and does also load ramdisks with u-boot header.
Signed-off-by: Soren Brinkmann
---
hw/arm/boot.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index f310f73..ab2b234 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -436,10 +436,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
info->entry = entry;
if (is_linux) {
if (info->initrd_filename) {
-initrd_size = load_image_targphys(info->initrd_filename,
- info->initrd_start,
- info->ram_size -
- info->initrd_start);
+initrd_size = load_ramdisk(info->initrd_filename,
+ info->initrd_start,
+ info->ram_size -
+ info->initrd_start);
if (initrd_size < 0) {
fprintf(stderr, "qemu: could not load initrd '%s'\n",
info->initrd_filename);
--
1.8.3.1
Re: [Qemu-devel] [PATCH v2] kvm/openpic: in-kernel mpic support
On 06/14/2013 09:59:18 AM, Andreas Färber wrote:
Am 13.06.2013 19:32, schrieb Scott Wood:
> On 06/13/2013 06:01:49 AM, Andreas Färber wrote:
>> Am 12.06.2013 22:32, schrieb Scott Wood:
>> > +typedef struct KVMOpenPICState {
>> > +SysBusDevice busdev;
>>
>> SysBusDevice parent_obj; please!
>>
>> http://wiki.qemu.org/QOMConventions
>
> The word "QOMConventions" doesn't exist once in the QEMU source
tree.
> How is one supposed to know that this documentation exists? :-P
I do expect people to read at least the subject of patch series on
qemu-devel,
I have over 12000 currently unread e-mails from that list. They are
not separated into "patch" and "other". Even among the patches, they
get posted at least twice due to the (unique to QEMU and KVM as far as
I can tell) practice of reposting everything before a pull request.
There's just no way I can keep up with all of this, *plus* all the
non-QEMU stuff I need to keep up with. Sorry. I generally rely on
Alex to guide me on things like qdev/QOM. Quite frankly, I didn't even
realize I was using QOM. I thought this was still qdev. I even create
it using qdev_create()...
short of individual review comments. CPU, PReP PCI,
Versatile PCI, ISA and more recently virtio series had been posted.
...which of those would make me think "hmm, there's something in here
that I need to read before submitting patches for in-kernel mpic"?
I'm not trying to be difficult -- I'm just trying to point out that
there's room for improvement in how the QEMU community communicates to
developers what is expected. Maybe an announcement list that just
contains important updates and summaries? Also, even starting on
http://wiki.qemu.org/ I don't see any obvious path to get to
QOMConventions. It's not even linked to from the main QOM page.
I do understand the frustration of having to correct people on the same
things over and over -- I experience it myself in other contexts. But
there are more constructive ways to deal with it than exclamation
points.
> Plus, this is just copied from the non-KVM MPIC file, and I see many
> other instances throughout the source tree.
Which exactly is the reason for my grief: Your ignorance is making
other
people even more work, and at least Alex should've spotted it - I
can't
review all patches just because they might or might not be touching
on QOM.
Just as we are supposed to not copy old Coding Style in new patches,
we
should be applying new patterns and conventions in new patches, too.
I'm usually the first person to complain about bad copy and paste, but
this is a situation where the KVM version of openpic is supposed to
interface with the rest of the system in exactly the same way as the
regular openpic. It really does not make sense to write the glue code
from scratch. And it's not as if the rest of QEMU had just been fixed,
and this patch reintroduces the old stuff. I count 166 instances of
"SysBusDevice busdev" and only 9 instances of "SysBusDevice
parent_obj". Perhaps these could all be fixed up in an automated way?
And "making other people even more work" goes both ways...
>> > +static int kvm_openpic_init(SysBusDevice *dev)
>>
>> Please make this instance_init + realize functions - "dev" should
rather
>> be reserved for DeviceState.
>
> Could you elaborate? I'm really not familiar with this stuff, and
have
> not found much documentation. Again, this is patterned after the
existing
> non-KVM openpic file.
static void kvm_openpic_init(Object *obj) should initialize simple
variables, MemoryRegions that don't depend on parameters and any QOM
properties.
static void kvm_openpic_realize(DeviceState *dev, Error **errp) should
initialize the rest.
kvm_openpic_unrealize(Device *dev, Error **errp) and
kvm_openpic_finalize(Object *obj) would be their counterparts for
cleanup.
When would kvm_openpic_realize/unrealize/finalize get called?
Note that we must create the kernel side of the device in
kvm_openpic_init(), because we need to return failure if it's not
supported, so that the platform can fall back onto creating a normal
QEMU openpic instead.
Also note that an in-kernel MPIC cannot be destroyed, without
destroying the entire VM. So I'm not sure what unrealize/finalize
would do.
All of this is basically done the way Alex told/showed me to do it.
>> > +{
>> > +KVMState *s = kvm_state;
>> > +KVMOpenPICState *opp = FROM_SYSBUS(typeof(*opp), dev);
>>
>> NACK, please introduce your own KVM_OPENPIC(obj) cast macro
instead for
>> new devices - has been a topic for several weeks and months now.
>
> There's way too much traffic on the list for me to know about
something
> just because it's "been a topic". Lately it's been too much for me
to
> even scan the subject lines looking for things that look important,
> given that QEMU is only a fraction of what I spend my time on.
>
> If you'd like to update hw/intc/openpic.c
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Change default machine for 64-bit
On Fri, 2013-06-14 at 16:28 +0200, Andreas Färber wrote: > > I would still love to see -M mac99 emulate a proper U2 or U1 based > system for 32bit. And I would also love to see a real U4 based > emulation model come up for qemu-system-ppc64. But I doubt I'll have > time to work on either :). Uninorth itself is easy, the guests don't mess with it much, and we have the infrastructure for iommu's nowadays. KL is the interesting bit. The main missing piece of the puzzle is really a PMU device. There are other bits in the chipset like GEM ethernet and the Frodo SATA but: - They are probed using PCI probing, so not having them isn't a huge deal. - A GEM emulation shouldn't be terribly hard, there's bunch of registers manipulated by the driver that we can probably just ignore. - A Frodo emulation would be trivial considered that the driver doesn't use anything but basic MMIO taskfile access (so does the MacOS driver) and a bit of SCRs. Now if only I had more spare time ... :-) Cheers, Ben.
[Qemu-devel] [PATCH v9 04/14] rdma: export throughput w/ MigrationStats QMP
From: "Michael R. Hines"
This exposes throughput (in megabits/sec) through QMP.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
hmp.c |2 ++
include/migration/migration.h |1 +
migration.c |6 ++
qapi-schema.json |5 -
4 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 494a9aa..148a3fb 100644
--- a/hmp.c
+++ b/hmp.c
@@ -169,6 +169,8 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
if (info->has_ram) {
monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n",
info->ram->transferred >> 10);
+monitor_printf(mon, "throughput: %0.2f mbps\n",
+ info->ram->mbps);
monitor_printf(mon, "remaining ram: %" PRIu64 " kbytes\n",
info->ram->remaining >> 10);
monitor_printf(mon, "total ram: %" PRIu64 " kbytes\n",
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 0be28a2..1eeaa40 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -47,6 +47,7 @@ struct MigrationState
int64_t dirty_bytes_rate;
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
int64_t xbzrle_cache_size;
+double mbps;
};
void process_incoming_migration(QEMUFile *f);
diff --git a/migration.c b/migration.c
index 058f9e6..441a3b2 100644
--- a/migration.c
+++ b/migration.c
@@ -66,6 +66,7 @@ MigrationState *migrate_get_current(void)
.state = MIG_STATE_SETUP,
.bandwidth_limit = MAX_THROTTLE,
.xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
+.mbps = -1,
};
return ¤t_migration;
@@ -201,6 +202,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
info->ram->normal = norm_mig_pages_transferred();
info->ram->normal_bytes = norm_mig_bytes_transferred();
info->ram->dirty_pages_rate = s->dirty_pages_rate;
+info->ram->mbps = s->mbps;
if (blk_mig_active()) {
info->has_disk = true;
@@ -230,6 +232,7 @@ MigrationInfo *qmp_query_migrate(Error **errp)
info->ram->skipped = skipped_mig_pages_transferred();
info->ram->normal = norm_mig_pages_transferred();
info->ram->normal_bytes = norm_mig_bytes_transferred();
+info->ram->mbps = s->mbps;
break;
case MIG_STATE_ERROR:
info->has_status = true;
@@ -543,6 +546,9 @@ static void *migration_thread(void *opaque)
double bandwidth = transferred_bytes / time_spent;
max_size = bandwidth * migrate_max_downtime() / 100;
+s->mbps = time_spent ? (((double) transferred_bytes * 8.0) /
+((double) time_spent / 1000.0)) / 1000.0 / 1000.0 : -1;
+
DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64
" bandwidth %g max_size %" PRId64 "\n",
transferred_bytes, time_spent, bandwidth, max_size);
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..b239adb 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -513,12 +513,15 @@
# @dirty-pages-rate: number of pages dirtied by second by the
#guest (since 1.3)
#
+# @mbps: throughput in megabits/sec. (since 1.5)
+#
# Since: 0.14.0
##
{ 'type': 'MigrationStats',
'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
- 'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
+ 'normal-bytes': 'int', 'dirty-pages-rate' : 'int',
+ 'mbps' : 'number' } }
##
# @XBZRLECacheStats
--
1.7.10.4
[Qemu-devel] [PATCH v9 06/14] rdma: export qemu_fflush()
From: "Michael R. Hines"
RDMA uses this to flush the control channel before sending its
own message to handle page registrations.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
include/migration/qemu-file.h |1 +
savevm.c |2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index dd3fd51..37d1604 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -112,6 +112,7 @@ void qemu_file_reset_rate_limit(QEMUFile *f);
void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate);
int64_t qemu_file_get_rate_limit(QEMUFile *f);
int qemu_file_get_error(QEMUFile *f);
+void qemu_fflush(QEMUFile *f);
static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
{
diff --git a/savevm.c b/savevm.c
index 1de6728..ba307b1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -588,7 +588,7 @@ static inline bool qemu_file_is_writable(QEMUFile *f)
* If there is writev_buffer QEMUFileOps it uses it otherwise uses
* put_buffer ops.
*/
-static void qemu_fflush(QEMUFile *f)
+void qemu_fflush(QEMUFile *f)
{
ssize_t ret = 0;
--
1.7.10.4
[Qemu-devel] [PATCH v9 08/14] rdma: introduce qemu_ram_foreach_block()
From: "Michael R. Hines"
This is used during RDMA initialization in order to
transmit a description of all the RAM blocks to the
peer for later dynamic chunk registration purposes.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
exec.c|9 +
include/exec/cpu-common.h |5 +
2 files changed, 14 insertions(+)
diff --git a/exec.c b/exec.c
index 5b8b40d..a094664 100644
--- a/exec.c
+++ b/exec.c
@@ -2604,3 +2604,12 @@ bool cpu_physical_memory_is_io(hwaddr phys_addr)
memory_region_is_romd(section->mr));
}
#endif
+
+void qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque)
+{
+RAMBlock *block;
+
+QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+func(block->host, block->offset, block->length, opaque);
+}
+}
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index e061e21..92a4223 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -113,6 +113,11 @@ void cpu_physical_memory_write_rom(hwaddr addr,
extern struct MemoryRegion io_mem_rom;
extern struct MemoryRegion io_mem_notdirty;
+typedef void (RAMBlockIterFunc)(void *host_addr,
+ram_addr_t offset, ram_addr_t length, void *opaque);
+
+void qemu_ram_foreach_block(RAMBlockIterFunc func, void *opaque);
+
#endif
#endif /* !CPU_COMMON_H */
--
1.7.10.4
[Qemu-devel] [PATCH v9 09/14] rdma: new QEMUFileOps hooks
From: "Michael R. Hines"
These are the prototypes and implementation of new hooks that
RDMA takes advantage of to perform dynamic page registration.
An optional hook is also introduced for a custom function
to be able to override the default save_page function.
Also included are the prototypes and accessor methods used by
arch_init.c which invoke funtions inside savevm.c to call out
to the hooks that may or may not have been overridden
inside of QEMUFileOps.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
include/migration/migration.h | 20 ++
include/migration/qemu-file.h | 29
savevm.c | 59 +
3 files changed, 108 insertions(+)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 110f4ad..d60c64c 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -21,6 +21,7 @@
#include "qapi/error.h"
#include "migration/vmstate.h"
#include "qapi-types.h"
+#include "exec/cpu-common.h"
struct MigrationParams {
bool blk;
@@ -132,4 +133,23 @@ int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
int64_t xbzrle_cache_resize(int64_t new_size);
+
+void ram_control_before_iterate(QEMUFile *f, uint64_t flags);
+void ram_control_after_iterate(QEMUFile *f, uint64_t flags);
+void ram_control_load_hook(QEMUFile *f, uint64_t flags);
+
+/* Whenever this is found in the data stream, the flags
+ * will be passed to ram_control_load_hook in the incoming-migration
+ * side. This lets before_ram_iterate/after_ram_iterate add
+ * transport-specific sections to the RAM migration data.
+ */
+#define RAM_SAVE_FLAG_HOOK 0x80
+
+#define RAM_SAVE_CONTROL_NOT_SUPP -1000
+#define RAM_SAVE_CONTROL_DELAYED -2000
+
+size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
+ ram_addr_t offset, size_t size,
+ int *bytes_sent);
+
#endif
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 37d1604..0f757fb 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -23,6 +23,7 @@
*/
#ifndef QEMU_FILE_H
#define QEMU_FILE_H 1
+#include "exec/cpu-common.h"
/* This function writes a chunk of data to a file at the given position.
* The pos argument can be ignored if the file is only being used for
@@ -57,12 +58,40 @@ typedef int (QEMUFileGetFD)(void *opaque);
typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov,
int iovcnt, int64_t pos);
+/*
+ * This function provides hooks around different
+ * stages of RAM migration.
+ */
+typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags);
+
+/*
+ * Constants used by ram_control_* hooks
+ */
+#define RAM_CONTROL_SETUP0
+#define RAM_CONTROL_ROUND1
+#define RAM_CONTROL_HOOK 2
+#define RAM_CONTROL_FINISH 3
+
+/*
+ * This function allows override of where the RAM page
+ * is saved (such as RDMA, for example.)
+ */
+typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque,
+ ram_addr_t block_offset,
+ ram_addr_t offset,
+ size_t size,
+ int *bytes_sent);
+
typedef struct QEMUFileOps {
QEMUFilePutBufferFunc *put_buffer;
QEMUFileGetBufferFunc *get_buffer;
QEMUFileCloseFunc *close;
QEMUFileGetFD *get_fd;
QEMUFileWritevBufferFunc *writev_buffer;
+QEMURamHookFunc *before_ram_iterate;
+QEMURamHookFunc *after_ram_iterate;
+QEMURamHookFunc *hook_ram_load;
+QEMURamSaveFunc *save_page;
} QEMUFileOps;
QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops);
diff --git a/savevm.c b/savevm.c
index ba307b1..d80af02 100644
--- a/savevm.c
+++ b/savevm.c
@@ -615,6 +615,65 @@ void qemu_fflush(QEMUFile *f)
}
}
+void ram_control_before_iterate(QEMUFile *f, uint64_t flags)
+{
+int ret = 0;
+
+if (f->ops->before_ram_iterate) {
+ret = f->ops->before_ram_iterate(f, f->opaque, flags);
+if (ret < 0) {
+qemu_file_set_error(f, ret);
+}
+}
+}
+
+void ram_control_after_iterate(QEMUFile *f, uint64_t flags)
+{
+int ret = 0;
+
+if (f->ops->after_ram_iterate) {
+ret = f->ops->after_ram_iterate(f, f->opaque, flags);
+if (ret < 0) {
+qemu_file_set_error(f, ret);
+}
+}
+}
+
+void ram_control_load_hook(QEMUFile *f, uint64_t flags)
+{
+int ret = 0;
+
+if (f->ops->hook_ram_load) {
+ret = f->ops->hook_ram_load(f, f->opaque, flags);
+if (ret < 0) {
+qemu_file_set_error(f, ret);
+}
+} else {
+qemu_file_set_error(f, ret);
+}
+}
+
+size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
+ ram_addr_t offset, size_t size, int *bytes_sent)
+{
+if (f->ops->save_pa
[Qemu-devel] [PATCH v9 03/14] rdma: export yield_until_fd_readable()
From: "Michael R. Hines"
The RDMA event channel can be made non-blocking just like a TCP
socket. Exporting this function allows us to yield so that the
QEMU monitor remains available.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
include/block/coroutine.h |6 ++
qemu-coroutine-io.c | 23 +++
savevm.c | 28
3 files changed, 29 insertions(+), 28 deletions(-)
diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index a978162..377805a 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -209,4 +209,10 @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
*/
void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
+/**
+ * Yield until a file descriptor becomes readable
+ *
+ * Note that this function clobbers the handlers for the file descriptor.
+ */
+void coroutine_fn yield_until_fd_readable(int fd);
#endif /* QEMU_COROUTINE_H */
diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c
index e8ad1a4..c4df35a 100644
--- a/qemu-coroutine-io.c
+++ b/qemu-coroutine-io.c
@@ -63,3 +63,26 @@ qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool
do_send)
struct iovec iov = { .iov_base = buf, .iov_len = bytes };
return qemu_co_sendv_recvv(sockfd, &iov, 1, 0, bytes, do_send);
}
+
+typedef struct {
+Coroutine *co;
+int fd;
+} FDYieldUntilData;
+
+static void fd_coroutine_enter(void *opaque)
+{
+FDYieldUntilData *data = opaque;
+qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
+qemu_coroutine_enter(data->co, NULL);
+}
+
+void coroutine_fn yield_until_fd_readable(int fd)
+{
+FDYieldUntilData data;
+
+assert(qemu_in_coroutine());
+data.co = qemu_coroutine_self();
+data.fd = fd;
+qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data);
+qemu_coroutine_yield();
+}
diff --git a/savevm.c b/savevm.c
index 2983905..2122bf0 100644
--- a/savevm.c
+++ b/savevm.c
@@ -149,34 +149,6 @@ typedef struct QEMUFileSocket
QEMUFile *file;
} QEMUFileSocket;
-typedef struct {
-Coroutine *co;
-int fd;
-} FDYieldUntilData;
-
-static void fd_coroutine_enter(void *opaque)
-{
-FDYieldUntilData *data = opaque;
-qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
-qemu_coroutine_enter(data->co, NULL);
-}
-
-/**
- * Yield until a file descriptor becomes readable
- *
- * Note that this function clobbers the handlers for the file descriptor.
- */
-static void coroutine_fn yield_until_fd_readable(int fd)
-{
-FDYieldUntilData data;
-
-assert(qemu_in_coroutine());
-data.co = qemu_coroutine_self();
-data.fd = fd;
-qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data);
-qemu_coroutine_yield();
-}
-
static ssize_t socket_writev_buffer(void *opaque, struct iovec *iov, int
iovcnt,
int64_t pos)
{
--
1.7.10.4
[Qemu-devel] [PATCH v9 05/14] rdma: introduce qemu_file_mode_is_not_valid()
From: "Michael R. Hines"
QEMUFileRDMA also has read and write modes. This function is now
shared to reduce code duplication.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
include/migration/qemu-file.h |1 +
savevm.c | 20 +---
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 8fab0dd..dd3fd51 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -80,6 +80,7 @@ void qemu_put_byte(QEMUFile *f, int v);
* The buffer should be available till it is sent asynchronously.
*/
void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size);
+bool qemu_file_mode_is_not_valid(const char *mode);
static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v)
{
diff --git a/savevm.c b/savevm.c
index 2122bf0..1de6728 100644
--- a/savevm.c
+++ b/savevm.c
@@ -449,14 +449,23 @@ static const QEMUFileOps socket_write_ops = {
.close = socket_close
};
-QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+bool qemu_file_mode_is_not_valid(const char *mode)
{
-QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
-
if (mode == NULL ||
(mode[0] != 'r' && mode[0] != 'w') ||
mode[1] != 'b' || mode[2] != 0) {
fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+return true;
+}
+
+return false;
+}
+
+QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+{
+QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
+
+if (qemu_file_mode_is_not_valid(mode)) {
return NULL;
}
@@ -474,10 +483,7 @@ QEMUFile *qemu_fopen(const char *filename, const char
*mode)
{
QEMUFileStdio *s;
-if (mode == NULL ||
- (mode[0] != 'r' && mode[0] != 'w') ||
- mode[1] != 'b' || mode[2] != 0) {
-fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+if (qemu_file_mode_is_not_valid(mode)) {
return NULL;
}
--
1.7.10.4
[Qemu-devel] [PATCH v9 01/14] rdma: add documentation
From: "Michael R. Hines" docs/rdma.txt contains full documentation, wiki links, github url and contact information. Reviewed-by: Paolo Bonzini Signed-off-by: Michael R. Hines --- docs/rdma.txt | 404 + 1 file changed, 404 insertions(+) create mode 100644 docs/rdma.txt diff --git a/docs/rdma.txt b/docs/rdma.txt new file mode 100644 index 000..4f98a3b --- /dev/null +++ b/docs/rdma.txt @@ -0,0 +1,404 @@ +(RDMA: Remote Direct Memory Access) +RDMA Live Migration Specification, Version # 1 +== +Wiki: http://wiki.qemu.org/Features/RDMALiveMigration +Github: g...@github.com:hinesmr/qemu.git, 'rdma' branch + +Copyright (C) 2013 Michael R. Hines + +An *exhaustive* paper (2010) shows additional performance details +linked on the QEMU wiki above. + +Contents: += +* Introduction +* Before running +* Running +* Performance +* RDMA Migration Protocol Description +* Versioning and Capabilities +* QEMUFileRDMA Interface +* Migration of pc.ram +* Error handling +* TODO + +Introduction: += + +RDMA helps make your migration more deterministic under heavy load because +of the significantly lower latency and higher throughput over TCP/IP. This is +because the RDMA I/O architecture reduces the number of interrupts and +data copies by bypassing the host networking stack. In particular, a TCP-based +migration, under certain types of memory-bound workloads, may take a more +unpredicatable amount of time to complete the migration if the amount of +memory tracked during each live migration iteration round cannot keep pace +with the rate of dirty memory produced by the workload. + +RDMA currently comes in two flavors: both Ethernet based (RoCE, or RDMA +over Convered Ethernet) as well as Infiniband-based. This implementation of +migration using RDMA is capable of using both technologies because of +the use of the OpenFabrics OFED software stack that abstracts out the +programming model irrespective of the underlying hardware. + +Refer to openfabrics.org or your respective RDMA hardware vendor for +an understanding on how to verify that you have the OFED software stack +installed in your environment. You should be able to successfully link +against the "librdmacm" and "libibverbs" libraries and development headers +for a working build of QEMU to run successfully using RDMA Migration. + +BEFORE RUNNING: +=== + +Use of RDMA during migration requires pinning and registering memory +with the hardware. This means that memory must be physically resident +before the hardware can transmit that memory to another machine. +If this is not acceptable for your application or product, then the use +of RDMA migration may in fact be harmful to co-located VMs or other +software on the machine if there is not sufficient memory available to +relocate the entire footprint of the virtual machine. If so, then the +use of RDMA is discouraged and it is recommended to use standard TCP migration. + +Experimental: Next, decide if you want dynamic page registration. +For example, if you have an 8GB RAM virtual machine, but only 1GB +is in active use, then enabling this feature will cause all 8GB to +be pinned and resident in memory. This feature mostly affects the +bulk-phase round of the migration and can be enabled for extremely +high-performance RDMA hardware using the following command: + +QEMU Monitor Command: +$ migrate_set_capability x-rdma-pin-all on # disabled by default + +Performing this action will cause all 8GB to be pinned, so if that's +not what you want, then please ignore this step altogether. + +On the other hand, this will also significantly speed up the bulk round +of the migration, which can greatly reduce the "total" time of your migration. +Example performance of this using an idle VM in the previous example +can be found in the "Performance" section. + +RUNNING: + + +First, set the migration speed to match your hardware's capabilities: + +QEMU Monitor Command: +$ migrate_set_speed 40g # or whatever is the MAX of your RDMA device + +Next, on the destination machine, add the following to the QEMU command line: + +qemu . -incoming x-rdma:host:port + +Finally, perform the actual migration on the source machine: + +QEMU Monitor Command: +$ migrate -d x-rdma:host:port + +PERFORMANCE +=== + +Here is a brief summary of total migration time and downtime using RDMA: +Using a 40gbps infiniband link performing a worst-case stress test, +using an 8GB RAM virtual machine: + +Using the following command: +$ apt-get install stress +$ stress --vm-bytes 7500M --vm 1 --vm-keep + +1. Migration throughput: 26 gigabits/second. +2. Downtime (stop time) varies between 15 and 100 milliseconds. + +EFFECTS of memory registration on bulk phase round: + +For example, in the same 8GB RAM example with all 8GB of memory in +active use and the VM itself is completely idle using the same 40 gbps +infiniband li
[Qemu-devel] [PATCH v9 10/14] rdma: introduce capability x-rdma-pin-all
From: "Michael R. Hines"
This capability allows you to disable dynamic chunk registration
for better throughput on high-performance links.
For example, using an 8GB RAM virtual machine with all 8GB of memory in
active use and the VM itself is completely idle using a 40 gbps infiniband link:
1. x-rdma-pin-all disabled total time: approximately 7.5 seconds @ 9.5 Gbps
2. x-rdma-pin-all enabled total time: approximately 4 seconds @ 26 Gbps
These numbers would of course scale up to whatever size virtual machine
you have to migrate using RDMA.
Enabling this feature does *not* have any measurable affect on
migration *downtime*. This is because, without this feature, all of the
memory will have already been registered already in advance during
the bulk round and does not need to be re-registered during the successive
iteration rounds.
Reviewed-by: Paolo Bonzini
Reviewed-by: Eric Blake
Signed-off-by: Michael R. Hines
---
include/migration/migration.h |2 ++
migration.c |9 +
qapi-schema.json |7 ++-
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index d60c64c..2e6001d 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -125,6 +125,8 @@ void migrate_add_blocker(Error *reason);
*/
void migrate_del_blocker(Error *reason);
+bool migrate_rdma_pin_all(void);
+
int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen,
uint8_t *dst, int dlen);
int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen);
diff --git a/migration.c b/migration.c
index 441a3b2..a704d48 100644
--- a/migration.c
+++ b/migration.c
@@ -476,6 +476,15 @@ void qmp_migrate_set_downtime(double value, Error **errp)
max_downtime = (uint64_t)value;
}
+bool migrate_rdma_pin_all(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[MIGRATION_CAPABILITY_X_RDMA_PIN_ALL];
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
diff --git a/qapi-schema.json b/qapi-schema.json
index b239adb..4e9cd6d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -608,10 +608,15 @@
# This feature allows us to minimize migration traffic for certain
work
# loads, by sending compressed difference of the pages
#
+# @x-rdma-pin-all: Controls whether or not the entire VM memory footprint is
+# mlock()'d on demand or all at once. Refer to docs/rdma.txt for
usage.
+# Disabled by default. Experimental: may (or may not) be renamed after
+# further testing is complete. (since 1.5)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
- 'data': ['xbzrle'] }
+ 'data': ['xbzrle', 'x-rdma-pin-all'] }
##
# @MigrationCapabilityStatus
--
1.7.10.4
[Qemu-devel] [PATCH v9 14/14] rdma: add pin-all accounting timestamp to QMP statistics
From: "Michael R. Hines"
For very large virtual machines, pinning can take a long time.
While this does not affect the migration's *actual* time itself,
it is still important for the user to know what's going on and
to know what component of the total time is actual taken up by
pinning.
For example, using a 14GB virtual machine, pinning can take as
long as 5 seconds, for which the user would not otherwise know
what was happening.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
hmp.c |4 +++
include/migration/migration.h |1 +
migration-rdma.c | 55 +++--
migration.c | 13 +-
qapi-schema.json |3 ++-
5 files changed, 56 insertions(+), 20 deletions(-)
diff --git a/hmp.c b/hmp.c
index 148a3fb..90c55f2 100644
--- a/hmp.c
+++ b/hmp.c
@@ -164,6 +164,10 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "downtime: %" PRIu64 " milliseconds\n",
info->downtime);
}
+if (info->has_pin_all_time) {
+monitor_printf(mon, "pin-all: %" PRIu64 " milliseconds\n",
+ info->pin_all_time);
+}
}
if (info->has_ram) {
diff --git a/include/migration/migration.h b/include/migration/migration.h
index b49e68b..d2ca75b 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -49,6 +49,7 @@ struct MigrationState
bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
int64_t xbzrle_cache_size;
double mbps;
+int64_t pin_all_time;
};
void process_incoming_migration(QEMUFile *f);
diff --git a/migration-rdma.c b/migration-rdma.c
index 853de18..e407dce 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -699,11 +699,11 @@ static int qemu_rdma_alloc_qp(RDMAContext *rdma)
return 0;
}
-static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma,
-RDMALocalBlocks *rdma_local_ram_blocks)
+static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
{
int i;
-uint64_t start = qemu_get_clock_ms(rt_clock);
+int64_t start = qemu_get_clock_ms(host_clock);
+RDMALocalBlocks *rdma_local_ram_blocks = &rdma->local_ram_blocks;
(void)start;
for (i = 0; i < rdma_local_ram_blocks->num_blocks; i++) {
@@ -721,7 +721,8 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma,
rdma->total_registrations++;
}
-DPRINTF("lock time: %" PRIu64 "\n", qemu_get_clock_ms(rt_clock) - start);
+DPRINTF("local lock time: %" PRId64 "\n",
+qemu_get_clock_ms(host_clock) - start);
if (i >= rdma_local_ram_blocks->num_blocks) {
return 0;
@@ -1262,7 +1263,8 @@ static void qemu_rdma_move_header(RDMAContext *rdma, int
idx,
*/
static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head,
uint8_t *data, RDMAControlHeader *resp,
- int *resp_idx)
+ int *resp_idx,
+ int (*callback)(RDMAContext *rdma))
{
int ret = 0;
int idx = 0;
@@ -1315,6 +1317,14 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma,
RDMAControlHeader *head,
* If we're expecting a response, block and wait for it.
*/
if (resp) {
+if (callback) {
+DPRINTF("Issuing callback before receiving response...\n");
+ret = callback(rdma);
+if (ret < 0) {
+return ret;
+}
+}
+
DDPRINTF("Waiting for response %s\n", control_desc[resp->type]);
ret = qemu_rdma_exchange_get_response(rdma, resp, resp->type, idx + 1);
@@ -1464,7 +1474,7 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext
*rdma,
chunk, sge.length, current_index, offset);
ret = qemu_rdma_exchange_send(rdma, &head,
-(uint8_t *) &comp, NULL, NULL);
+(uint8_t *) &comp, NULL, NULL, NULL);
if (ret < 0) {
return -EIO;
@@ -1487,7 +1497,7 @@ static int qemu_rdma_write_one(QEMUFile *f, RDMAContext
*rdma,
chunk, sge.length, current_index, offset);
ret = qemu_rdma_exchange_send(rdma, &head, (uint8_t *) ®,
-&resp, ®_result_idx);
+&resp, ®_result_idx, NULL);
if (ret < 0) {
return ret;
}
@@ -2126,7 +2136,7 @@ static int qemu_rdma_put_buffer(void *opaque, const
uint8_t *buf,
head.len = r->len;
head.type = RDMA_CONTROL_QEMU_FILE;
-ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL);
+ret = qemu_rdma_exchange_send(rdma, &head, data, NULL, NULL, NULL);
if (ret < 0) {
rdma->error_state =
[Qemu-devel] [PATCH v9 12/14] rdma: send pc.ram
From: "Michael R. Hines"
This takes advantages of the previous patches:
1. use the new QEMUFileOps hook 'save_page'
2. call out to the right accessor methods to invoke
the iteration hooks defined in QEMUFileOps
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
arch_init.c | 38 +-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch_init.c b/arch_init.c
index 2f1fdd3..ebb601b 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -115,6 +115,7 @@ const uint32_t arch_type = QEMU_ARCH;
#define RAM_SAVE_FLAG_EOS 0x10
#define RAM_SAVE_FLAG_CONTINUE 0x20
#define RAM_SAVE_FLAG_XBZRLE 0x40
+/* 0x80 is reserved in migration.h start with 0x100 next */
static struct defconfig_file {
@@ -447,6 +448,7 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
ram_bulk_stage = false;
}
} else {
+int ret;
uint8_t *p;
int cont = (block == last_sent_block) ?
RAM_SAVE_FLAG_CONTINUE : 0;
@@ -455,7 +457,18 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
/* In doubt sent page as normal */
bytes_sent = -1;
-if (is_zero_page(p)) {
+ret = ram_control_save_page(f, block->offset,
+ offset, TARGET_PAGE_SIZE, &bytes_sent);
+
+if (ret != RAM_SAVE_CONTROL_NOT_SUPP) {
+if (ret != RAM_SAVE_CONTROL_DELAYED) {
+if (bytes_sent > 0) {
+acct_info.norm_pages++;
+} else if (bytes_sent == 0) {
+acct_info.dup_pages++;
+}
+}
+} else if (is_zero_page(p)) {
acct_info.dup_pages++;
if (!ram_bulk_stage) {
bytes_sent = save_block_hdr(f, block, offset, cont,
@@ -610,6 +623,15 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
}
qemu_mutex_unlock_ramlist();
+
+/*
+ * Please leave in place. These calls generate reserved messages in
+ * the RDMA protocol in order to pre-register RDMA memory in the
+ * future before the bulk round begins.
+ */
+ram_control_before_iterate(f, RAM_CONTROL_SETUP);
+ram_control_after_iterate(f, RAM_CONTROL_SETUP);
+
qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
return 0;
@@ -628,6 +650,8 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
reset_ram_globals();
}
+ram_control_before_iterate(f, RAM_CONTROL_ROUND);
+
t0 = qemu_get_clock_ns(rt_clock);
i = 0;
while ((ret = qemu_file_rate_limit(f)) == 0) {
@@ -658,6 +682,12 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
qemu_mutex_unlock_ramlist();
+/*
+ * Must occur before EOS (or any QEMUFile operation)
+ * because of RDMA protocol.
+ */
+ram_control_after_iterate(f, RAM_CONTROL_ROUND);
+
if (ret < 0) {
bytes_transferred += total_sent;
return ret;
@@ -675,6 +705,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
qemu_mutex_lock_ramlist();
migration_bitmap_sync();
+ram_control_before_iterate(f, RAM_CONTROL_FINISH);
+
/* try transferring iterative blocks of memory */
/* flush all remaining blocks regardless of rate limiting */
@@ -688,6 +720,8 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
}
bytes_transferred += bytes_sent;
}
+
+ram_control_after_iterate(f, RAM_CONTROL_FINISH);
migration_end();
qemu_mutex_unlock_ramlist();
@@ -884,6 +918,8 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
ret = -EINVAL;
goto done;
}
+} else if (flags & RAM_SAVE_FLAG_HOOK) {
+ram_control_load_hook(f, flags);
}
error = qemu_file_get_error(f);
if (error) {
--
1.7.10.4
[Qemu-devel] [PATCH v9 07/14] rdma: introduce ram_handle_compressed()
From: "Michael R. Hines"
This gives RDMA shared access to madvise() on the destination side
when an entire chunk is found to be zero.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
arch_init.c | 24
include/migration/migration.h |2 ++
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 819ca5a..2f1fdd3 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -782,6 +782,21 @@ static inline void *host_from_stream_offset(QEMUFile *f,
return NULL;
}
+/*
+ * If a page (or a whole RDMA chunk) has been
+ * determined to be zero, then zap it.
+ */
+void ram_handle_compressed(void *host, uint8_t ch, uint64_t size)
+{
+memset(host, ch, TARGET_PAGE_SIZE);
+#ifndef _WIN32
+if (ch == 0 && (!kvm_enabled() || kvm_has_sync_mmu()) &&
+getpagesize() <= TARGET_PAGE_SIZE) {
+qemu_madvise(host, size, QEMU_MADV_DONTNEED);
+}
+#endif
+}
+
static int ram_load(QEMUFile *f, void *opaque, int version_id)
{
ram_addr_t addr;
@@ -849,14 +864,7 @@ static int ram_load(QEMUFile *f, void *opaque, int
version_id)
}
ch = qemu_get_byte(f);
-memset(host, ch, TARGET_PAGE_SIZE);
-#ifndef _WIN32
-if (ch == 0 &&
-(!kvm_enabled() || kvm_has_sync_mmu()) &&
-getpagesize() <= TARGET_PAGE_SIZE) {
-qemu_madvise(host, TARGET_PAGE_SIZE, QEMU_MADV_DONTNEED);
-}
-#endif
+ram_handle_compressed(host, ch, TARGET_PAGE_SIZE);
} else if (flags & RAM_SAVE_FLAG_PAGE) {
void *host;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 1eeaa40..110f4ad 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -108,6 +108,8 @@ uint64_t xbzrle_mig_pages_transferred(void);
uint64_t xbzrle_mig_pages_overflow(void);
uint64_t xbzrle_mig_pages_cache_miss(void);
+void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);
+
/**
* @migrate_add_blocker - prevent migration from proceeding
*
--
1.7.10.4
Re: [Qemu-devel] RDMA: please pull and re-test freezing fixes
Chegu, I sent a V9 to the mailing list: The version goes even further, by explicitly timing the pinning latency and pushing the value out to QMP so the user clearly knows which component of total migration time is consumed by pinning. If you're satisfied, I'd appreciate if I could add your Reviewed-By: =) - Michael On 06/14/2013 02:30 AM, Michael R. Hines wrote: Chegu, I believe I've fixed all the pinning / freezing issues per Paolo's recommendations. I've re-submitted as v8 - Can you "git pull" from github and re-test? Thanks, - Michael
[Qemu-devel] [PATCH v9 02/14] rdma: introduce qemu_update_position()
From: "Michael R. Hines"
RDMA writes happen asynchronously, and thus the performance accounting
also needs to be able to occur asynchronously. This allows anybody
to call into savevm.c to update both f->pos as well as into arch_init.c
to update the acct_info structure with up-to-date values when
the RDMA transfer actually completes.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
arch_init.c | 12
include/migration/migration.h |2 ++
include/migration/qemu-file.h |1 +
savevm.c |5 +
4 files changed, 20 insertions(+)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..819ca5a 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -498,6 +498,18 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
static uint64_t bytes_transferred;
+void acct_update_position(QEMUFile *f, size_t size, bool zero)
+{
+uint64_t pages = size / TARGET_PAGE_SIZE;
+if (zero) {
+acct_info.dup_pages += pages;
+} else {
+acct_info.norm_pages += pages;
+bytes_transferred += size;
+qemu_update_position(f, size);
+}
+}
+
static ram_addr_t ram_save_remaining(void)
{
return migration_dirty_pages;
diff --git a/include/migration/migration.h b/include/migration/migration.h
index e2acec6..0be28a2 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -92,6 +92,8 @@ uint64_t ram_bytes_remaining(void);
uint64_t ram_bytes_transferred(void);
uint64_t ram_bytes_total(void);
+void acct_update_position(QEMUFile *f, size_t size, bool zero);
+
extern SaveVMHandlers savevm_ram_handlers;
uint64_t dup_mig_bytes_transferred(void);
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index 7519464..8fab0dd 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -93,6 +93,7 @@ void qemu_put_be32(QEMUFile *f, unsigned int v);
void qemu_put_be64(QEMUFile *f, uint64_t v);
int qemu_get_buffer(QEMUFile *f, uint8_t *buf, int size);
int qemu_get_byte(QEMUFile *f);
+void qemu_update_position(QEMUFile *f, size_t size);
static inline unsigned int qemu_get_ubyte(QEMUFile *f)
{
diff --git a/savevm.c b/savevm.c
index 2ce439f..2983905 100644
--- a/savevm.c
+++ b/savevm.c
@@ -670,6 +670,11 @@ int qemu_get_fd(QEMUFile *f)
return -1;
}
+void qemu_update_position(QEMUFile *f, size_t size)
+{
+f->pos += size;
+}
+
/** Closes the file
*
* Returns negative error value if any error happened on previous operations or
--
1.7.10.4
[Qemu-devel] [PATCH v9 00/14] rdma: migration support
From: "Michael R. Hines"
Changes since v8:
For very large virtual machines, pinning can take a long time.
While this does not affect the migration's *actual* time itself,
it is still important for the user to know what's going on and
to know what component of the total time is actual taken up by
pinning.
For example, using a 14GB virtual machine, pinning can take as
long as 5 seconds, for which the user would not otherwise know
what was happening.
Reviewed-by: Eric Blake
Reviewed-by: Paolo Bonzini
Wiki: http://wiki.qemu.org/Features/RDMALiveMigration
Github: g...@github.com:hinesmr/qemu.git
Here is a brief summary of total migration time and downtime using RDMA:
Using a 40gbps infiniband link performing a worst-case stress test,
using an 8GB RAM virtual machine:
Using the following command:
$ apt-get install stress
$ stress --vm-bytes 7500M --vm 1 --vm-keep
RESULTS:
1. Migration throughput: 26 gigabits/second.
2. Downtime (stop time) varies between 15 and 100 milliseconds.
EFFECTS of memory registration on bulk phase round:
For example, in the same 8GB RAM example with all 8GB of memory in
active use and the VM itself is completely idle using the same 40 gbps
infiniband link:
1. x-rdma-pin-all disabled total time: approximately 7.5 seconds @ 9.5 Gbps
2. x-rdma-pin-all enabled total time: approximately 4 seconds @ 26 Gbps
These numbers would of course scale up to whatever size virtual machine
you have to migrate using RDMA.
Enabling this feature does *not* have any measurable affect on
migration *downtime*. This is because, without this feature, all of the
memory will have already been registered already in advance during
the bulk round and does not need to be re-registered during the successive
iteration rounds.
The following changes since commit f3aa844bbb2922a5b8393d17620eca7d7e921ab3:
build: include config-{, all-}devices.mak after defining CONFIG_SOFTMMU and
CONFIG_USER_ONLY (2013-04-24 12:18:41 -0500)
are available in the git repository at:
g...@github.com:hinesmr/qemu.git rdma_patch_v9
for you to fetch changes up to 75e6fac1f642885b93cefe6e1874d648e9850f8f:
rdma: send pc.ram (2013-04-24 14:55:01 -0400)
Michael R. Hines (14):
rdma: add documentation
rdma: introduce qemu_update_position()
rdma: export yield_until_fd_readable()
rdma: export throughput w/ MigrationStats QMP
rdma: introduce qemu_file_mode_is_not_valid()
rdma: export qemu_fflush()
rdma: introduce ram_handle_compressed()
rdma: introduce qemu_ram_foreach_block()
rdma: new QEMUFileOps hooks
rdma: introduce capability x-rdma-pin-all
rdma: core logic
rdma: send pc.ram
rdma: fix mlock() freezes and accounting
rdma: add pin-all accounting timestamp to QMP statistics
Makefile.objs |1 +
arch_init.c | 69 +-
configure | 29 +
docs/rdma.txt | 415 ++
exec.c|9 +
hmp.c |6 +
include/block/coroutine.h |6 +
include/exec/cpu-common.h |5 +
include/migration/migration.h | 32 +
include/migration/qemu-file.h | 32 +
migration-rdma.c | 2831 +
migration.c | 36 +-
qapi-schema.json | 15 +-
qemu-coroutine-io.c | 23 +
savevm.c | 114 +-
15 files changed, 3574 insertions(+), 49 deletions(-)
create mode 100644 docs/rdma.txt
create mode 100644 migration-rdma.c
--
1.7.10.4
[Qemu-devel] [PATCH v9 13/14] rdma: fix mlock() freezes and accounting
From: "Michael R. Hines"
This patch is contained to migration-rdma.c and fixes the problems
experienced by others when the x-rdma-pin-all feature appeared to
freeze the VM. By moving this operation out of the connection setup
time and instead moving it to ram_save_setup() code, we no longer
execute pinning inside the BQL and thus the pinning is parallelized
with the VM execution and also properly accounted for inside the
QMP migrate total time statistics.
Reviewed-by: Paolo Bonzini
Signed-off-by: Michael R. Hines
---
arch_init.c |5 --
docs/rdma.txt| 31 +---
migration-rdma.c | 211 +++---
3 files changed, 128 insertions(+), 119 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index ebb601b..a1557d2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -624,11 +624,6 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
qemu_mutex_unlock_ramlist();
-/*
- * Please leave in place. These calls generate reserved messages in
- * the RDMA protocol in order to pre-register RDMA memory in the
- * future before the bulk round begins.
- */
ram_control_before_iterate(f, RAM_CONTROL_SETUP);
ram_control_after_iterate(f, RAM_CONTROL_SETUP);
diff --git a/docs/rdma.txt b/docs/rdma.txt
index 4f98a3b..09d36ef 100644
--- a/docs/rdma.txt
+++ b/docs/rdma.txt
@@ -76,6 +76,13 @@ of the migration, which can greatly reduce the "total" time
of your migration.
Example performance of this using an idle VM in the previous example
can be found in the "Performance" section.
+Note: for very large virtual machines (hundreds of GBs), pinning all
+*all* of the memory of your virtual machine in the kernel is very expensive
+may extend the initial bulk iteration time by many seconds,
+and thus extending the total migration time. However, this will not
+affect the determinism or predictability of your migration you will
+still gain from the benefits of advanced pinning with RDMA.
+
RUNNING:
@@ -195,22 +202,26 @@ The maximum number of repeats is hard-coded to 4096. This
is a conservative
limit based on the maximum size of a SEND message along with emperical
observations on the maximum future benefit of simultaneous page registrations.
-The 'type' field has 9 different command values:
+The 'type' field has 10 different command values:
1. Unused
-2. Error (sent to the source during bad things)
-3. Ready (control-channel is available)
-4. QEMU File (for sending non-live device state)
-5. RAM Blocks(used right after connection setup)
-6. Compress page (zap zero page and skip registration)
-7. Register request (dynamic chunk registration)
-8. Register result ('rkey' to be used by sender)
-9. Register finished (registration for current iteration finished)
+2. Error (sent to the source during bad things)
+3. Ready (control-channel is available)
+4. QEMU File (for sending non-live device state)
+5. RAM Blocks request (used right after connection setup)
+6. RAM Blocks result (used right after connection setup)
+7. Compress page (zap zero page and skip registration)
+8. Register request (dynamic chunk registration)
+9. Register result('rkey' to be used by sender)
+10. Register finished (registration for current iteration finished)
A single control message, as hinted above, can contain within the data
portion an array of many commands of the same type. If there is more than
one command, then the 'repeat' field will be greater than 1.
-After connection setup is completed, we have two protocol-level
+After connection setup, message 5 & 6 are used to exchange ram block
+information and optionally pin all the memory if requested by the user.
+
+After ram block exchange is completed, we have two protocol-level
functions, responsible for communicating control-channel commands
using the above list of values:
diff --git a/migration-rdma.c b/migration-rdma.c
index 79aaa0a..853de18 100644
--- a/migration-rdma.c
+++ b/migration-rdma.c
@@ -146,13 +146,14 @@ const char *wrid_desc[] = {
enum {
RDMA_CONTROL_NONE = 0,
RDMA_CONTROL_ERROR,
-RDMA_CONTROL_READY, /* ready to receive */
-RDMA_CONTROL_QEMU_FILE, /* QEMUFile-transmitted bytes */
-RDMA_CONTROL_RAM_BLOCKS,/* RAMBlock synchronization */
-RDMA_CONTROL_COMPRESS, /* page contains repeat values */
-RDMA_CONTROL_REGISTER_REQUEST, /* dynamic page registration */
-RDMA_CONTROL_REGISTER_RESULT, /* key to use after registration */
-RDMA_CONTROL_REGISTER_FINISHED, /* current iteration finished */
+RDMA_CONTROL_READY, /* ready to receive */
+RDMA_CONTROL_QEMU_FILE, /* QEMUFile-transmitted bytes */
+RDMA_CONTROL_RAM_BLOCKS_REQUEST, /* RAMBlock synchronization */
+RDMA_CONTROL_RAM_BLOCKS_RESULT, /* RA
[Qemu-devel] [PATCH] Revert "Unbreak -no-quit for GTK, validate SDL options"
This reverts commit 047d4e151dd462915786a4fddc12f774d0028af5.
The commit in question introduced old/legacy options (-no-quit
and some more) for the new interface type (gtk), -- on the
second thought I think we shouldn't do that, instead, we should
use the display-specific options (like -display sdl,frame=no),
and should not extend the legacy interface further which makes
messy quite fast.
And more, that change broke build without sdl, by referring to
`no_frame' variable which were guarded by #if SDL.
So reverting it for now I thinks is the best option. Further
plan, I think, is to make the documentation more clear that
these options are legacy.
Cc: Peter Wu
Cc: qemu-triv...@nongnu.org
Signed-off-By: Michael Tokarev
---
vl.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/vl.c b/vl.c
index 9f8fd6e..169c807 100644
--- a/vl.c
+++ b/vl.c
@@ -3524,6 +3524,7 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_full_screen:
full_screen = 1;
break;
+#ifdef CONFIG_SDL
case QEMU_OPTION_no_frame:
no_frame = 1;
break;
@@ -3536,11 +3537,14 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_no_quit:
no_quit = 1;
break;
-#ifdef CONFIG_SDL
case QEMU_OPTION_sdl:
display_type = DT_SDL;
break;
#else
+case QEMU_OPTION_no_frame:
+case QEMU_OPTION_alt_grab:
+case QEMU_OPTION_ctrl_grab:
+case QEMU_OPTION_no_quit:
case QEMU_OPTION_sdl:
fprintf(stderr, "SDL support is disabled\n");
exit(1);
@@ -4081,15 +4085,6 @@ int main(int argc, char **argv, char **envp)
#endif
}
-if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
-fprintf(stderr, "-no-frame, -alt-grab and -ctrl-grab are only valid "
-"for SDL, ignoring option\n");
-}
-if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
-fprintf(stderr, "-no-quit is only valid for GTK and SDL, "
-"ignoring option\n");
-}
-
#if defined(CONFIG_GTK)
if (display_type == DT_GTK) {
early_gtk_display_init();
--
1.7.10.4
Re: [Qemu-devel] [PATCH] vl: fix compilation without SDL
Am 14.06.2013 20:12, schrieb Vincent Stehlé:
> Fix the following error, which happens when CONFIG_SDL is not defined:
>
> vl.c: In function ‘main’:
> vl.c:3528:17: error: ‘no_frame’ undeclared (first use in this function)
> vl.c:3528:17: note: each undeclared identifier is reported only once for
> each function it appears in
This problem was recently introduced by commit
047d4e151dd462915786a4fddc12f774d0028af5
(Unbreak -no-quit for GTK, validate SDL options).
Your patch makes SDL command line options which
were allowed until now invalid, so it's the wrong
kind of fix.
Declaring no_frame unconditionally would be a better
solution.
Regards
Stefan Weil
> Protect code accessing no_frame and while at it, protect even more code
> related
> to alt_grab and ctrl_grab.
>
> Signed-off-by: Vincent Stehlé
> Cc: Anthony Liguori
> ---
> vl.c |7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 9f8fd6e..67de399 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -229,8 +229,10 @@ int nb_option_roms;
> int semihosting_enabled = 0;
> int old_param = 0;
> const char *qemu_name;
> +#ifdef CONFIG_SDL
> int alt_grab = 0;
> int ctrl_grab = 0;
> +#endif
> unsigned int nb_prom_envs = 0;
> const char *prom_envs[MAX_PROM_ENVS];
> int boot_menu;
> @@ -3524,6 +3526,7 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_full_screen:
> full_screen = 1;
> break;
> +#ifdef CONFIG_SDL
> case QEMU_OPTION_no_frame:
> no_frame = 1;
> break;
> @@ -3533,6 +3536,7 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_ctrl_grab:
> ctrl_grab = 1;
> break;
> +#endif
> case QEMU_OPTION_no_quit:
> no_quit = 1;
> break;
> @@ -4080,11 +4084,12 @@ int main(int argc, char **argv, char **envp)
> display_type = DT_NONE;
> #endif
> }
> -
> +#ifdef CONFIG_SDL
> if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
> fprintf(stderr, "-no-frame, -alt-grab and -ctrl-grab are only valid "
> "for SDL, ignoring option\n");
> }
> +#endif
> if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
> fprintf(stderr, "-no-quit is only valid for GTK and SDL, "
> "ignoring option\n");
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
Corey Bryant writes: > On 06/14/2013 11:56 AM, Anthony Liguori wrote: >> Corey Bryant writes: >> >>> On 06/14/2013 11:38 AM, Anthony Liguori wrote: Corey Bryant writes: > On 06/14/2013 10:01 AM, Anthony Liguori wrote: >> Corey Bryant writes: >> >>> This patch series provides persistent storage support that a TPM >>> can use to store NVRAM data. It uses QEMU's block driver to store >>> data on a drive image. The libtpms TPM 1.2 backend will be the >>> initial user of this functionality to store data that must persist >>> through a reboot or migration. A sample command line may look like >>> this: >> >> This should be folded into the libtpms backend series. >> >> There are no users for this so this would just be untestable code in the >> tree subject to bitrot. >> >> Regards, >> >> Anthony Liguori >> > > Fair enough. I assume you're ok with this code though? I don't understand why it's needed to be honest. I suspect this has to do with the fact that the libtpms implementation will need significant reworking. Regards, Anthony Liguori >>> >>> In regards to why it is needed.. The QEMU software-emulated vTPM >>> backend will pass callback functions to libtpms for writing/reading >>> nvram data. Those callbacks will use the code in this patch series to >>> do the writing/reading of nvram data to/from image files so that the >>> data persists through migration/reboot. >>> >>> I'm not sure I completely understand your second sentence, but yes the >>> software-emulated vTPM backend code for QEMU will certainly need rework >>> to use the code in this patch series. >> >> I think it's easiest to discuss this in the context of the actual patch >> series. >> >> Regards, >> >> Anthony Liguori >> > > I suppose, but the earlier we can get feedback the better so that we > don't waste any more time. This NVRAM code alone has gone through far > too many iterations as folks have asked for it to go in different > directions, and we went in those directions to find that they were the > wrong directions. Yes, it's iterating because the context is missing. > Anyway, for the record, this latest patch series > adheres to the direction you suggested we take last month: > http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg04275.html No, my last suggestion was to just do bdrv_aio_write() within the device itself. I realize there is some complexity because libtpms is threaded and this is exactly why the context matters so much. But code isn't merged unless it's useful on its own. This code is not useful on its own. Regards, Anthony Liguori > > -- > Regards, > Corey Bryant > >>> >>> -- >>> Regards, >>> Corey Bryant >>> > > -- > Regards, > Corey Bryant > >>> >>> qemu-system-x86_64 ... >>> -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 >>> -tpmdev libtpms,id=tpm-tpm0 >>> -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 >>> >>> Thanks, >>> Corey >>> >>> Corey Bryant (3): >>> nvram: Add TPM NVRAM implementation >>> nvram: Add tpm-tis drive support >>> TPM NVRAM test >>> >>> hw/tpm/Makefile.objs |1 + >>> hw/tpm/tpm_int.h |2 + >>> hw/tpm/tpm_nvram.c | 324 >>> ++ >>> hw/tpm/tpm_nvram.h | 25 >>> hw/tpm/tpm_passthrough.c | 85 >>> hw/tpm/tpm_tis.c |8 + >>> 6 files changed, 445 insertions(+), 0 deletions(-) >>> create mode 100644 hw/tpm/tpm_nvram.c >>> create mode 100644 hw/tpm/tpm_nvram.h >> >> >> >> >> >> >>
Re: [Qemu-devel] [PATCH] make screendump an async command
Alon Levy writes:
> This fixes the broken screendump behavior for qxl devices in native mode
> since 81fb6f1504fb9ef71f2382f44af34756668296e8.
>
> Note: due to QAPI not generating async commands yet I had to remove the
> schema screendump definition.
>
> Related RHBZ: 973374
> This patch is not enough to fix said bz, with the linux qxl driver you get
> garbage still, just not out of date garbage.
>
> Signed-off-by: Alon Levy
Async commands are badly broken with respect to error handling.
This needs to be done after we get the new QMP server.
Why is QXL unable to do a synchronous screendump?
Regards,
Anthony Liguori
> ---
> hmp.c | 2 +-
> hw/display/qxl-render.c | 1 +
> hw/display/vga.c | 1 +
> include/qapi/qmp/qerror.h | 6 +
> include/ui/console.h | 10
> qapi-schema.json | 13 ---
> qmp-commands.hx | 3 ++-
> ui/console.c | 58
> ++-
> 8 files changed, 78 insertions(+), 16 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index 494a9aa..2a37975 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1346,7 +1346,7 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
> const char *filename = qdict_get_str(qdict, "filename");
> Error *err = NULL;
>
> -qmp_screendump(filename, &err);
> +hmp_screen_dump_helper(filename, &err);
> hmp_handle_error(mon, &err);
> }
>
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index f511a62..d719448 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -139,6 +139,7 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice
> *qxl)
> qxl->dirty[i].bottom - qxl->dirty[i].top);
> }
> qxl->num_dirty_rects = 0;
> +console_screendump_complete(vga->con);
> }
>
> /*
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 21a108d..1fc52eb 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1922,6 +1922,7 @@ static void vga_update_display(void *opaque)
> break;
> }
> }
> +console_screendump_complete(s->con);
> }
>
> /* force a full display refresh */
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 6c0a18d..1bd7efa 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -237,6 +237,12 @@ void assert_no_error(Error *err);
> #define QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION \
> ERROR_CLASS_GENERIC_ERROR, "Migration is disabled when VirtFS export
> path '%s' is mounted in the guest using mount_tag '%s'"
>
> +#define QERR_SCREENDUMP_NOT_AVAILABLE \
> +ERROR_CLASS_GENERIC_ERROR, "There is no QemuConsole I can screendump
> from"
> +
> +#define QERR_SCREENDUMP_IN_PROGRESS \
> +ERROR_CLASS_GENERIC_ERROR, "Existing screendump in progress"
> +
> #define QERR_SOCKET_CONNECT_FAILED \
> ERROR_CLASS_GENERIC_ERROR, "Failed to connect to socket"
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 4307b5f..643da45 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -341,4 +341,14 @@ int index_from_keycode(int code);
> void early_gtk_display_init(void);
> void gtk_display_init(DisplayState *ds);
>
> +/* hw/display/vga.c hw/display/qxl.c */
> +void console_screendump_complete(QemuConsole *con);
> +
> +/* monitor.c */
> +int qmp_screendump(Monitor *mon, const QDict *qdict,
> + MonitorCompletion cb, void *opaque);
> +
> +/* hmp.c */
> +void hmp_screen_dump_helper(const char *filename, Error **errp);
> +
> #endif
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5ad6894..f5fdc2f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3112,19 +3112,6 @@
>'data': { 'keys': ['KeyValue'], '*hold-time': 'int' } }
>
> ##
> -# @screendump:
> -#
> -# Write a PPM of the VGA screen to a file.
> -#
> -# @filename: the path of a new PPM file to store the image
> -#
> -# Returns: Nothing on success
> -#
> -# Since: 0.14.0
> -##
> -{ 'command': 'screendump', 'data': {'filename': 'str'} }
> -
> -##
> # @nbd-server-start:
> #
> # Start an NBD server listening on the given host and port. Block
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8cea5e5..bde8f43 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -146,7 +146,8 @@ EQMP
> {
> .name = "screendump",
> .args_type = "filename:F",
> -.mhandler.cmd_new = qmp_marshal_input_screendump,
> +.mhandler.cmd_async = qmp_screendump,
> +.flags = MONITOR_CMD_ASYNC,
> },
>
> SQMP
> diff --git a/ui/console.c b/ui/console.c
> index 28bba6d..0efd588 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -116,6 +116,12 @@ typedef enum {
> struct QemuConsole {
> Object parent;
>
> +struct {
> +char *filename;
> +MonitorCompletion *completion_cb;
> +void *completion_opaque;
> +} screendump;
> +
> int index;
>
Re: [Qemu-devel] [PULL 21/26] ide-test: Add FLUSH CACHE test case
Peter Maydell writes: > On 14 June 2013 18:54, Peter Maydell wrote: >> On 7 June 2013 12:58, Stefan Hajnoczi wrote: >>> From: Kevin Wolf >>> >>> This checks in particular that BSY is set while the flush request is in >>> flight. >> >> This test doesn't seem to pass. > >> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/ide-test.c:460:test_flush: >> assertion failed ((data) & (BSY | DF | ERR | DRQ) == 0): (0x0080 >> == 0x) > > ...fixed by http://patchwork.ozlabs.org/patch/250334/ I'm testing it myself now. Since it's already in the block tree, I'll push it directly as it's blocking my queue. Unless Kevin or Stefan are planning on sending a pull request today of course. Regards, Anthony Liguori > > thanks > -- PMM
[Qemu-devel] [PATCH] vl: fix compilation without SDL
Fix the following error, which happens when CONFIG_SDL is not defined:
vl.c: In function ‘main’:
vl.c:3528:17: error: ‘no_frame’ undeclared (first use in this function)
vl.c:3528:17: note: each undeclared identifier is reported only once for each
function it appears in
Protect code accessing no_frame and while at it, protect even more code related
to alt_grab and ctrl_grab.
Signed-off-by: Vincent Stehlé
Cc: Anthony Liguori
---
vl.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/vl.c b/vl.c
index 9f8fd6e..67de399 100644
--- a/vl.c
+++ b/vl.c
@@ -229,8 +229,10 @@ int nb_option_roms;
int semihosting_enabled = 0;
int old_param = 0;
const char *qemu_name;
+#ifdef CONFIG_SDL
int alt_grab = 0;
int ctrl_grab = 0;
+#endif
unsigned int nb_prom_envs = 0;
const char *prom_envs[MAX_PROM_ENVS];
int boot_menu;
@@ -3524,6 +3526,7 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_full_screen:
full_screen = 1;
break;
+#ifdef CONFIG_SDL
case QEMU_OPTION_no_frame:
no_frame = 1;
break;
@@ -3533,6 +3536,7 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_ctrl_grab:
ctrl_grab = 1;
break;
+#endif
case QEMU_OPTION_no_quit:
no_quit = 1;
break;
@@ -4080,11 +4084,12 @@ int main(int argc, char **argv, char **envp)
display_type = DT_NONE;
#endif
}
-
+#ifdef CONFIG_SDL
if ((no_frame || alt_grab || ctrl_grab) && display_type != DT_SDL) {
fprintf(stderr, "-no-frame, -alt-grab and -ctrl-grab are only valid "
"for SDL, ignoring option\n");
}
+#endif
if (no_quit && (display_type != DT_GTK && display_type != DT_SDL)) {
fprintf(stderr, "-no-quit is only valid for GTK and SDL, "
"ignoring option\n");
--
1.7.10.4
Re: [Qemu-devel] [PULL 21/26] ide-test: Add FLUSH CACHE test case
On 14 June 2013 18:54, Peter Maydell wrote: > On 7 June 2013 12:58, Stefan Hajnoczi wrote: >> From: Kevin Wolf >> >> This checks in particular that BSY is set while the flush request is in >> flight. > > This test doesn't seem to pass. > ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/ide-test.c:460:test_flush: > assertion failed ((data) & (BSY | DF | ERR | DRQ) == 0): (0x0080 > == 0x) ...fixed by http://patchwork.ozlabs.org/patch/250334/ thanks -- PMM
Re: [Qemu-devel] [PATCH] make screendump an async command
> > Hi,
> >
> > > Note: due to QAPI not generating async commands yet I had to remove the
> > > schema screendump definition.
> >
> > Hmm, that will break libvirt I suspect. Guess this one has to wait
> > until QAPI gained proper async command support.
>
> It doesn't break anything. I've tested,
To clarify, I tested with "virsh screenshot" as well.
> and here is the QMP transcript, the
> only difference is internal to qemu, from libvirt point of view it issues
> the same command and gets the same response:
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio
> {"QMP": {"version": {"qemu": {"micro": 50, "minor": 5, "major": 1},
> "package": ""}, "capabilities": []}}
> {"execute":"qmp_capabilities"}
> {"return": {}}
> {"execute":"screendump","arguments":{"filename":"test2.ppm"}}
> {"return": {}}
>
>
> >
> > cheers,
> > Gerd
> >
> >
> >
> >
>
>
Re: [Qemu-devel] [PATCH] make screendump an async command
> Hi,
>
> > Note: due to QAPI not generating async commands yet I had to remove the
> > schema screendump definition.
>
> Hmm, that will break libvirt I suspect. Guess this one has to wait
> until QAPI gained proper async command support.
It doesn't break anything. I've tested, and here is the QMP transcript, the
only difference is internal to qemu, from libvirt point of view it issues the
same command and gets the same response:
$ ./x86_64-softmmu/qemu-system-x86_64 -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 5, "major": 1}, "package":
""}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"screendump","arguments":{"filename":"test2.ppm"}}
{"return": {}}
>
> cheers,
> Gerd
>
>
>
>
Re: [Qemu-devel] [PULL 21/26] ide-test: Add FLUSH CACHE test case
On 7 June 2013 12:58, Stefan Hajnoczi wrote:
> From: Kevin Wolf
>
> This checks in particular that BSY is set while the flush request is in
> flight.
This test doesn't seem to pass. At commit bd07684aac
(where the test landed):
cam-vm-266:precise:qemu$ (cd build/x86;
QTEST_QEMU_BINARY=i386-softmmu/qemu-system-i386
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k
--verbose -m=quick tests/ide-test)
TEST: tests/ide-test... (pid=25118)
/i386/ide/identify: OK
/i386/ide/bmdma/setup: OK
/i386/ide/bmdma/simple_rw: OK
/i386/ide/bmdma/short_prdt: OK
/i386/ide/bmdma/long_prdt: OK
/i386/ide/bmdma/teardown:OK
/i386/ide/flush:
blkdebug: Suspended request 'A'
blkdebug: Resuming request 'A'
**
ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/tests/ide-test.c:460:test_flush:
assertion failed ((data) & (BSY | DF | ERR | DRQ) == 0): (0x0080
== 0x)
FAIL
GTester: last random seed: R02S67ac87128346f4ba02226525fc3ecccf
(pid=25136)
FAIL: tests/ide-test
thanks
-- PMM
Re: [Qemu-devel] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header
On Fri, Jun 14, 2013 at 06:36:06PM +0100, Peter Maydell wrote: > On 14 June 2013 18:14, Sören Brinkmann wrote: > > On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote: > >> If we're providing separate functions for kernel and > >> ramdisk loading shouldn't they check that the uimage > >> has the appropriate type? > > I'm not sure I understand what you mean here. Moving the check for the > > appropriate u-boot header type down here? I tried to find some > > reasonable way to further split the load_uboot_image() routine to move > > some of it's functionality into the two functions down here. But it all > > seemed pretty messy and I left pretty much all functionality in > > load_uboot_image() and just pass errors through. > > I had in mind that these functions should pass the "expected" > type byte in to the load_uboot_image() function so it can > fail if the blob it is passed isn't actually the right type > (eg you pass the kernel to -initrd and vice-versa). Thanks for the clarification. I agree. I'll add such a check. Sören
Re: [Qemu-devel] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header
On 14 June 2013 18:14, Sören Brinkmann wrote: > On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote: >> If we're providing separate functions for kernel and >> ramdisk loading shouldn't they check that the uimage >> has the appropriate type? > I'm not sure I understand what you mean here. Moving the check for the > appropriate u-boot header type down here? I tried to find some > reasonable way to further split the load_uboot_image() routine to move > some of it's functionality into the two functions down here. But it all > seemed pretty messy and I left pretty much all functionality in > load_uboot_image() and just pass errors through. I had in mind that these functions should pass the "expected" type byte in to the load_uboot_image() function so it can fail if the blob it is passed isn't actually the right type (eg you pass the kernel to -initrd and vice-versa). thanks -- PMM
Re: [Qemu-devel] [PATCH] arm/boot: Free dtb blob memory after use
On 14 June 2013 18:14, Andreas Färber wrote:
> Am 14.06.2013 13:27, schrieb Peter Maydell:
>> The dtb blob returned by load_device_tree() is in memory allocated
>> with g_malloc(). Free it accordingly once we have copied its
>> contents into the guest memory. To make this easy, we need also to
>> clean up the error handling in load_dtb() so that we consistently
>> handle errors in the same way (by printing a message and then
>> returning -1, rather than either plowing on or exiting immediately).
>>
>> Signed-off-by: Peter Maydell
>
> Cc: qemu-sta...@nongnu.org ?
It's only a once-per-run leak of a blob that's going to be <10K
in size, so it doesn't really seem worth worrying about for stable.
>> ---
>> hw/arm/boot.c | 20 +++-
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index f8c2031..f5870f6 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -238,14 +238,14 @@ static int load_dtb(hwaddr addr, const struct
>> arm_boot_info *binfo)
>> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
>> if (!filename) {
>> fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
>> -return -1;
>> +goto fail;
>
> fdt is NULL-initialized, so this is OK.
>
>> }
>>
>> fdt = load_device_tree(filename, &size);
>> if (!fdt) {
>> fprintf(stderr, "Couldn't open dtb file %s\n", filename);
>> g_free(filename);
>> -return -1;
>> +goto fail;
>> }
>> g_free(filename);
>>
>
> Personally I would've left those two unchanged for clarity, but your
> call. ;)
I liked the consistency of having every error path go the same way.
> arm_load_kernel() does exit(1) on failure, so only functional change is
> not ignoring failure to set memory, cmdline and initrd properties.
>
> Rest looks okay, except that I wonder whether we might later want to
> propagate these errors up the call chain and therefore use error_setg()
> and void here and a more central fprintf() in arm_load_kernel() - we
> surely don't want to duplicate it into each board even though there
> being four other fprintf()s in arm_load_kernel().
Mmm. Certainly now the error functions can handle ad-hoc
strings this is more attractive than when the code was first
written. I'm not sure it's worthwhile unless we want to move
in the direction of having all board init errors passed up
to the top level vl.c code, though.
> But that's orthogonal to making fdt errors fatal and the fdt cleanup, so
>
> Reviewed-by: Andreas Färber
Thanks.
-- PMM
Re: [Qemu-devel] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header
Hi Peter,
On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
> On 14 June 2013 17:36, Soren Brinkmann wrote:
> > Introduce 'load_ramdisk()' which can load "normal" ramdisks and ramdisks
> > with a u-boot header.
> > To enable this and leverage synergies 'load_uimage()' is refactored to
> > accomodate this additional use case.
>
> Hi; thanks for this patch; some review comments below.
>
> >
> > Signed-off-by: Soren Brinkmann
> > ---
> > hw/core/loader.c| 86
> > +
> > include/hw/loader.h | 1 +
> > 2 files changed, 61 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/core/loader.c b/hw/core/loader.c
> > index 7507914..e72d682 100644
> > --- a/hw/core/loader.c
> > +++ b/hw/core/loader.c
> > @@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen,
> > uint8_t *src,
> > }
> >
> > /* Load a U-Boot image. */
> > -int load_uimage(const char *filename, hwaddr *ep,
> > -hwaddr *loadaddr, int *is_linux)
> > +static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr
> > *loadaddr,
> > +int *is_linux)
> > {
> > int fd;
> > int size;
> > +hwaddr address;
> > uboot_image_header_t h;
> > uboot_image_header_t *hdr = &h;
> > uint8_t *data = NULL;
> > int ret = -1;
> > +int do_uncompress = 0;
> >
> > fd = open(filename, O_RDONLY | O_BINARY);
> > if (fd < 0)
> > @@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
> > goto out;
> >
> > /* TODO: Implement other image types. */
>
> Doesn't this patch fix this TODO item?
Well, partly. There are a couple of more image types which are still
not supported. So, I didnt' bother touching this comment just because
this adds support for a second out of 10 or something types.
>
> > -if (hdr->ih_type != IH_TYPE_KERNEL) {
> > -fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
> > -goto out;
> > -}
> > +switch (hdr->ih_type) {
> > +case IH_TYPE_KERNEL:
> > +address = hdr->ih_load;
> > +if (loadaddr) {
> > +*loadaddr = hdr->ih_load;
> > +}
> > +
> > +switch (hdr->ih_comp) {
> > +case IH_COMP_NONE:
> > +break;
> > +case IH_COMP_GZIP:
> > +do_uncompress = 1;
> > +break;
> > +default:
> > +fprintf(stderr,
> > +"Unable to load u-boot images with compression type
> > %d\n",
> > +hdr->ih_comp);
> > +goto out;
> > +}
> > +
> > +if (ep) {
> > +*ep = hdr->ih_ep;
> > +}
> >
> > -switch (hdr->ih_comp) {
> > -case IH_COMP_NONE:
> > -case IH_COMP_GZIP:
> > +/* TODO: Check CPU type. */
> > +if (is_linux) {
> > +if (hdr->ih_os == IH_OS_LINUX) {
> > +*is_linux = 1;
> > +} else {
> > +*is_linux = 0;
> > +}
> > +}
> > +
> > +break;
> > +case IH_TYPE_RAMDISK:
> > +address = *loadaddr;
> > break;
> > default:
> > -fprintf(stderr,
> > -"Unable to load u-boot images with compression type %d\n",
> > -hdr->ih_comp);
> > +fprintf(stderr, "Unsupported u-boot image type\n");
>
> You could include the type byte here, the way we do for
> unknown compression types.
good call. I'll add that.
>
> > goto out;
> > }
> >
> > -/* TODO: Check CPU type. */
> > -if (is_linux) {
> > -if (hdr->ih_os == IH_OS_LINUX)
> > -*is_linux = 1;
> > -else
> > -*is_linux = 0;
> > -}
> > -
> > -*ep = hdr->ih_ep;
> > data = g_malloc(hdr->ih_size);
> >
> > if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> > @@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> > goto out;
> > }
> >
> > -if (hdr->ih_comp == IH_COMP_GZIP) {
> > +if (do_uncompress) {
> > uint8_t *compressed_data;
> > size_t max_bytes;
> > ssize_t bytes;
> > @@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> > hdr->ih_size = bytes;
> > }
> >
> > -rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> > -
> > -if (loadaddr)
> > -*loadaddr = hdr->ih_load;
> > +rom_add_blob_fixed(filename, data, hdr->ih_size, address);
> >
> > ret = hdr->ih_size;
> >
> > @@ -522,6 +538,24 @@ out:
> > return ret;
> > }
> >
> > +int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> > +int *is_linux)
> > +{
> > +return load_uboot_image(filename, ep, loadaddr, is_linux);
> > +}
> > +
> > +/* Load a ramdisk. */
> > +int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
> > +{
> > +int size = load_uboot_image(filename, NULL, &addr, NULL);
> > +
> > +if (s
Re: [Qemu-devel] [PATCH] arm/boot: Free dtb blob memory after use
Am 14.06.2013 13:27, schrieb Peter Maydell:
> The dtb blob returned by load_device_tree() is in memory allocated
> with g_malloc(). Free it accordingly once we have copied its
> contents into the guest memory. To make this easy, we need also to
> clean up the error handling in load_dtb() so that we consistently
> handle errors in the same way (by printing a message and then
> returning -1, rather than either plowing on or exiting immediately).
>
> Signed-off-by: Peter Maydell
Cc: qemu-sta...@nongnu.org ?
> ---
> hw/arm/boot.c | 20 +++-
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index f8c2031..f5870f6 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -238,14 +238,14 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
> if (!filename) {
> fprintf(stderr, "Couldn't open dtb file %s\n", binfo->dtb_filename);
> -return -1;
> +goto fail;
fdt is NULL-initialized, so this is OK.
> }
>
> fdt = load_device_tree(filename, &size);
> if (!fdt) {
> fprintf(stderr, "Couldn't open dtb file %s\n", filename);
> g_free(filename);
> -return -1;
> +goto fail;
> }
> g_free(filename);
>
Personally I would've left those two unchanged for clarity, but your
call. ;)
arm_load_kernel() does exit(1) on failure, so only functional change is
not ignoring failure to set memory, cmdline and initrd properties.
Rest looks okay, except that I wonder whether we might later want to
propagate these errors up the call chain and therefore use error_setg()
and void here and a more central fprintf() in arm_load_kernel() - we
surely don't want to duplicate it into each board even though there
being four other fprintf()s in arm_load_kernel().
But that's orthogonal to making fdt errors fatal and the fdt cleanup, so
Reviewed-by: Andreas Färber
Andreas
> @@ -253,7 +253,7 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
> if (acells == 0 || scells == 0) {
> fprintf(stderr, "dtb file invalid (#address-cells or #size-cells
> 0)\n");
> -return -1;
> +goto fail;
> }
>
> mem_reg_propsize = acells + scells;
> @@ -265,7 +265,7 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> } else if (hival != 0) {
> fprintf(stderr, "qemu: dtb file not compatible with "
> "RAM start address > 4GB\n");
> -exit(1);
> +goto fail;
> }
> mem_reg_property[acells + scells - 1] = cpu_to_be32(binfo->ram_size);
> hival = cpu_to_be32(binfo->ram_size >> 32);
> @@ -274,13 +274,14 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> } else if (hival != 0) {
> fprintf(stderr, "qemu: dtb file not compatible with "
> "RAM size > 4GB\n");
> -exit(1);
> +goto fail;
> }
>
> rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
>mem_reg_propsize * sizeof(uint32_t));
> if (rc < 0) {
> fprintf(stderr, "couldn't set /memory/reg\n");
> +goto fail;
> }
>
> if (binfo->kernel_cmdline && *binfo->kernel_cmdline) {
> @@ -288,6 +289,7 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
>binfo->kernel_cmdline);
> if (rc < 0) {
> fprintf(stderr, "couldn't set /chosen/bootargs\n");
> +goto fail;
> }
> }
>
> @@ -296,20 +298,28 @@ static int load_dtb(hwaddr addr, const struct
> arm_boot_info *binfo)
> binfo->initrd_start);
> if (rc < 0) {
> fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n");
> +goto fail;
> }
>
> rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end",
> binfo->initrd_start + binfo->initrd_size);
> if (rc < 0) {
> fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n");
> +goto fail;
> }
> }
> qemu_devtree_dumpdtb(fdt, size);
>
> cpu_physical_memory_write(addr, fdt, size);
>
> +g_free(fdt);
> +
> return 0;
>
> +fail:
> +g_free(fdt);
> +return -1;
> +
> #else
> fprintf(stderr, "Device tree requested, "
> "but qemu was compiled without fdt support\n");
>
--
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] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header
On 14 June 2013 17:36, Soren Brinkmann wrote:
> Introduce 'load_ramdisk()' which can load "normal" ramdisks and ramdisks
> with a u-boot header.
> To enable this and leverage synergies 'load_uimage()' is refactored to
> accomodate this additional use case.
Hi; thanks for this patch; some review comments below.
>
> Signed-off-by: Soren Brinkmann
> ---
> hw/core/loader.c| 86
> +
> include/hw/loader.h | 1 +
> 2 files changed, 61 insertions(+), 26 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 7507914..e72d682 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t
> *src,
> }
>
> /* Load a U-Boot image. */
> -int load_uimage(const char *filename, hwaddr *ep,
> -hwaddr *loadaddr, int *is_linux)
> +static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr
> *loadaddr,
> +int *is_linux)
> {
> int fd;
> int size;
> +hwaddr address;
> uboot_image_header_t h;
> uboot_image_header_t *hdr = &h;
> uint8_t *data = NULL;
> int ret = -1;
> +int do_uncompress = 0;
>
> fd = open(filename, O_RDONLY | O_BINARY);
> if (fd < 0)
> @@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
> goto out;
>
> /* TODO: Implement other image types. */
Doesn't this patch fix this TODO item?
> -if (hdr->ih_type != IH_TYPE_KERNEL) {
> -fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
> -goto out;
> -}
> +switch (hdr->ih_type) {
> +case IH_TYPE_KERNEL:
> +address = hdr->ih_load;
> +if (loadaddr) {
> +*loadaddr = hdr->ih_load;
> +}
> +
> +switch (hdr->ih_comp) {
> +case IH_COMP_NONE:
> +break;
> +case IH_COMP_GZIP:
> +do_uncompress = 1;
> +break;
> +default:
> +fprintf(stderr,
> +"Unable to load u-boot images with compression type
> %d\n",
> +hdr->ih_comp);
> +goto out;
> +}
> +
> +if (ep) {
> +*ep = hdr->ih_ep;
> +}
>
> -switch (hdr->ih_comp) {
> -case IH_COMP_NONE:
> -case IH_COMP_GZIP:
> +/* TODO: Check CPU type. */
> +if (is_linux) {
> +if (hdr->ih_os == IH_OS_LINUX) {
> +*is_linux = 1;
> +} else {
> +*is_linux = 0;
> +}
> +}
> +
> +break;
> +case IH_TYPE_RAMDISK:
> +address = *loadaddr;
> break;
> default:
> -fprintf(stderr,
> -"Unable to load u-boot images with compression type %d\n",
> -hdr->ih_comp);
> +fprintf(stderr, "Unsupported u-boot image type\n");
You could include the type byte here, the way we do for
unknown compression types.
> goto out;
> }
>
> -/* TODO: Check CPU type. */
> -if (is_linux) {
> -if (hdr->ih_os == IH_OS_LINUX)
> -*is_linux = 1;
> -else
> -*is_linux = 0;
> -}
> -
> -*ep = hdr->ih_ep;
> data = g_malloc(hdr->ih_size);
>
> if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> @@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> goto out;
> }
>
> -if (hdr->ih_comp == IH_COMP_GZIP) {
> +if (do_uncompress) {
> uint8_t *compressed_data;
> size_t max_bytes;
> ssize_t bytes;
> @@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> hdr->ih_size = bytes;
> }
>
> -rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> -
> -if (loadaddr)
> -*loadaddr = hdr->ih_load;
> +rom_add_blob_fixed(filename, data, hdr->ih_size, address);
>
> ret = hdr->ih_size;
>
> @@ -522,6 +538,24 @@ out:
> return ret;
> }
>
> +int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> +int *is_linux)
> +{
> +return load_uboot_image(filename, ep, loadaddr, is_linux);
> +}
> +
> +/* Load a ramdisk. */
> +int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
> +{
> +int size = load_uboot_image(filename, NULL, &addr, NULL);
> +
> +if (size < 0) {
> +size = load_image_targphys(filename, addr, max_sz);
> +}
So I can sort of see why we end up this way, but it's a bit
asymmetric that we handle 'uimage or raw' ramdisk at this
level, but require the caller to do it for the kernel.
> +
> +return size;
> +}
If we're providing separate functions for kernel and
ramdisk loading shouldn't they check that the uimage
has the appropriate type?
> +
> /*
> * Functions for reboot-persistent memory regions.
> * - used for vga bios and option roms.
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 0958f06..8ef403e 1
Re: [Qemu-devel] [RFC] sanitize memory on system reset
Only on a real 286. At least since 486 the legacy switch has been INIT, not RESET. Alexander Graf wrote: > > >Am 14.06.2013 um 08:56 schrieb Christian Borntraeger >: > >> On 13/06/13 13:56, Anthony Liguori wrote: >>> Markus Armbruster writes: >>> Peter Lieven writes: > On 13.06.2013 10:40, Stefan Hajnoczi wrote: >> On Thu, Jun 13, 2013 at 08:09:09AM +0200, Peter Lieven wrote: >>> I was thinking if it would be a good idea to zeroize all memory >>> resources on system reset and >>> madvise dontneed them afterwards. This would avoid system reset >>> attacks in case the attacker >>> has only access to the console of a vServer but not on the >physical >>> host and it would shrink >>> RSS size of the vServer siginificantly. >> I wonder if you'll hit weird OS installers or PXE clients that >rely on >> stashing stuff in memory across reset. > One point: > Wouldn't a memory test which some systems do at startup break >these as well? Systems that distinguish between warm and cold boot (such as PCs) generally run POST only on cold boot. I'm not saying triggering warm reboot and expecting memory contents >to survive is a good idea, but it has been done. >>> >>> Doesn't kexec do a warm reboot stashing the new kernel somewhere in >>> memory? >> >> It does something like that on s390. >> There is a diagnose instruction to the machine, that resets all >> subsystems and cpus in a defined state, but lets the memory >untouched. >> So we want to be able to define the components of the system which we >are >> going to reset and have two cases: >> 1. reset everything and clear the memory >> 2. just reset the cpus and devices, but leave the memory untouched >> >> For case 2 we basically want to avoid memory clearing AND bios >reloading > >Legacy 286 protected mode to real mode switching also happens through >the CPU reset PIN, so there certainly is a need to distinguish. > >Alex > >> >> >> -- Sent from my mobile phone. Please excuse brevity and lack of formatting.
[Qemu-devel] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header
Introduce 'load_ramdisk()' which can load "normal" ramdisks and ramdisks
with a u-boot header.
To enable this and leverage synergies 'load_uimage()' is refactored to
accomodate this additional use case.
Signed-off-by: Soren Brinkmann
---
hw/core/loader.c| 86 +
include/hw/loader.h | 1 +
2 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7507914..e72d682 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t
*src,
}
/* Load a U-Boot image. */
-int load_uimage(const char *filename, hwaddr *ep,
-hwaddr *loadaddr, int *is_linux)
+static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
+int *is_linux)
{
int fd;
int size;
+hwaddr address;
uboot_image_header_t h;
uboot_image_header_t *hdr = &h;
uint8_t *data = NULL;
int ret = -1;
+int do_uncompress = 0;
fd = open(filename, O_RDONLY | O_BINARY);
if (fd < 0)
@@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
goto out;
/* TODO: Implement other image types. */
-if (hdr->ih_type != IH_TYPE_KERNEL) {
-fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
-goto out;
-}
+switch (hdr->ih_type) {
+case IH_TYPE_KERNEL:
+address = hdr->ih_load;
+if (loadaddr) {
+*loadaddr = hdr->ih_load;
+}
+
+switch (hdr->ih_comp) {
+case IH_COMP_NONE:
+break;
+case IH_COMP_GZIP:
+do_uncompress = 1;
+break;
+default:
+fprintf(stderr,
+"Unable to load u-boot images with compression type %d\n",
+hdr->ih_comp);
+goto out;
+}
+
+if (ep) {
+*ep = hdr->ih_ep;
+}
-switch (hdr->ih_comp) {
-case IH_COMP_NONE:
-case IH_COMP_GZIP:
+/* TODO: Check CPU type. */
+if (is_linux) {
+if (hdr->ih_os == IH_OS_LINUX) {
+*is_linux = 1;
+} else {
+*is_linux = 0;
+}
+}
+
+break;
+case IH_TYPE_RAMDISK:
+address = *loadaddr;
break;
default:
-fprintf(stderr,
-"Unable to load u-boot images with compression type %d\n",
-hdr->ih_comp);
+fprintf(stderr, "Unsupported u-boot image type\n");
goto out;
}
-/* TODO: Check CPU type. */
-if (is_linux) {
-if (hdr->ih_os == IH_OS_LINUX)
-*is_linux = 1;
-else
-*is_linux = 0;
-}
-
-*ep = hdr->ih_ep;
data = g_malloc(hdr->ih_size);
if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
@@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
goto out;
}
-if (hdr->ih_comp == IH_COMP_GZIP) {
+if (do_uncompress) {
uint8_t *compressed_data;
size_t max_bytes;
ssize_t bytes;
@@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
hdr->ih_size = bytes;
}
-rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
-
-if (loadaddr)
-*loadaddr = hdr->ih_load;
+rom_add_blob_fixed(filename, data, hdr->ih_size, address);
ret = hdr->ih_size;
@@ -522,6 +538,24 @@ out:
return ret;
}
+int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
+int *is_linux)
+{
+return load_uboot_image(filename, ep, loadaddr, is_linux);
+}
+
+/* Load a ramdisk. */
+int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
+{
+int size = load_uboot_image(filename, NULL, &addr, NULL);
+
+if (size < 0) {
+size = load_image_targphys(filename, addr, max_sz);
+}
+
+return size;
+}
+
/*
* Functions for reboot-persistent memory regions.
* - used for vga bios and option roms.
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 0958f06..8ef403e 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -15,6 +15,7 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
int bswap_needed, hwaddr target_page_size);
int load_uimage(const char *filename, hwaddr *ep,
hwaddr *loadaddr, int *is_linux);
+int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);
ssize_t read_targphys(const char *name,
int fd, hwaddr dst_addr, size_t nbytes);
--
1.8.3.1
[Qemu-devel] [PATCH 2/2] hw/arm: Use 'load_ramdisk()' for loading ramdisk
The load_ramdisk function takes over loading traditional ramdisks images
and does also load ramdisks with u-boot header.
Signed-off-by: Soren Brinkmann
---
hw/arm/boot.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index f310f73..ab2b234 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -436,10 +436,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
info->entry = entry;
if (is_linux) {
if (info->initrd_filename) {
-initrd_size = load_image_targphys(info->initrd_filename,
- info->initrd_start,
- info->ram_size -
- info->initrd_start);
+initrd_size = load_ramdisk(info->initrd_filename,
+ info->initrd_start,
+ info->ram_size -
+ info->initrd_start);
if (initrd_size < 0) {
fprintf(stderr, "qemu: could not load initrd '%s'\n",
info->initrd_filename);
--
1.8.3.1
[Qemu-devel] [PATCH 0/2] Support ramdisks with U-Boot header
For loading Linux kernels, QEMU already is able to recognize and load them when they feature a U-Boot header. This patch series targets to extend this support to ramdisks. Furthermore the ARM support code is changed to use the new functionality. Regards, Sören Soren Brinkmann (2): hw/loader: Support ramdisk with u-boot header hw/arm: Use 'load_ramdisk()' for loading ramdisk hw/arm/boot.c | 8 ++--- hw/core/loader.c| 86 + include/hw/loader.h | 1 + 3 files changed, 65 insertions(+), 30 deletions(-) -- 1.8.3.1
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
On 06/14/2013 11:56 AM, Anthony Liguori wrote: Corey Bryant writes: On 06/14/2013 11:38 AM, Anthony Liguori wrote: Corey Bryant writes: On 06/14/2013 10:01 AM, Anthony Liguori wrote: Corey Bryant writes: This patch series provides persistent storage support that a TPM can use to store NVRAM data. It uses QEMU's block driver to store data on a drive image. The libtpms TPM 1.2 backend will be the initial user of this functionality to store data that must persist through a reboot or migration. A sample command line may look like this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori Fair enough. I assume you're ok with this code though? I don't understand why it's needed to be honest. I suspect this has to do with the fact that the libtpms implementation will need significant reworking. Regards, Anthony Liguori In regards to why it is needed.. The QEMU software-emulated vTPM backend will pass callback functions to libtpms for writing/reading nvram data. Those callbacks will use the code in this patch series to do the writing/reading of nvram data to/from image files so that the data persists through migration/reboot. I'm not sure I completely understand your second sentence, but yes the software-emulated vTPM backend code for QEMU will certainly need rework to use the code in this patch series. I think it's easiest to discuss this in the context of the actual patch series. Regards, Anthony Liguori I suppose, but the earlier we can get feedback the better so that we don't waste any more time. This NVRAM code alone has gone through far too many iterations as folks have asked for it to go in different directions, and we went in those directions to find that they were the wrong directions. Anyway, for the record, this latest patch series adheres to the direction you suggested we take last month: http://lists.nongnu.org/archive/html/qemu-devel/2013-05/msg04275.html -- Regards, Corey Bryant -- Regards, Corey Bryant -- Regards, Corey Bryant qemu-system-x86_64 ... -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 -tpmdev libtpms,id=tpm-tpm0 -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 Thanks, Corey Corey Bryant (3): nvram: Add TPM NVRAM implementation nvram: Add tpm-tis drive support TPM NVRAM test hw/tpm/Makefile.objs |1 + hw/tpm/tpm_int.h |2 + hw/tpm/tpm_nvram.c | 324 ++ hw/tpm/tpm_nvram.h | 25 hw/tpm/tpm_passthrough.c | 85 hw/tpm/tpm_tis.c |8 + 6 files changed, 445 insertions(+), 0 deletions(-) create mode 100644 hw/tpm/tpm_nvram.c create mode 100644 hw/tpm/tpm_nvram.h
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Change default machine for 64-bit
Am 13.06.2013 21:02, schrieb Anthony Liguori: > Scott Wood writes: > >> On 06/13/2013 01:31:48 PM, Anthony Liguori wrote: >>> I'll note that we have a CONFIG_PSERIES today. I'd suggest getting >>> rid >>> of it first before making pseries the default. >> >> Why? > > I believe it's there only because libfdt used to be optional. Isn't that define rather related to pseries being ppc64-only? 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] [PATCH v3 0/3] TPM NVRAM persistent storage
Corey Bryant writes: > On 06/14/2013 11:38 AM, Anthony Liguori wrote: >> Corey Bryant writes: >> >>> On 06/14/2013 10:01 AM, Anthony Liguori wrote: Corey Bryant writes: > This patch series provides persistent storage support that a TPM > can use to store NVRAM data. It uses QEMU's block driver to store > data on a drive image. The libtpms TPM 1.2 backend will be the > initial user of this functionality to store data that must persist > through a reboot or migration. A sample command line may look like > this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori >>> >>> Fair enough. I assume you're ok with this code though? >> >> I don't understand why it's needed to be honest. I suspect this has to >> do with the fact that the libtpms implementation will need significant >> reworking. >> >> Regards, >> >> Anthony Liguori >> > > In regards to why it is needed.. The QEMU software-emulated vTPM > backend will pass callback functions to libtpms for writing/reading > nvram data. Those callbacks will use the code in this patch series to > do the writing/reading of nvram data to/from image files so that the > data persists through migration/reboot. > > I'm not sure I completely understand your second sentence, but yes the > software-emulated vTPM backend code for QEMU will certainly need rework > to use the code in this patch series. I think it's easiest to discuss this in the context of the actual patch series. Regards, Anthony Liguori > > -- > Regards, > Corey Bryant > >>> >>> -- >>> Regards, >>> Corey Bryant >>> > > qemu-system-x86_64 ... > -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 > -tpmdev libtpms,id=tpm-tpm0 > -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 > > Thanks, > Corey > > Corey Bryant (3): > nvram: Add TPM NVRAM implementation > nvram: Add tpm-tis drive support > TPM NVRAM test > >hw/tpm/Makefile.objs |1 + >hw/tpm/tpm_int.h |2 + >hw/tpm/tpm_nvram.c | 324 > ++ >hw/tpm/tpm_nvram.h | 25 >hw/tpm/tpm_passthrough.c | 85 >hw/tpm/tpm_tis.c |8 + >6 files changed, 445 insertions(+), 0 deletions(-) >create mode 100644 hw/tpm/tpm_nvram.c >create mode 100644 hw/tpm/tpm_nvram.h >> >> >>
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
On 06/14/2013 11:38 AM, Anthony Liguori wrote: Corey Bryant writes: On 06/14/2013 10:01 AM, Anthony Liguori wrote: Corey Bryant writes: This patch series provides persistent storage support that a TPM can use to store NVRAM data. It uses QEMU's block driver to store data on a drive image. The libtpms TPM 1.2 backend will be the initial user of this functionality to store data that must persist through a reboot or migration. A sample command line may look like this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori Fair enough. I assume you're ok with this code though? I don't understand why it's needed to be honest. I suspect this has to do with the fact that the libtpms implementation will need significant reworking. Regards, Anthony Liguori In regards to why it is needed.. The QEMU software-emulated vTPM backend will pass callback functions to libtpms for writing/reading nvram data. Those callbacks will use the code in this patch series to do the writing/reading of nvram data to/from image files so that the data persists through migration/reboot. I'm not sure I completely understand your second sentence, but yes the software-emulated vTPM backend code for QEMU will certainly need rework to use the code in this patch series. -- Regards, Corey Bryant -- Regards, Corey Bryant qemu-system-x86_64 ... -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 -tpmdev libtpms,id=tpm-tpm0 -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 Thanks, Corey Corey Bryant (3): nvram: Add TPM NVRAM implementation nvram: Add tpm-tis drive support TPM NVRAM test hw/tpm/Makefile.objs |1 + hw/tpm/tpm_int.h |2 + hw/tpm/tpm_nvram.c | 324 ++ hw/tpm/tpm_nvram.h | 25 hw/tpm/tpm_passthrough.c | 85 hw/tpm/tpm_tis.c |8 + 6 files changed, 445 insertions(+), 0 deletions(-) create mode 100644 hw/tpm/tpm_nvram.c create mode 100644 hw/tpm/tpm_nvram.h
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
On 06/14/2013 11:38 AM, Anthony Liguori wrote: Corey Bryant writes: On 06/14/2013 10:01 AM, Anthony Liguori wrote: Corey Bryant writes: This patch series provides persistent storage support that a TPM can use to store NVRAM data. It uses QEMU's block driver to store data on a drive image. The libtpms TPM 1.2 backend will be the initial user of this functionality to store data that must persist through a reboot or migration. A sample command line may look like this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori Fair enough. I assume you're ok with this code though? I don't understand why it's needed to be honest. I suspect this has to do with the fact that the libtpms implementation will need significant reworking. libtpms does not implement a file storage layer. It neither writes data into a FILE * nor into a QEMU BDRV. Instead it provides callbacks for users to implement the file storage layer. Is there a problem with that? Regards, Stefan Regards, Anthony Liguori -- Regards, Corey Bryant qemu-system-x86_64 ... -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 -tpmdev libtpms,id=tpm-tpm0 -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 Thanks, Corey Corey Bryant (3): nvram: Add TPM NVRAM implementation nvram: Add tpm-tis drive support TPM NVRAM test hw/tpm/Makefile.objs |1 + hw/tpm/tpm_int.h |2 + hw/tpm/tpm_nvram.c | 324 ++ hw/tpm/tpm_nvram.h | 25 hw/tpm/tpm_passthrough.c | 85 hw/tpm/tpm_tis.c |8 + 6 files changed, 445 insertions(+), 0 deletions(-) create mode 100644 hw/tpm/tpm_nvram.c create mode 100644 hw/tpm/tpm_nvram.h
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
Corey Bryant writes: > On 06/14/2013 10:01 AM, Anthony Liguori wrote: >> Corey Bryant writes: >> >>> This patch series provides persistent storage support that a TPM >>> can use to store NVRAM data. It uses QEMU's block driver to store >>> data on a drive image. The libtpms TPM 1.2 backend will be the >>> initial user of this functionality to store data that must persist >>> through a reboot or migration. A sample command line may look like >>> this: >> >> This should be folded into the libtpms backend series. >> >> There are no users for this so this would just be untestable code in the >> tree subject to bitrot. >> >> Regards, >> >> Anthony Liguori >> > > Fair enough. I assume you're ok with this code though? I don't understand why it's needed to be honest. I suspect this has to do with the fact that the libtpms implementation will need significant reworking. Regards, Anthony Liguori > > -- > Regards, > Corey Bryant > >>> >>> qemu-system-x86_64 ... >>> -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 >>> -tpmdev libtpms,id=tpm-tpm0 >>> -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 >>> >>> Thanks, >>> Corey >>> >>> Corey Bryant (3): >>>nvram: Add TPM NVRAM implementation >>>nvram: Add tpm-tis drive support >>>TPM NVRAM test >>> >>> hw/tpm/Makefile.objs |1 + >>> hw/tpm/tpm_int.h |2 + >>> hw/tpm/tpm_nvram.c | 324 >>> ++ >>> hw/tpm/tpm_nvram.h | 25 >>> hw/tpm/tpm_passthrough.c | 85 >>> hw/tpm/tpm_tis.c |8 + >>> 6 files changed, 445 insertions(+), 0 deletions(-) >>> create mode 100644 hw/tpm/tpm_nvram.c >>> create mode 100644 hw/tpm/tpm_nvram.h >> >> >> >>
[Qemu-devel] [Bug 829455] Re: user mode network stack - hostfwd not working with restrict=y
Did you guys merge back the fix in: http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg01519.html ? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/829455 Title: user mode network stack - hostfwd not working with restrict=y Status in QEMU: New Bug description: I find that explicit hostfwd commands do not seem to work with restrict=yes option, even if the docs clearly state that hostfwd should override restrict setting. I am using this config: -net user,name=test,net=192.168.100.0/24,host=192.168.100.44,restrict=y,hostfwd=tcp:127.0.0.1:3389-192.168.100.1:3389 (my guest has static IP address configured as 192.168.100.1/24) and I cannot log into my guest via rdp. the client hanging indefinitely. by just changing to "restrict=no" I can log in. maybe I am doing something wrong, but I cannot figure out what. running QEMU emulator version 0.14.0 (qemu-kvm-0.14.0) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/829455/+subscriptions
[Qemu-devel] [PULL 06/10] configure: dtc: Probe for libfdt_env.h
From: Peter Crosthwaite
Currently QEMU provides a local clone of the file libfdt_env.h in
/include. This file is supposed to come with the libfdt package and is
only needed for broken installs of libfdt. Now that we have submodule
dtc, just ignore these broken installs and prompt for the dtc submodule
install instead. QEMU's local libfdt_env.h is removed accordingly.
Manifests as a bug when building QEMU with modern libfdt. The new
version of libfdt does not compile when QEMUs libfdt_env.h takes
precedence over the hosts.
Signed-off-by: Peter Crosthwaite
Reviewed-by: Peter Maydell
Acked-by: David Gibson
Signed-off-by: Kim Phillips
Acked-by: Paolo Bonzini
Message-id:
9b6a3a52e3f46cfbc1ded9ab56385ec045e46705.1369628289.git.peter.crosthwa...@xilinx.com
Signed-off-by: Peter Maydell
---
configure|2 ++
include/libfdt_env.h | 36
2 files changed, 2 insertions(+), 36 deletions(-)
delete mode 100644 include/libfdt_env.h
diff --git a/configure b/configure
index 8732185..31b7783 100755
--- a/configure
+++ b/configure
@@ -2488,7 +2488,9 @@ fi
# fdt probe
if test "$fdt" != "no" ; then
fdt_libs="-lfdt"
+ # explicitly check for libfdt_env.h as it is missing in some stable installs
cat > $TMPC << EOF
+#include
int main(void) { return 0; }
EOF
if compile_prog "" "$fdt_libs" ; then
diff --git a/include/libfdt_env.h b/include/libfdt_env.h
deleted file mode 100644
index 3667d4c..000
--- a/include/libfdt_env.h
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License, version 2, as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, see
[Qemu-devel] [PULL 04/10] main: use TARGET_ARCH only for the target-specific #define
From: Paolo Bonzini
Everything else needs to match the executable name, which is
TARGET_NAME.
Before:
$ sh4eb-linux-user/qemu-sh4eb --help
usage: qemu-sh4 [options] program [arguments...]
Linux CPU emulator (compiled for sh4 emulation)
After:
$ sh4eb-linux-user/qemu-sh4eb --help
usage: qemu-sh4eb [options] program [arguments...]
Linux CPU emulator (compiled for sh4eb emulation)
Signed-off-by: Paolo Bonzini
Message-id: 1370349928-20419-5-git-send-email-pbonz...@redhat.com
Signed-off-by: Peter Maydell
---
arch_init.c |2 +-
bsd-user/main.c |6 +++---
configure |1 -
linux-user/main.c |6 +++---
scripts/create_config | 13 -
5 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..23ca953 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -123,7 +123,7 @@ static struct defconfig_file {
bool userconfig;
} default_config_files[] = {
{ CONFIG_QEMU_CONFDIR "/qemu.conf", true },
-{ CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf", true },
+{ CONFIG_QEMU_CONFDIR "/target-" TARGET_NAME ".conf", true },
{ NULL }, /* end of list */
};
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0da3ab9..572f13a 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -670,8 +670,8 @@ void cpu_loop(CPUSPARCState *env)
static void usage(void)
{
-printf("qemu-" TARGET_ARCH " version " QEMU_VERSION ", Copyright (c)
2003-2008 Fabrice Bellard\n"
- "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
+printf("qemu-" TARGET_NAME " version " QEMU_VERSION ", Copyright (c)
2003-2008 Fabrice Bellard\n"
+ "usage: qemu-" TARGET_NAME " [options] program [arguments...]\n"
"BSD CPU emulator (compiled for %s emulation)\n"
"\n"
"Standard options:\n"
@@ -706,7 +706,7 @@ static void usage(void)
"Note that if you provide several changes to single variable\n"
"last change will stay in effect.\n"
,
- TARGET_ARCH,
+ TARGET_NAME,
interp_prefix,
x86_stack_size);
exit(1);
diff --git a/configure b/configure
index 1dfa712..46a2173 100755
--- a/configure
+++ b/configure
@@ -4243,7 +4243,6 @@ upper() {
echo "$@"| LC_ALL=C tr '[a-z]' '[A-Z]'
}
-echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak
target_arch_name="`upper $TARGET_ARCH`"
echo "TARGET_$target_arch_name=y" >> $config_target_mak
echo "TARGET_NAME=$target_name" >> $config_target_mak
diff --git a/linux-user/main.c b/linux-user/main.c
index b97b8cf..21725a4 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3339,7 +3339,7 @@ static void handle_arg_strace(const char *arg)
static void handle_arg_version(const char *arg)
{
-printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION
+printf("qemu-" TARGET_NAME " version " QEMU_VERSION QEMU_PKGVERSION
", Copyright (c) 2003-2008 Fabrice Bellard\n");
exit(0);
}
@@ -3400,8 +3400,8 @@ static void usage(void)
int maxarglen;
int maxenvlen;
-printf("usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
- "Linux CPU emulator (compiled for " TARGET_ARCH " emulation)\n"
+printf("usage: qemu-" TARGET_NAME " [options] program [arguments...]\n"
+ "Linux CPU emulator (compiled for " TARGET_NAME " emulation)\n"
"\n"
"Options and associated environment variables:\n"
"\n");
diff --git a/scripts/create_config b/scripts/create_config
index 6461ef6..b1adbf5 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -77,16 +77,10 @@ case $line in
value=${line#*=}
echo "#define $name $value"
;;
- TARGET_ARCH=*) # configuration
-target_arch=${line#*=}
-echo "#define TARGET_ARCH \"$target_arch\""
-;;
TARGET_BASE_ARCH=*) # configuration
target_base_arch=${line#*=}
-if [ "$target_base_arch" != "$target_arch" ]; then
- base_arch_name=`echo $target_base_arch | LC_ALL=C tr '[a-z]' '[A-Z]'`
- echo "#define TARGET_$base_arch_name 1"
-fi
+base_arch_name=`echo $target_base_arch | LC_ALL=C tr '[a-z]' '[A-Z]'`
+echo "#define TARGET_$base_arch_name 1"
;;
TARGET_XML_FILES=*)
# do nothing
@@ -95,7 +89,8 @@ case $line in
# do nothing
;;
TARGET_NAME=*)
-# do nothing
+target_name=${line#*=}
+echo "#define TARGET_NAME \"$target_name\""
;;
TARGET_DIRS=*)
# do nothing
--
1.7.9.5
Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-platform device initialization
> -Original Message- > From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > Bonzini > Sent: 14 June 2013 15:58 > To: Paul Durrant > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen- > de...@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device > initialization > > Il 14/06/2013 10:11, Paul Durrant ha scritto: > > I think we're still going to need -M xenpv, I think; it's quite > > distinct from pc. > > Of course! Even more: "-M xenpv" should be reused on ARM. > > > I guess we could use -M pc for HVM and gate the > > accel code as you suggest but, if that's the way we're going, it > > would seem more logical just to ditch the accel code for xenpv > > completely (assuming we can do all we need from the machine init) and > > then use -M pc -accel=xen for HVM guests going forward. > > There is common code between pv and fv, and that one definitely belongs > in xen_init. Most fv-only code probably should be in pc_init. The rest > should move to xen_init though, because it would apply just as well for > example to Q35. It's a bit ugly to have fv-only code there, but it's > better than having a Xen-specific machine type. Xen/KVM/TCG should be > as similar as possible at the QEMU level, any difference should be > handled in the toolstack. > > > But that does > > rather screw up my autodiscovery plans because I would not know, for > > a given qemu binary, which machine type to use. > > There's no need for that. 4.4 can just use "-M pc" unconditionally, > <=4.3 will just use "-M xenfv" unconditionally. > > > If I create a new > > xenfv-2.0 machine type though I *can* do auto discovery... in which > > case do we need the -accel=xen option at all? > > Yes. Please try not do things differently from other accelerators. > Ok. I guess we can have the ability to override the machine type in the VM config, so you could still kick off an older qemu with a newer libxl - but it sounds like the auto-discovery idea is a no-go then. Paul
Re: [Qemu-devel] [PATCH v3 0/3] TPM NVRAM persistent storage
On 06/14/2013 10:01 AM, Anthony Liguori wrote: Corey Bryant writes: This patch series provides persistent storage support that a TPM can use to store NVRAM data. It uses QEMU's block driver to store data on a drive image. The libtpms TPM 1.2 backend will be the initial user of this functionality to store data that must persist through a reboot or migration. A sample command line may look like this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori Fair enough. I assume you're ok with this code though? -- Regards, Corey Bryant qemu-system-x86_64 ... -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 -tpmdev libtpms,id=tpm-tpm0 -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 Thanks, Corey Corey Bryant (3): nvram: Add TPM NVRAM implementation nvram: Add tpm-tis drive support TPM NVRAM test hw/tpm/Makefile.objs |1 + hw/tpm/tpm_int.h |2 + hw/tpm/tpm_nvram.c | 324 ++ hw/tpm/tpm_nvram.h | 25 hw/tpm/tpm_passthrough.c | 85 hw/tpm/tpm_tis.c |8 + 6 files changed, 445 insertions(+), 0 deletions(-) create mode 100644 hw/tpm/tpm_nvram.c create mode 100644 hw/tpm/tpm_nvram.h
Re: [Qemu-devel] [PATCH v2 03/17] memory: add ref/unref calls
On 06/14/2013 11:56 PM, Paolo Bonzini wrote: > Il 14/06/2013 06:09, Alexey Kardashevskiy ha scritto: >> And - crash: >> >> >> #0 object_unref (obj=0xa7a7a7a7a7a7a7a7) at >> /home/alexey/pcipassthru/qemu-impreza/qom/object.c:691 > > Dangling pointer. One ref, two unrefs probably. No, subpages (and nested MRs) are just g_free'd, it is not a result of unreferencing, that's the point. > I'm redoing the series according to Peter's request, it could fix it > automatically. When is it expected to be pushed to github? -- Alexey
[Qemu-devel] [PULL 05/10] build: drop TARGET_TYPE
From: Paolo Bonzini
Just use the TARGET_NAME free string.
Signed-off-by: Paolo Bonzini
Reviewed-by: Eric Blake
Message-id: 1370349928-20419-6-git-send-email-pbonz...@redhat.com
Signed-off-by: Peter Maydell
---
arch_init.c |2 +-
configure|1 -
qapi-schema.json | 18 +-
3 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 23ca953..699c927 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1094,7 +1094,7 @@ TargetInfo *qmp_query_target(Error **errp)
{
TargetInfo *info = g_malloc0(sizeof(*info));
-info->arch = TARGET_TYPE;
+info->arch = g_strdup(TARGET_NAME);
return info;
}
diff --git a/configure b/configure
index 46a2173..8732185 100755
--- a/configure
+++ b/configure
@@ -4246,7 +4246,6 @@ upper() {
target_arch_name="`upper $TARGET_ARCH`"
echo "TARGET_$target_arch_name=y" >> $config_target_mak
echo "TARGET_NAME=$target_name" >> $config_target_mak
-echo "TARGET_TYPE=TARGET_TYPE_`upper $target_name`" >> $config_target_mak
echo "TARGET_BASE_ARCH=$TARGET_BASE_ARCH" >> $config_target_mak
if [ "$TARGET_ABI_DIR" = "" ]; then
TARGET_ABI_DIR=$TARGET_ARCH
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..a80ee40 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3013,22 +3013,6 @@
{ 'command': 'query-fdsets', 'returns': ['FdsetInfo'] }
##
-# @TargetType
-#
-# Target CPU emulation type
-#
-# These parameters correspond to the softmmu binary CPU name that is currently
-# running.
-#
-# Since: 1.2.0
-##
-{ 'enum': 'TargetType',
- 'data': [ 'alpha', 'arm', 'cris', 'i386', 'lm32', 'm68k', 'microblazeel',
-'microblaze', 'mips64el', 'mips64', 'mipsel', 'mips', 'moxie',
-'or32', 'ppc64', 'ppcemb', 'ppc', 's390x', 'sh4eb', 'sh4',
-'sparc64', 'sparc', 'unicore32', 'x86_64', 'xtensaeb', 'xtensa' ] }
-
-##
# @TargetInfo:
#
# Information describing the QEMU target.
@@ -3038,7 +3022,7 @@
# Since: 1.2.0
##
{ 'type': 'TargetInfo',
- 'data': { 'arch': 'TargetType' } }
+ 'data': { 'arch': 'str' } }
##
# @query-target:
--
1.7.9.5
Re: [Qemu-devel] [PULL 00/10] configury queue
Il 14/06/2013 10:53, Peter Maydell ha scritto: > [NB: I don't know if we get enough build-machinery patches to make > it worth having a permanent subtree/submaintainer for them; it's > merely that at the moment there are enough patches I personally > care about that I felt like curating them ;-)] I think that's exactly the right thing to do. Thanks for sending the pull request. Paolo
Re: [Qemu-devel] [PATCH 3/5] qcow2: Options to enable discard for freed clusters
Il 14/06/2013 10:31, Kevin Wolf ha scritto: >> It looks like QCOW2_OPT_DISCARD_OTHER is a rare case, so I don't mind >> leaving it as default to false. It won't waste more than a few clusters. > > Yes, it's generally relatively rare, like growing L1 or refcount table. > There is one case where it should trigger a lot, though: Overwriting > clusters of a compressed image. > > Hm, though actually it doesn't make a lot of sense there. The freed > cluster will immediately be used by the next write. Maybe COW should > actually be QCOW2_OPT_DISCARD_NEVER... Sounds reasonable. >> In the end discard_snapshot and discard_other should rarely be needed in >> practice, so I don't think having discard=... is a mistake. Too many >> knobs won't really be needed. >> >> In fact, perhaps we do not need discard_snapshot and discard_request, >> only discard_other. discard_snapshot can be replaced by >> file.discard=ignore, discard_request by discard=unmap. > > This is only true if you rule out some combination as useless. For > example you would say that if you want to process guest requests, you > always want to have snapshots discarded as well. You also assume that > nobody wants the current behaviour (free clusters in qcow2 metadata, but > don't send discards to raw-posix). > > Isn't this assuming a bit too much? > > To be clear, I don't expect these knobs to be used much either, but I > have some feeling that some people (including us while debugging or > asking questions) may be glad later to have such low-level options that > control each layer separately. Yeah, you're right. They're definitely useful to have, even if it is "just in case". Paolo
Re: [Qemu-devel] [PATCH v2] kvm/openpic: in-kernel mpic support
Am 13.06.2013 19:32, schrieb Scott Wood:
> On 06/13/2013 06:01:49 AM, Andreas Färber wrote:
>> Am 12.06.2013 22:32, schrieb Scott Wood:
>> > +typedef struct KVMOpenPICState {
>> > +SysBusDevice busdev;
>>
>> SysBusDevice parent_obj; please!
>>
>> http://wiki.qemu.org/QOMConventions
>
> The word "QOMConventions" doesn't exist once in the QEMU source tree.
> How is one supposed to know that this documentation exists? :-P
I do expect people to read at least the subject of patch series on
qemu-devel, short of individual review comments. CPU, PReP PCI,
Versatile PCI, ISA and more recently virtio series had been posted.
Some such review comments have been collected into the above Wiki page
because this is a recurring topic unfortunately.
> Plus, this is just copied from the non-KVM MPIC file, and I see many
> other instances throughout the source tree.
Which exactly is the reason for my grief: Your ignorance is making other
people even more work, and at least Alex should've spotted it - I can't
review all patches just because they might or might not be touching on QOM.
Just as we are supposed to not copy old Coding Style in new patches, we
should be applying new patterns and conventions in new patches, too.
>> > +static int kvm_openpic_init(SysBusDevice *dev)
>>
>> Please make this instance_init + realize functions - "dev" should rather
>> be reserved for DeviceState.
>
> Could you elaborate? I'm really not familiar with this stuff, and have
> not found much documentation. Again, this is patterned after the existing
> non-KVM openpic file.
static void kvm_openpic_init(Object *obj) should initialize simple
variables, MemoryRegions that don't depend on parameters and any QOM
properties.
static void kvm_openpic_realize(DeviceState *dev, Error **errp) should
initialize the rest.
kvm_openpic_unrealize(Device *dev, Error **errp) and
kvm_openpic_finalize(Object *obj) would be their counterparts for cleanup.
>> > +{
>> > +KVMState *s = kvm_state;
>> > +KVMOpenPICState *opp = FROM_SYSBUS(typeof(*opp), dev);
>>
>> NACK, please introduce your own KVM_OPENPIC(obj) cast macro instead for
>> new devices - has been a topic for several weeks and months now.
>
> There's way too much traffic on the list for me to know about something
> just because it's "been a topic". Lately it's been too much for me to
> even scan the subject lines looking for things that look important,
> given that QEMU is only a fraction of what I spend my time on.
>
> If you'd like to update hw/intc/openpic.c to comply with the style of
> the day, then this could be updated to match...
>
> Also note that this is hardly the first time this patch has been posted
> (v1 was a few weeks ago, and there were RFC patches well before that).
> The first version may have even preceded this "topic". This seems a bit
> late in the process for a bunch of style churn, when existing code
> hasn't been updated.
I'm not talking about style churn, I'm talking about using the wrong
infrastructure and making it even more difficult to drop FROM_SYSBUS()
macro.
Again, whether or not some particular other file uses some style doesn't
mean that it's okay to apply that to new files. Instead of complaining
it would've been a task of five minutes to supply Alex with a fixup
patch to squash/follow-up or to post a v3. QOM realize is merged since
January 2013 and was presented by Anthony in January 2012.
>> > +int kvm_openpic_model;
>> > +struct kvm_create_device cd = {0};
>> > +int ret, i;
>> > +
>> > +if (!kvm_check_extension(s, KVM_CAP_DEVICE_CTRL)) {
>> > +return -EINVAL;
>> > +}
>> > +
>> > +switch (opp->model) {
>> > +case OPENPIC_MODEL_FSL_MPIC_20:
>> > +kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_20;
>> > +break;
>> > +
>> > +case OPENPIC_MODEL_FSL_MPIC_42:
>> > +kvm_openpic_model = KVM_DEV_TYPE_FSL_MPIC_42;
>> > +break;
>> > +
>> > +default:
>> > +return -EINVAL;
>> > +}
>>
>> If there's only two supported enum-style options, why not make it two
>> devices with the value set as a class field?
>
> I'm not 100% sure what you mean here, but it is not intended that there
> will always be only two supported options. At the least we will
> probably support v4.3 at some point.
Afterwards I saw that you copied the "model" property from the non-KVM.
What I was pointing about here is that unlike qdev QOM allows to have
arbitrary levels of inheritence, i.e. you can have a kvm-openpic base
class and specialized subclasses like mipc42-kvm-openpic rather than
kvm-openpic,model=42 given that the user can't specify arbitrary
integers - whether two or three doesn't really matter in that respect.
Regards,
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] [Xen-devel] [PATCH] Remove hardcoded xen-platform device initialization
Il 14/06/2013 10:11, Paul Durrant ha scritto: > I think we're still going to need -M xenpv, I think; it's quite > distinct from pc. Of course! Even more: "-M xenpv" should be reused on ARM. > I guess we could use -M pc for HVM and gate the > accel code as you suggest but, if that's the way we're going, it > would seem more logical just to ditch the accel code for xenpv > completely (assuming we can do all we need from the machine init) and > then use -M pc -accel=xen for HVM guests going forward. There is common code between pv and fv, and that one definitely belongs in xen_init. Most fv-only code probably should be in pc_init. The rest should move to xen_init though, because it would apply just as well for example to Q35. It's a bit ugly to have fv-only code there, but it's better than having a Xen-specific machine type. Xen/KVM/TCG should be as similar as possible at the QEMU level, any difference should be handled in the toolstack. > But that does > rather screw up my autodiscovery plans because I would not know, for > a given qemu binary, which machine type to use. There's no need for that. 4.4 can just use "-M pc" unconditionally, <=4.3 will just use "-M xenfv" unconditionally. > If I create a new > xenfv-2.0 machine type though I *can* do auto discovery... in which > case do we need the -accel=xen option at all? Yes. Please try not do things differently from other accelerators. Paolo
[Qemu-devel] [PULL 03/10] build: do not use TARGET_ARCH
From: Paolo Bonzini
TARGET_ARCH is generally wrong to use, there are better variables
provided in config-target.mak. The right one is usually TARGET_NAME
(previously TARGET_ARCH2), but for bsd-user we can also use TARGET_ABI_DIR
for consistency with linux-user.
Signed-off-by: Paolo Bonzini
Message-id: 1370349928-20419-4-git-send-email-pbonz...@redhat.com
Signed-off-by: Peter Maydell
---
Makefile.target |8
docs/tracing.txt |2 +-
scripts/tracetool.py | 18 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index fcc880b..9a49852 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -48,7 +48,7 @@ $(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events
--format=stap \
--backend=$(TRACE_BACKEND) \
--binary=$(bindir)/$(QEMU_PROG) \
- --target-arch=$(TARGET_ARCH) \
+ --target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
< $< > $@," GEN $(TARGET_DIR)$(QEMU_PROG).stp-installed")
@@ -57,7 +57,7 @@ $(QEMU_PROG).stp: $(SRC_PATH)/trace-events
--format=stap \
--backend=$(TRACE_BACKEND) \
--binary=$(realpath .)/$(QEMU_PROG) \
- --target-arch=$(TARGET_ARCH) \
+ --target-name=$(TARGET_NAME) \
--target-type=$(TARGET_TYPE) \
< $< > $@," GEN $(TARGET_DIR)$(QEMU_PROG).stp")
@@ -103,7 +103,7 @@ endif #CONFIG_LINUX_USER
ifdef CONFIG_BSD_USER
-QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
+QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ABI_DIR)
obj-y += bsd-user/
obj-y += gdbstub.o user-exec.o
@@ -128,7 +128,7 @@ obj-$(CONFIG_XEN) += xen-all.o xen-mapcache.o
obj-$(CONFIG_NO_XEN) += xen-stub.o
# Hardware support
-ifeq ($(TARGET_ARCH), sparc64)
+ifeq ($(TARGET_NAME), sparc64)
obj-y += hw/sparc64/
else
obj-y += hw/$(TARGET_BASE_ARCH)/
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 60ff9c5..bfc261b 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -225,7 +225,7 @@ probes:
scripts/tracetool --dtrace --stap \
--binary path/to/qemu-binary \
--target-type system \
- --target-arch x86_64 \
+ --target-name x86_64 \
qemu.stp
== Trace event properties ==
diff --git a/scripts/tracetool.py b/scripts/tracetool.py
index a79ec0f..5f4890f 100755
--- a/scripts/tracetool.py
+++ b/scripts/tracetool.py
@@ -46,9 +46,9 @@ Options:
--check-backend Check if the given backend is valid.
--binary Full path to QEMU binary.
--target-type QEMU emulator target type ('system' or 'user').
---target-arch QEMU emulator target arch.
+--target-name QEMU emulator target name.
--probe-prefix Prefix for dtrace probe names
- (default: qemu--).\
+ (default: qemu--).\
""" % {
"script" : _SCRIPT,
"backends" : backend_descr,
@@ -66,7 +66,7 @@ def main(args):
_SCRIPT = args[0]
long_opts = [ "backend=", "format=", "help", "list-backends",
"check-backend" ]
-long_opts += [ "binary=", "target-type=", "target-arch=", "probe-prefix=" ]
+long_opts += [ "binary=", "target-type=", "target-name=", "probe-prefix=" ]
try:
opts, args = getopt.getopt(args[1:], "", long_opts)
@@ -78,7 +78,7 @@ def main(args):
arg_format = ""
binary = None
target_type = None
-target_arch = None
+target_name = None
probe_prefix = None
for opt, arg in opts:
if opt == "--help":
@@ -100,8 +100,8 @@ def main(args):
binary = arg
elif opt == '--target-type':
target_type = arg
-elif opt == '--target-arch':
-target_arch = arg
+elif opt == '--target-name':
+target_name = arg
elif opt == '--probe-prefix':
probe_prefix = arg
@@ -122,11 +122,11 @@ def main(args):
error_opt("--binary is required for SystemTAP tapset generator")
if probe_prefix is None and target_type is None:
error_opt("--target-type is required for SystemTAP tapset
generator")
-if probe_prefix is None and target_arch is None:
-error_opt("--target-arch is required for SystemTAP tapset
generator")
+if probe_prefix is None and target_name is None:
+error_opt("--target-name is required for SystemTAP tapset
generator")
if probe_prefix is None:
-probe_prefix = ".".join([ "qemu", target_type, target_arch ])
+probe_prefix = ".".join([ "qemu", target_type, target_name ])
try:
tracetool.generate(sys.stdin, arg_format, arg_backend,
--
1.7.9.5
[Qemu-devel] [PULL 08/10] arm: Remove CONFIG_FDT conditionals
Now that we know we're compiling with libfdt, we can remove the
CONFIG_FDT conditionals.
Signed-off-by: Peter Maydell
Reviewed-by: Edgar E. Iglesias
Reviewed-by: Peter Crosthwaite
Tested-by: Edgar E. Iglesias
Message-id: 1369409217-7553-3-git-send-email-peter.mayd...@linaro.org
---
hw/arm/boot.c |7 ---
1 file changed, 7 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index f451529..defcf15 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -227,7 +227,6 @@ static void set_kernel_args_old(const struct arm_boot_info
*info)
static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
{
-#ifdef CONFIG_FDT
uint32_t *mem_reg_property;
uint32_t mem_reg_propsize;
void *fdt = NULL;
@@ -308,12 +307,6 @@ static int load_dtb(hwaddr addr, const struct
arm_boot_info *binfo)
cpu_physical_memory_write(addr, fdt, size);
return 0;
-
-#else
-fprintf(stderr, "Device tree requested, "
-"but qemu was compiled without fdt support\n");
-return -1;
-#endif
}
static void do_cpu_reset(void *opaque)
--
1.7.9.5
[Qemu-devel] [PULL 00/10] configury queue
Hi; this is a pullrequest for a set of patches to the configure machinery: * Paolo's cleanups of TARGET_ARCH/TARGET_NAME * Peter C's fix for the configure fdt probe against new libfdt * my series making libfdt mandatory on arm/ppc/microblaze The patches have all been on the list and reviewed; this is just gathering them up in the right order for convenience (I fixed up a trivial post-rebase conflict in the 'rename TARGET_ARCH2' patch; and I added some (harmlessly) missing hyphens in my 'configure: Require libfdt' patch.) Please pull. [NB: I don't know if we get enough build-machinery patches to make it worth having a permanent subtree/submaintainer for them; it's merely that at the moment there are enough patches I personally care about that I felt like curating them ;-)] thanks -- PMM The following changes since commit 301255e6303457e10b9a42dc208f80c058004c1c: Merge remote-tracking branch 'mjt/trivial-patches-next' into staging (2013-06-14 07:51:45 -0500) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git configury.next for you to fetch changes up to 187f1bcb9ce8e3cd3f634dd5405f9e5ed02b38ce: ppc: Remove CONFIG_FDT conditionals (2013-06-14 15:34:19 +0100) Alon Levy (1): Add a stp file for usage from build directory Paolo Bonzini (4): build: rename TARGET_ARCH2 to TARGET_NAME build: do not use TARGET_ARCH main: use TARGET_ARCH only for the target-specific #define build: drop TARGET_TYPE Peter Crosthwaite (1): configure: dtc: Probe for libfdt_env.h Peter Maydell (4): configure: Require libfdt for arm, ppc, microblaze softmmu targets arm: Remove CONFIG_FDT conditionals microblaze: Remove CONFIG_FDT conditionals ppc: Remove CONFIG_FDT conditionals Makefile.target| 28 +++-- arch_init.c|4 +-- bsd-user/main.c|6 ++-- configure | 60 default-configs/ppc-softmmu.mak|2 +- default-configs/ppc64-softmmu.mak |4 +-- default-configs/ppcemb-softmmu.mak |2 +- docs/tracing.txt |2 +- hw/arm/boot.c |7 - hw/microblaze/boot.c | 12 hw/ppc/ppc440_bamboo.c |2 -- hw/ppc/spapr_vio.c |6 hw/ppc/virtex_ml507.c | 18 --- include/libfdt_env.h | 36 -- linux-user/main.c |6 ++-- qapi-schema.json | 18 +-- scripts/create_config | 15 +++-- scripts/tracetool.py | 18 +-- 18 files changed, 87 insertions(+), 159 deletions(-) delete mode 100644 include/libfdt_env.h
[Qemu-devel] [PULL 01/10] Add a stp file for usage from build directory
From: Alon Levy For systemtap the location of the process being tapped is crucial, as a result the existing stp file requires installation for use. There are now two files: $(TARGET_DIR)/$(QEMU_PROG).stp-installed: copied to $(tapdir)/$(QEMU_PROG).stp $(TARGET_DIR)/$(QEMU_PROG).stp: pointing to the built binary, usable without installation To use: stap -I $(TARGET_DIR) ... Signed-off-by: Alon Levy Acked-by: Stefan Hajnoczi Signed-off-by: Paolo Bonzini Message-id: 1370349928-20419-2-git-send-email-pbonz...@redhat.com Signed-off-by: Peter Maydell --- Makefile.target | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile.target b/Makefile.target index b0be124..2903897 100644 --- a/Makefile.target +++ b/Makefile.target @@ -35,7 +35,7 @@ config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak ifdef CONFIG_TRACE_SYSTEMTAP -stap: $(QEMU_PROG).stp +stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp ifdef CONFIG_USER_ONLY TARGET_TYPE=user @@ -43,14 +43,24 @@ else TARGET_TYPE=system endif -$(QEMU_PROG).stp: $(SRC_PATH)/trace-events +$(QEMU_PROG).stp-installed: $(SRC_PATH)/trace-events $(call quiet-command,$(TRACETOOL) \ --format=stap \ --backend=$(TRACE_BACKEND) \ --binary=$(bindir)/$(QEMU_PROG) \ --target-arch=$(TARGET_ARCH) \ --target-type=$(TARGET_TYPE) \ + < $< > $@," GEN $(TARGET_DIR)$(QEMU_PROG).stp-installed") + +$(QEMU_PROG).stp: $(SRC_PATH)/trace-events + $(call quiet-command,$(TRACETOOL) \ + --format=stap \ + --backend=$(TRACE_BACKEND) \ + --binary=$(realpath .)/$(QEMU_PROG) \ + --target-arch=$(TARGET_ARCH) \ + --target-type=$(TARGET_TYPE) \ < $< > $@," GEN $(TARGET_DIR)$(QEMU_PROG).stp") + else stap: endif @@ -182,7 +192,7 @@ endif endif ifdef CONFIG_TRACE_SYSTEMTAP $(INSTALL_DIR) "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset" - $(INSTALL_DATA) $(QEMU_PROG).stp "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset" + $(INSTALL_DATA) $(QEMU_PROG).stp-installed "$(DESTDIR)$(qemu_datadir)/../systemtap/tapset/$(QEMU_PROG).stp" endif GENERATED_HEADERS += config-target.h -- 1.7.9.5
[Qemu-devel] [PULL 02/10] build: rename TARGET_ARCH2 to TARGET_NAME
From: Paolo Bonzini
Do not introduce any new use yet.
Signed-off-by: Paolo Bonzini
Message-id: 1370349928-20419-3-git-send-email-pbonz...@redhat.com
Signed-off-by: Peter Maydell
---
Makefile.target |6 +++---
configure | 38 +++---
scripts/create_config |2 +-
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/Makefile.target b/Makefile.target
index 2903897..fcc880b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -15,14 +15,14 @@ QEMU_CFLAGS+=-I$(SRC_PATH)/include
ifdef CONFIG_USER_ONLY
# user emulator name
-QEMU_PROG=qemu-$(TARGET_ARCH2)
+QEMU_PROG=qemu-$(TARGET_NAME)
else
# system emulator name
ifneq (,$(findstring -mwindows,$(libs_softmmu)))
# Terminate program name with a 'w' because the linker builds a windows
executable.
-QEMU_PROGW=qemu-system-$(TARGET_ARCH2)w$(EXESUF)
+QEMU_PROGW=qemu-system-$(TARGET_NAME)w$(EXESUF)
endif # windows executable
-QEMU_PROG=qemu-system-$(TARGET_ARCH2)$(EXESUF)
+QEMU_PROG=qemu-system-$(TARGET_NAME)$(EXESUF)
endif
PROGS=$(QEMU_PROG)
diff --git a/configure b/configure
index 60b0811..1dfa712 100755
--- a/configure
+++ b/configure
@@ -4083,10 +4083,10 @@ fi
for target in $target_list; do
target_dir="$target"
config_target_mak=$target_dir/config-target.mak
-target_arch2=`echo $target | cut -d '-' -f 1`
+target_name=`echo $target | cut -d '-' -f 1`
target_bigendian="no"
-case "$target_arch2" in
+case "$target_name" in
armeb|lm32|m68k|microblaze|mips|mipsn32|mips64|moxie|or32|ppc|ppcemb|ppc64|ppc64abi32|s390x|sh4eb|sparc|sparc64|sparc32plus|xtensaeb)
target_bigendian=yes
;;
@@ -4096,17 +4096,17 @@ target_user_only="no"
target_linux_user="no"
target_bsd_user="no"
case "$target" in
- ${target_arch2}-softmmu)
+ ${target_name}-softmmu)
target_softmmu="yes"
;;
- ${target_arch2}-linux-user)
+ ${target_name}-linux-user)
if test "$linux" != "yes" ; then
error_exit "Target '$target' is only available on a Linux host"
fi
target_user_only="yes"
target_linux_user="yes"
;;
- ${target_arch2}-bsd-user)
+ ${target_name}-bsd-user)
if test "$bsd" != "yes" ; then
error_exit "Target '$target' is only available on a BSD host"
fi
@@ -4124,14 +4124,14 @@ echo "# Automatically generated by configure - do not
modify" > $config_target_m
bflt="no"
target_nptl="no"
-interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_arch2/g"`
+interp_prefix1=`echo "$interp_prefix" | sed "s/%M/$target_name/g"`
gdb_xml_files=""
-TARGET_ARCH="$target_arch2"
+TARGET_ARCH="$target_name"
TARGET_BASE_ARCH=""
TARGET_ABI_DIR=""
-case "$target_arch2" in
+case "$target_name" in
i386)
;;
x86_64)
@@ -4246,14 +4246,14 @@ upper() {
echo "TARGET_ARCH=$TARGET_ARCH" >> $config_target_mak
target_arch_name="`upper $TARGET_ARCH`"
echo "TARGET_$target_arch_name=y" >> $config_target_mak
-echo "TARGET_ARCH2=$target_arch2" >> $config_target_mak
-echo "TARGET_TYPE=TARGET_TYPE_`upper $target_arch2`" >> $config_target_mak
+echo "TARGET_NAME=$target_name" >> $config_target_mak
+echo "TARGET_TYPE=TARGET_TYPE_`upper $target_name`" >> $config_target_mak
echo "TARGET_BASE_ARCH=$TARGET_BASE_ARCH" >> $config_target_mak
if [ "$TARGET_ABI_DIR" = "" ]; then
TARGET_ABI_DIR=$TARGET_ARCH
fi
echo "TARGET_ABI_DIR=$TARGET_ABI_DIR" >> $config_target_mak
-case "$target_arch2" in
+case "$target_name" in
i386|x86_64)
if test "$xen" = "yes" -a "$target_softmmu" = "yes" ; then
echo "CONFIG_XEN=y" >> $config_target_mak
@@ -4264,17 +4264,17 @@ case "$target_arch2" in
;;
*)
esac
-case "$target_arch2" in
+case "$target_name" in
arm|i386|x86_64|ppcemb|ppc|ppc64|s390x)
# Make sure the target and host cpus are compatible
if test "$kvm" = "yes" -a "$target_softmmu" = "yes" -a \
- \( "$target_arch2" = "$cpu" -o \
- \( "$target_arch2" = "ppcemb" -a "$cpu" = "ppc" \) -o \
- \( "$target_arch2" = "ppc64" -a "$cpu" = "ppc" \) -o \
- \( "$target_arch2" = "ppc"-a "$cpu" = "ppc64" \) -o \
- \( "$target_arch2" = "ppcemb" -a "$cpu" = "ppc64" \) -o \
- \( "$target_arch2" = "x86_64" -a "$cpu" = "i386" \) -o \
- \( "$target_arch2" = "i386" -a "$cpu" = "x86_64" \) \) ; then
+ \( "$target_name" = "$cpu" -o \
+ \( "$target_name" = "ppcemb" -a "$cpu" = "ppc" \) -o \
+ \( "$target_name" = "ppc64" -a "$cpu" = "ppc" \) -o \
+ \( "$target_name" = "ppc"-a "$cpu" = "ppc64" \) -o \
+ \( "$target_name" = "ppcemb" -a "$cpu" = "ppc64" \) -o \
+ \( "$target_name" = "x86_64" -a "$cpu" = "i386" \) -o \
+ \( "$target_name" = "i386" -a "$cpu" = "x86_64" \) \) ; then
echo "CONFIG_KVM=y" >> $config_target_mak
if test "$vhost_net" = "yes" ; then
echo "CONFIG_VHOST_NET=y" >> $config_target_mak
diff --git a/scripts/create_config b/scripts/create_config
index 258513a..6461ef6 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -94,7 +94,7
[Qemu-devel] [PULL 07/10] configure: Require libfdt for arm, ppc, microblaze softmmu targets
A number of our softmmu targets (PPC, ARM, Microblaze) now more or less require flattened device tree support for various board models to work correctly. Make libfdt mandatory if the target list includes these, rather than building unhelpful half-functional binaries. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Peter Crosthwaite Tested-by: Edgar E. Iglesias Message-id: 1369409217-7553-2-git-send-email-peter.mayd...@linaro.org --- configure | 20 1 file changed, 20 insertions(+) diff --git a/configure b/configure index 31b7783..ad32f87 100755 --- a/configure +++ b/configure @@ -2486,6 +2486,26 @@ fi ## # fdt probe +# fdt support is mandatory for at least some target architectures, +# so insist on it if we're building those system emulators. +fdt_required=no +for target in $target_list; do + case $target in +arm*-softmmu|ppc*-softmmu|microblaze*-softmmu) + fdt_required=yes +;; + esac +done + +if test "$fdt_required" = "yes"; then + if test "$fdt" = "no"; then +error_exit "fdt disabled but some requested targets require it." \ + "You can turn off fdt only if you also disable all the system emulation" \ + "targets which need it (by specifying a cut down --target-list)." + fi + fdt=yes +fi + if test "$fdt" != "no" ; then fdt_libs="-lfdt" # explicitly check for libfdt_env.h as it is missing in some stable installs -- 1.7.9.5
[Qemu-devel] [PULL 09/10] microblaze: Remove CONFIG_FDT conditionals
Now that we know we're compiling with libfdt we can remove the
CONFIG_FDT conditionals.
Signed-off-by: Peter Maydell
Reviewed-by: Edgar E. Iglesias
Reviewed-by: Peter Crosthwaite
Tested-by: Edgar E. Iglesias
Message-id: 1369409217-7553-4-git-send-email-peter.mayd...@linaro.org
---
hw/microblaze/boot.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index e543d88..3f1d70e 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -61,7 +61,6 @@ static int microblaze_load_dtb(hwaddr addr,
const char *dtb_filename)
{
int fdt_size;
-#ifdef CONFIG_FDT
void *fdt = NULL;
int r;
@@ -81,17 +80,6 @@ static int microblaze_load_dtb(hwaddr addr,
}
cpu_physical_memory_write(addr, fdt, fdt_size);
-#else
-/* We lack libfdt so we cannot manipulate the fdt. Just pass on the blob
- to the kernel. */
-if (dtb_filename) {
-fdt_size = load_image_targphys(dtb_filename, addr, 0x1);
-}
-if (kernel_cmdline) {
-fprintf(stderr,
-"Warning: missing libfdt, cannot pass cmdline to kernel!\n");
-}
-#endif
return fdt_size;
}
--
1.7.9.5
[Qemu-devel] [PULL 10/10] ppc: Remove CONFIG_FDT conditionals
Now that we know we're compiling with libfdt we can remove the
CONFIG_FDT conditionals.
Signed-off-by: Peter Maydell
Reviewed-by: Edgar E. Iglesias
Reviewed-by: Peter Crosthwaite
Tested-by: Edgar E. Iglesias
Message-id: 1369409217-7553-5-git-send-email-peter.mayd...@linaro.org
---
default-configs/ppc-softmmu.mak|2 +-
default-configs/ppc64-softmmu.mak |4 ++--
default-configs/ppcemb-softmmu.mak |2 +-
hw/ppc/ppc440_bamboo.c |2 --
hw/ppc/spapr_vio.c |6 --
hw/ppc/virtex_ml507.c | 18 --
6 files changed, 4 insertions(+), 30 deletions(-)
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index cc3587f..bcaa52f 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -42,6 +42,6 @@ CONFIG_I8259=y
CONFIG_XILINX=y
CONFIG_XILINX_ETHLITE=y
CONFIG_OPENPIC=y
-CONFIG_E500=$(CONFIG_FDT)
+CONFIG_E500=y
# For PReP
CONFIG_MC146818RTC=y
diff --git a/default-configs/ppc64-softmmu.mak
b/default-configs/ppc64-softmmu.mak
index 884ea8a..8b7874e 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -42,8 +42,8 @@ CONFIG_I8259=y
CONFIG_XILINX=y
CONFIG_XILINX_ETHLITE=y
CONFIG_OPENPIC=y
-CONFIG_PSERIES=$(CONFIG_FDT)
-CONFIG_E500=$(CONFIG_FDT)
+CONFIG_PSERIES=y
+CONFIG_E500=y
# For pSeries
CONFIG_PCI_HOTPLUG=y
# For PReP
diff --git a/default-configs/ppcemb-softmmu.mak
b/default-configs/ppcemb-softmmu.mak
index be93e03..61920ff 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -37,6 +37,6 @@ CONFIG_I8259=y
CONFIG_XILINX=y
CONFIG_XILINX_ETHLITE=y
CONFIG_OPENPIC=y
-CONFIG_E500=$(CONFIG_FDT)
+CONFIG_E500=y
# For PReP
CONFIG_MC146818RTC=y
diff --git a/hw/ppc/ppc440_bamboo.c b/hw/ppc/ppc440_bamboo.c
index a55e717..b0c1c02 100644
--- a/hw/ppc/ppc440_bamboo.c
+++ b/hw/ppc/ppc440_bamboo.c
@@ -58,7 +58,6 @@ static int bamboo_load_device_tree(hwaddr addr,
const char *kernel_cmdline)
{
int ret = -1;
-#ifdef CONFIG_FDT
uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) };
char *filename;
int fdt_size;
@@ -115,7 +114,6 @@ static int bamboo_load_device_tree(hwaddr addr,
g_free(fdt);
out:
-#endif
return ret;
}
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 1405c32..304f316 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -34,9 +34,7 @@
#include "hw/ppc/spapr_vio.h"
#include "hw/ppc/xics.h"
-#ifdef CONFIG_FDT
#include
-#endif /* CONFIG_FDT */
/* #define DEBUG_SPAPR */
@@ -94,7 +92,6 @@ VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus,
uint32_t reg)
return NULL;
}
-#ifdef CONFIG_FDT
static int vio_make_devnode(VIOsPAPRDevice *dev,
void *fdt)
{
@@ -159,7 +156,6 @@ static int vio_make_devnode(VIOsPAPRDevice *dev,
return node_off;
}
-#endif /* CONFIG_FDT */
/*
* CRQ handling
@@ -570,7 +566,6 @@ static void spapr_vio_register_types(void)
type_init(spapr_vio_register_types)
-#ifdef CONFIG_FDT
static int compare_reg(const void *p1, const void *p2)
{
VIOsPAPRDevice const *dev1, *dev2;
@@ -655,4 +650,3 @@ int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus
*bus)
return ret;
}
-#endif /* CONFIG_FDT */
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 1b4ce76..709a707 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -141,7 +141,6 @@ static int xilinx_load_device_tree(hwaddr addr,
{
char *path;
int fdt_size;
-#ifdef CONFIG_FDT
void *fdt;
int r;
@@ -162,23 +161,6 @@ static int xilinx_load_device_tree(hwaddr addr,
if (r < 0)
fprintf(stderr, "couldn't set /chosen/bootargs\n");
cpu_physical_memory_write(addr, fdt, fdt_size);
-#else
-/* We lack libfdt so we cannot manipulate the fdt. Just pass on the blob
- to the kernel. */
-fdt_size = load_image_targphys("ppc.dtb", addr, 0x1);
-if (fdt_size < 0) {
-path = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
-if (path) {
-fdt_size = load_image_targphys(path, addr, 0x1);
-g_free(path);
-}
-}
-
-if (kernel_cmdline) {
-fprintf(stderr,
-"Warning: missing libfdt, cannot pass cmdline to kernel!\n");
-}
-#endif
return fdt_size;
}
--
1.7.9.5
Re: [Qemu-devel] [Xen-devel] [BUG 1747]Guest could't find bootable device with memory more than 3600M
On 14/06/13 15:14, George Dunlap wrote: On 14/06/13 12:34, Ian Campbell wrote: On Fri, 2013-06-14 at 11:53 +0100, George Dunlap wrote: On Thu, Jun 13, 2013 at 6:22 PM, Ian Campbell wrote: On Thu, 2013-06-13 at 17:55 +0100, Stefano Stabellini wrote: We could have a xenstore flag somewhere that enables the old behaviour so that people can revert back to qemu-xen-traditional and make the pci hole below 4G even bigger than 448MB, but I think that keeping the old behaviour around is going to make the code more difficult to maintain. The downside of that is that things which worked with the old scheme may not work with the new one though. Early in a release cycle when we have time to discover what has broken then that might be OK, but is post rc4 really the time to be risking it? Yes, you are right: there are some scenarios that would have worked before that wouldn't work anymore with the new scheme. Are they important enough to have a workaround, pretty difficult to identify for a user? That question would be reasonable early in the development cycle. At rc4 the question should be: do we think this problem is so critical that we want to risk breaking something else which currently works for people. Remember that we are invalidating whatever passthrough testing people have already done up to this point of the release. It is also worth noting that the things which this change ends up breaking may for all we know be equally difficult for a user to identify (they are after all approximately the same class of issue). The problem here is that the risk is difficult to evaluate, we just don't know what will break with this change, and we don't know therefore if the cure is worse than the disease. The conservative approach at this point in the release would be to not change anything, or to change the minimal possible number of things (which would preclude changes which impact qemu-trad IMHO). WRT pretty difficult to identify -- the root of this thread suggests the guest entered a reboot loop with "No bootable device", that sounds eminently release notable to me. I also not that it was changing the size of the PCI hole which caused the issue -- which does somewhat underscore the risks involved in this sort of change. But that bug was a bug in the first attempt to fix the root problem. The root problem shows up as qemu crashing at some point because it tried to access invalid guest gpfn space; see http://lists.xen.org/archives/html/xen-devel/2013-03/msg00559.html. Stefano tried to fix it with the above patch, just changing the hole to start at 0xe; but that was incomplete, as it didn't match with hvmloader and seabios's view of the world. That's what this bug report is about. This thread is an attempt to find a better fix. So the root problem is that if we revert this patch, and someone passes through a pci device using qemu-xen (the default) and the MMIO hole is resized, at some point in the future qemu will randomly die. Right, I see, thanks for explaining. If it's a choice between users experiencing, "My VM randomly crashes" and experiencing, "I tried to pass through this device but the guest OS doesn't see it", I'd rather choose the latter. All other things being equal, obviously we all would. But the point I've been trying to make is that we don't know the other consequences of making that fix -- e.g. on existing working configurations. So the choice is "some VMs randomly crash, but other stuff works fine and we have had a reasonable amount of user testing" and "those particular VMs don't crash any more, but we don't know what other stuff no longer works and the existing test base has been at least partially invalidated". I think that at post rc4 in a release we ought to be being pretty conservative about the risks of this sort of change, especially wrt invalidating testing and the unknowns involved. Aren't the configurations which might trip over this issue are going to be in the minority compared to those which we risk breaking? So there are the technical proposals we've been discussing, each of which has different risks. 1. Set the default MMIO hole size to 0xe000. 2. If possible, relocate PCI devices that don't fit in the hole to the 64-bit hole. - Here "if possible" will mean a) the device has a 64-bit BAR, and b) this hasn't been disabled by libxl (probably via a xenstore key). 3. If possible, resize the MMIO hole; otherwise refuse to map the device - Currently "if possible" is always true; the new thing here would be making it possible for libxl to disable this, probably via a xenstore key. Each of these will have different risks for qemu-traditional and qemu-xen. Implementing #3 would have no risk for qemu-traditional, because we won't be changing the way anything works; what works will still work, what is broken (if anything) will still be broken. Implementing #3 for qemu-xen only changes one kind of failure for another. If you resize the
Re: [Qemu-devel] [PATCH 3/5] qcow2: Options to enable discard for freed clusters
Am 14.06.2013 um 16:16 hat Paolo Bonzini geschrieben: > Il 14/06/2013 04:31, Kevin Wolf ha scritto: > >>> > > +s->discard_passthrough[QCOW2_DISCARD_NEVER] = false, > >>> > > +s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true, > >>> > > +s->discard_passthrough[QCOW2_DISCARD_REQUEST] = > >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST, > >>> > > + flags & BDRV_O_UNMAP), > >> > > >> > I think there should not be two ways to enable it, it is confusing. > > Hm, yes... But it's also confusing to have qcow2 provide an incomplete > > set of categories. Maybe we shouldn't have introduced -drive discard=... > > as a global option to begin with. > > > >>> > > +s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] = > >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true), > >>> > > +s->discard_passthrough[QCOW2_DISCARD_OTHER] = > >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false), > >> > > >> > Please document the defaults in qcow2_runtime_opts. (BTW, what is the > >> > rationale?) > > The idea was that discard is slow and therefore disabled by default, > > except when you're doing an expensive snapshot operation that can > > potentially free a lot of space at once with not too many requests, so > > there it's enabled. And if you said -drive discard=on, you obviously > > want guest requests to take effect. > > > > We could let QCOW2_OPT_DISCARD_OTHER default to BDRV_O_UNMAP as well if > > you prefer. > > It looks like QCOW2_OPT_DISCARD_OTHER is a rare case, so I don't mind > leaving it as default to false. It won't waste more than a few clusters. Yes, it's generally relatively rare, like growing L1 or refcount table. There is one case where it should trigger a lot, though: Overwriting clusters of a compressed image. Hm, though actually it doesn't make a lot of sense there. The freed cluster will immediately be used by the next write. Maybe COW should actually be QCOW2_OPT_DISCARD_NEVER... > In the end discard_snapshot and discard_other should rarely be needed in > practice, so I don't think having discard=... is a mistake. Too many > knobs won't really be needed. > > In fact, perhaps we do not need discard_snapshot and discard_request, > only discard_other. discard_snapshot can be replaced by > file.discard=ignore, discard_request by discard=unmap. This is only true if you rule out some combination as useless. For example you would say that if you want to process guest requests, you always want to have snapshots discarded as well. You also assume that nobody wants the current behaviour (free clusters in qcow2 metadata, but don't send discards to raw-posix). Isn't this assuming a bit too much? To be clear, I don't expect these knobs to be used much either, but I have some feeling that some people (including us while debugging or asking questions) may be glad later to have such low-level options that control each layer separately. Kevin
Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Change default machine for 64-bit
Am 13.06.2013 15:49, schrieb Alexander Graf: > On 17.05.2013, at 09:42, Andreas Färber wrote: >> Am 17.05.2013 06:25, schrieb David Gibson: >>> Currently, for qemu-system-ppc64, the default machine type is 'mac99'. >>> Since the mac99 machine is not being actively maintained, and shows quite >>> a few signs of bitrot, >> >> Please be more specific than making such general claims in a commit >> message! As the default machine it certainly compiles, so where are you >> seeing bitrot? The DEC bridge cleanup that I once started kind of >> depends on the PCI cleanup you recently looked into. > > The mac99 machine for 64bit is actually worse than anything bitrot could give > you. It emulates a machine that in its form never possibly could have existed > in real hardware, which makes it very fragile and dependent on the guest's > mercy to handle this gracefully. Still we should put that in the commit message and not "shows quite a few signs of bitrot". Bitrot is what gus.c and cs4231a.c may have endured while not being compiled in or those disabled DPRINTF()s sprinkled all over the code base. Further, David is looking at this from a biased perspective: -M pseries is best maintained (apart from e500) ppc target, but it doesn't work with PR KVM (not on my box anyway). But neither does mac99 on KVM due to page sizes or something IIRC. However under TCG either is okay with the guests I've seen so far and they all compile warning-free. pseries used to hang, that is no longer the case, it just doesn't do anything now - still need to debug how to improve that. > Given the current state and amount of development on the pseries target, I > think it makes sense to declare that as the default for qemu-system-ppc64. No disagreement there then. Andreas > I would still love to see -M mac99 emulate a proper U2 or U1 based system for > 32bit. And I would also love to see a real U4 based emulation model come up > for qemu-system-ppc64. But I doubt I'll have time to work on either :). > > > Alex -- 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] [RFC 04/13] virtio-blk: implement BlockDevOps->drain_threads_cb()
Il 14/06/2013 05:48, Stefan Hajnoczi ha scritto:
> Drain and stop the dataplane thread when bdrv_drain_all() is called.
> Note that the thread will be restarted in virtio_blk_handle_output() the
> next time the guest kicks the virtqueue.
Long term I still think we want to move towards
aio_lock(bs)
bdrv_drain(bs)
...
aio_unlock(bs)
but this approach works well at this point. It is a hack, but a very
smart one. :)
Add a comment in the code about what will start the dataplane thread,
though.
Paolo
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/block/virtio-blk.c | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index cf12469..f9c2b79 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -618,8 +618,20 @@ static void virtio_blk_resize(void *opaque)
> virtio_notify_config(vdev);
> }
>
> +static void virtio_blk_drain_threads(void *opaque)
> +{
> +#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> +VirtIOBlock *s = VIRTIO_BLK(opaque);
> +
> +if (s->dataplane) {
> +virtio_blk_data_plane_stop(s->dataplane);
> +}
> +#endif
> +}
> +
> static const BlockDevOps virtio_block_ops = {
> .resize_cb = virtio_blk_resize,
> +.drain_threads_cb = virtio_blk_drain_threads,
> };
>
> void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
>
Re: [Qemu-devel] [PATCH 3/5] qcow2: Options to enable discard for freed clusters
Il 14/06/2013 04:31, Kevin Wolf ha scritto: >>> > > +s->discard_passthrough[QCOW2_DISCARD_NEVER] = false, >>> > > +s->discard_passthrough[QCOW2_DISCARD_ALWAYS] = true, >>> > > +s->discard_passthrough[QCOW2_DISCARD_REQUEST] = >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_REQUEST, >>> > > + flags & BDRV_O_UNMAP), >> > >> > I think there should not be two ways to enable it, it is confusing. > Hm, yes... But it's also confusing to have qcow2 provide an incomplete > set of categories. Maybe we shouldn't have introduced -drive discard=... > as a global option to begin with. > >>> > > +s->discard_passthrough[QCOW2_DISCARD_SNAPSHOT] = >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_SNAPSHOT, true), >>> > > +s->discard_passthrough[QCOW2_DISCARD_OTHER] = >>> > > +qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false), >> > >> > Please document the defaults in qcow2_runtime_opts. (BTW, what is the >> > rationale?) > The idea was that discard is slow and therefore disabled by default, > except when you're doing an expensive snapshot operation that can > potentially free a lot of space at once with not too many requests, so > there it's enabled. And if you said -drive discard=on, you obviously > want guest requests to take effect. > > We could let QCOW2_OPT_DISCARD_OTHER default to BDRV_O_UNMAP as well if > you prefer. It looks like QCOW2_OPT_DISCARD_OTHER is a rare case, so I don't mind leaving it as default to false. It won't waste more than a few clusters. In the end discard_snapshot and discard_other should rarely be needed in practice, so I don't think having discard=... is a mistake. Too many knobs won't really be needed. In fact, perhaps we do not need discard_snapshot and discard_request, only discard_other. discard_snapshot can be replaced by file.discard=ignore, discard_request by discard=unmap. Paolo
Re: [Qemu-devel] [Xen-devel] [BUG 1747]Guest could't find bootable device with memory more than 3600M
On 14/06/13 12:34, Ian Campbell wrote: On Fri, 2013-06-14 at 11:53 +0100, George Dunlap wrote: On Thu, Jun 13, 2013 at 6:22 PM, Ian Campbell wrote: On Thu, 2013-06-13 at 17:55 +0100, Stefano Stabellini wrote: We could have a xenstore flag somewhere that enables the old behaviour so that people can revert back to qemu-xen-traditional and make the pci hole below 4G even bigger than 448MB, but I think that keeping the old behaviour around is going to make the code more difficult to maintain. The downside of that is that things which worked with the old scheme may not work with the new one though. Early in a release cycle when we have time to discover what has broken then that might be OK, but is post rc4 really the time to be risking it? Yes, you are right: there are some scenarios that would have worked before that wouldn't work anymore with the new scheme. Are they important enough to have a workaround, pretty difficult to identify for a user? That question would be reasonable early in the development cycle. At rc4 the question should be: do we think this problem is so critical that we want to risk breaking something else which currently works for people. Remember that we are invalidating whatever passthrough testing people have already done up to this point of the release. It is also worth noting that the things which this change ends up breaking may for all we know be equally difficult for a user to identify (they are after all approximately the same class of issue). The problem here is that the risk is difficult to evaluate, we just don't know what will break with this change, and we don't know therefore if the cure is worse than the disease. The conservative approach at this point in the release would be to not change anything, or to change the minimal possible number of things (which would preclude changes which impact qemu-trad IMHO). WRT pretty difficult to identify -- the root of this thread suggests the guest entered a reboot loop with "No bootable device", that sounds eminently release notable to me. I also not that it was changing the size of the PCI hole which caused the issue -- which does somewhat underscore the risks involved in this sort of change. But that bug was a bug in the first attempt to fix the root problem. The root problem shows up as qemu crashing at some point because it tried to access invalid guest gpfn space; see http://lists.xen.org/archives/html/xen-devel/2013-03/msg00559.html. Stefano tried to fix it with the above patch, just changing the hole to start at 0xe; but that was incomplete, as it didn't match with hvmloader and seabios's view of the world. That's what this bug report is about. This thread is an attempt to find a better fix. So the root problem is that if we revert this patch, and someone passes through a pci device using qemu-xen (the default) and the MMIO hole is resized, at some point in the future qemu will randomly die. Right, I see, thanks for explaining. If it's a choice between users experiencing, "My VM randomly crashes" and experiencing, "I tried to pass through this device but the guest OS doesn't see it", I'd rather choose the latter. All other things being equal, obviously we all would. But the point I've been trying to make is that we don't know the other consequences of making that fix -- e.g. on existing working configurations. So the choice is "some VMs randomly crash, but other stuff works fine and we have had a reasonable amount of user testing" and "those particular VMs don't crash any more, but we don't know what other stuff no longer works and the existing test base has been at least partially invalidated". I think that at post rc4 in a release we ought to be being pretty conservative about the risks of this sort of change, especially wrt invalidating testing and the unknowns involved. Aren't the configurations which might trip over this issue are going to be in the minority compared to those which we risk breaking? So there are the technical proposals we've been discussing, each of which has different risks. 1. Set the default MMIO hole size to 0xe000. 2. If possible, relocate PCI devices that don't fit in the hole to the 64-bit hole. - Here "if possible" will mean a) the device has a 64-bit BAR, and b) this hasn't been disabled by libxl (probably via a xenstore key). 3. If possible, resize the MMIO hole; otherwise refuse to map the device - Currently "if possible" is always true; the new thing here would be making it possible for libxl to disable this, probably via a xenstore key. Each of these will have different risks for qemu-traditional and qemu-xen. Implementing #3 would have no risk for qemu-traditional, because we won't be changing the way anything works; what works will still work, what is broken (if anything) will still be broken. Implementing #3 for qemu-xen only changes one kind of failure for another. If you resize the MMIO hole for qemu-xen, then you *will* eventually crash.
Re: [Qemu-devel] [SeaBIOS] solaris x86 in qemu? [bisected]
14.06.2013 16:36, Gerd Hoffmann wrote: > Hi, > >> Hmm. Speaking of the splitting. Does the current bios include the >> the tables which were split into separate files? > > Yes, they are in out/ too after building seabios. Use "qemu -L > /path/to/seabios/out" to make sure qemu picks up matching bios.bin and dsdt. Yes I know they're built in seabios/out, and I know one may use -L option here. So that, for example, -bios option of qemu makes less sense now than it was before. But my question was about something different. Can I use current seabios with old qemu which does not provide the separate ACPI tables? For example, does current bios contain these tables too, so they're both separate and embedded? And the reverse, can I use old bios with new qemu which do provide the separate tables? Which tables will be used in each case? Thanks, /mjt
Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-platform device initialization
> -Original Message- > From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > Bonzini > Sent: 14 June 2013 14:51 > To: Paul Durrant > Cc: Ian Campbell; Stefano Stabellini; qemu-devel@nongnu.org; xen- > de...@lists.xen.org > Subject: Re: [Xen-devel] [PATCH] Remove hardcoded xen-platform device > initialization > > Il 14/06/2013 06:38, Paul Durrant ha scritto: > I think the right solution for this is to move towards using > the normal "-M pc" machine. libxl can simply use "-M pc > -machine accel=xen -device xen-platform-pv"; older versions > that use the xenfv machine will still work. > > And if you do this, you will also get the benefit of > versioned machine types. > > >> > >> Thanks. I'll have a look at that. > > > > It's a little more complicated than I thought. There are two machine > > types, xenpv and xenfv (i.e. HVM), and they share the accel=xen > > option. > > Yes, I was talking of xenfv only. > > > Thus QEMU attaches to the VM in the machine code rather than > > the accelerator init code. Using -M pc is therefore not an option, > > unless we perhaps have separate accel options. > > You're talking about xen_hvm_init, right? IIRC there is a hypercall > that lets you know if a domain is PV or FV so you could move large parts > of it to accelerator init. What's left can be done in "if > (xen_enabled())" (especially those parts that have matching TCG/KVM code > in normal "-M pc" initialization, and have that code currently disabled > for Xen: unifying the two or at least doing an if/else would be nicer). > > > Either way this > > doesn't address the backwards compatibility issue. I think QEMU is > > going to need to know which version of the Xen toolstack invoked it > > unless we specify a compatibility matrix. > > Yes, "xenfv" needs to stay as legacy for compatibility purposes. But if > you move the toolchain and QEMU towards using "-M pc" (requiring new > QEMU for new toolchains that do) it would be a very nice cleanup. It is > also needed if you ever want to support Q35/PCIe. > I think we're still going to need -M xenpv, I think; it's quite distinct from pc. I guess we could use -M pc for HVM and gate the accel code as you suggest but, if that's the way we're going, it would seem more logical just to ditch the accel code for xenpv completely (assuming we can do all we need from the machine init) and then use -M pc -accel=xen for HVM guests going forward. But that does rather screw up my autodiscovery plans because I would not know, for a given qemu binary, which machine type to use. If I create a new xenfv-2.0 machine type though I *can* do auto discovery... in which case do we need the -accel=xen option at all? Paul
Re: [Qemu-devel] [RFC 08/13] block: drop bdrv_get_aio_context()
Il 14/06/2013 05:48, Stefan Hajnoczi ha scritto: > Associating a BlockDriverState with a single AioContext is not flexible > enough. Once we make BlockDriverState thread-safe, it will be possible > to call bdrv_*() functions from multiple event loops. I'm afraid that this is trading some pain now (converting qemu_bh_new/qemu_set_fd_handler to aio_bh_new/aio_set_fd_handler) for more pain later (having to make BDS thread-safe). There aren't that many (~40) in block layer code. Making BlockDriverState thread-safe is hard, it is much simpler to run all the BlockDriverState code in the AioContext thread itself. There are some things that cannot (basically monitor commands and other places that are currently using bdrv_drain_all) but they can simply take a "big AioContext lock". Paolo
Re: [Qemu-devel] [Qemu-trivial] [PATCH] cputlb: fix debug logs
Am 05.06.2013 21:51, schrieb Michael Tokarev:
> 05.06.2013 16:16, Hervé Poussineau wrote:
>> 'pd' variable has been removed in 06ef3525e1f271b6a842781a05eace5cf63b95c2.
>
> It's been removed indeed, but the value has been replaced by using
> a MemoryRegionSection instead. I understand current code is wrong
> when DEBUG_TLB is #defined, but I think it is also wrong to just
> remove this value in printf. The question is whenever this debugging
> is useful now, and whenever this now-missing value is interesting
> for that and should be provided by other means. Maybe we may just
> remove whole thing here and everywhere.
I interpret it as logging the function arguments, so dropping it sounded
like the right solution to me. Would be nice to turn it into a
tracepoint or at least some DPRINTF() macro that gets compile-tested.
Andreas
>
> Cc'ing authors.
>
> While this patch looks really trivial, it is something which is of
> use by the subsystem maintainer(s) at their disposal.
>
> Thanks,
>
> /mjt
>
>> Signed-off-by: Hervé Poussineau
>> ---
>> cputlb.c |4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/cputlb.c b/cputlb.c
>> index 8c8..1230e9e 100644
>> --- a/cputlb.c
>> +++ b/cputlb.c
>> @@ -262,8 +262,8 @@ void tlb_set_page(CPUArchState *env, target_ulong vaddr,
>>
>> #if defined(DEBUG_TLB)
>> printf("tlb_set_page: vaddr=" TARGET_FMT_lx " paddr=0x" TARGET_FMT_plx
>> - " prot=%x idx=%d pd=0x%08lx\n",
>> - vaddr, paddr, prot, mmu_idx, pd);
>> + " prot=%x idx=%d\n",
>> + vaddr, paddr, prot, mmu_idx);
>> #endif
>>
>> address = vaddr;
>>
>
>
--
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] [PATCH v3 15/16] boot-order-test: Support fw_cfg in I/O space
Am 14.06.2013 15:53, schrieb Anthony Liguori:
> Markus Armbruster writes:
>
>> Next commit needs it.
>>
>> Cc: Blue Swirl
>> Signed-off-by: Markus Armbruster
>> ---
>> tests/boot-order-test.c | 24
>> 1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
>> index 7b1edc1..d1d99f8 100644
>> --- a/tests/boot-order-test.c
>> +++ b/tests/boot-order-test.c
>> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
>> test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
>> }
>>
>> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
>> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
>> void *buf, size_t len)
>
> I missed it earlier, but you can use libqos/fw_cfg.h for this.
Agree, but that didn't exist at the time. :-)
Regards,
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] [PATCH v3 0/3] TPM NVRAM persistent storage
Corey Bryant writes: > This patch series provides persistent storage support that a TPM > can use to store NVRAM data. It uses QEMU's block driver to store > data on a drive image. The libtpms TPM 1.2 backend will be the > initial user of this functionality to store data that must persist > through a reboot or migration. A sample command line may look like > this: This should be folded into the libtpms backend series. There are no users for this so this would just be untestable code in the tree subject to bitrot. Regards, Anthony Liguori > > qemu-system-x86_64 ... > -drive file=/path/to/nvram.qcow2,id=drive-nvram0-0-0 > -tpmdev libtpms,id=tpm-tpm0 > -device tpm-tis,tpmdev=tpm-tpm0,id=tpm0,drive=drive-nvram0-0-0 > > Thanks, > Corey > > Corey Bryant (3): > nvram: Add TPM NVRAM implementation > nvram: Add tpm-tis drive support > TPM NVRAM test > > hw/tpm/Makefile.objs |1 + > hw/tpm/tpm_int.h |2 + > hw/tpm/tpm_nvram.c | 324 > ++ > hw/tpm/tpm_nvram.h | 25 > hw/tpm/tpm_passthrough.c | 85 > hw/tpm/tpm_tis.c |8 + > 6 files changed, 445 insertions(+), 0 deletions(-) > create mode 100644 hw/tpm/tpm_nvram.c > create mode 100644 hw/tpm/tpm_nvram.h
[Qemu-devel] [RFC PATCH v6 2/3] Add 'auto-converge' migration capability
The auto-converge migration capability allows the user to specify if they
choose live migration seqeunce to automatically detect and force convergence.
Signed-off-by: Chegu Vinod
Reviewed-by: Paolo Bonzini
Reviewed-by: Eric Blake
---
include/migration/migration.h |2 ++
migration.c |9 +
qapi-schema.json |5 -
3 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/include/migration/migration.h b/include/migration/migration.h
index e2acec6..ace91b0 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -127,4 +127,6 @@ int migrate_use_xbzrle(void);
int64_t migrate_xbzrle_cache_size(void);
int64_t xbzrle_cache_resize(int64_t new_size);
+
+bool migrate_auto_converge(void);
#endif
diff --git a/migration.c b/migration.c
index 058f9e6..d0759c1 100644
--- a/migration.c
+++ b/migration.c
@@ -473,6 +473,15 @@ void qmp_migrate_set_downtime(double value, Error **errp)
max_downtime = (uint64_t)value;
}
+bool migrate_auto_converge(void)
+{
+MigrationState *s;
+
+s = migrate_get_current();
+
+return s->enabled_capabilities[MIGRATION_CAPABILITY_AUTO_CONVERGE];
+}
+
int migrate_use_xbzrle(void)
{
MigrationState *s;
diff --git a/qapi-schema.json b/qapi-schema.json
index 5ad6894..882a7fd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -605,10 +605,13 @@
# This feature allows us to minimize migration traffic for certain
work
# loads, by sending compressed difference of the pages
#
+# @auto-converge: If enabled, QEMU will automatically throttle down the guest
+# to speed up convergence of RAM migration. (since 1.6)
+#
# Since: 1.2
##
{ 'enum': 'MigrationCapability',
- 'data': ['xbzrle'] }
+ 'data': ['xbzrle', 'auto-converge'] }
##
# @MigrationCapabilityStatus
--
1.7.1
[Qemu-devel] [RFC PATCH v6 0/3] Throttle-down guest to help with live migration convergence
Busy enterprise workloads hosted on large sized VM's tend to dirty memory faster than the transfer rate achieved via live guest migration. Despite some good recent improvements (& using dedicated 10Gig NICs between hosts) the live migration does NOT converge. If a user chooses to force convergence of their migration via a new migration capability "auto-converge" then this change will auto-detect lack of convergence scenario and trigger a slow down of the workload by explicitly disallowing the VCPUs from spending much time in the VM context. The migration thread tries to catchup and this eventually leads to convergence in some "deterministic" amount of time. Yes it does impact the performance of all the VCPUs but in our observation that lasts only for a short duration of time. i.e. end up entering stage 3 (downtime phase) soon after that. No external monitoring/triggers are required. Thanks to Juan and Paolo for their useful suggestions. --- Changes from v5: - incorporated feedback from Paolo & Igor. - rebased to latest qemu.git Changes from v4: - incorporated feedback from Paolo. - split into 3 patches. Changes from v3: - incorporated feedback from Paolo and Eric - rebased to latest qemu.git Changes from v2: - incorporated feedback from Orit, Juan and Eric - stop the throttling thread at the start of stage 3 - rebased to latest qemu.git Changes from v1: - rebased to latest qemu.git - added auto-converge capability(default off) - suggested by Anthony Liguori & Eric Blake. Signed-off-by: Chegu Vinod --- Chegu Vinod (3): Introduce async_run_on_cpu() Add 'auto-converge' migration capability Force auto-convegence of live migration arch_init.c | 85 + cpus.c| 29 ++ include/migration/migration.h |2 + include/qemu-common.h |1 + include/qom/cpu.h | 10 + migration.c |9 qapi-schema.json |5 ++- 7 files changed, 140 insertions(+), 1 deletions(-)
[Qemu-devel] [RFC PATCH v6 3/3] Force auto-convegence of live migration
If a user chooses to turn on the auto-converge migration capability
these changes detect the lack of convergence and throttle down the
guest. i.e. force the VCPUs out of the guest for some duration
and let the migration thread catchup and help converge.
Verified the convergence using the following :
- Java Warehouse workload running on a 20VCPU/256G guest(~80% busy)
- OLTP like workload running on a 80VCPU/512G guest (~80% busy)
Sample results with Java warehouse workload : (migrate speed set to 20Gb and
migrate downtime set to 4seconds).
(qemu) info migrate
capabilities: xbzrle: off auto-converge: off <
Migration status: active
total time: 1487503 milliseconds
expected downtime: 519 milliseconds
transferred ram: 383749347 kbytes
remaining ram: 2753372 kbytes
total ram: 268444224 kbytes
duplicate: 65461532 pages
skipped: 64901568 pages
normal: 95750218 pages
normal bytes: 383000872 kbytes
dirty pages rate: 67551 pages
---
(qemu) info migrate
capabilities: xbzrle: off auto-converge: on <
Migration status: completed
total time: 241161 milliseconds
downtime: 6373 milliseconds
transferred ram: 28235307 kbytes
remaining ram: 0 kbytes
total ram: 268444224 kbytes
duplicate: 64946416 pages
skipped: 64903523 pages
normal: 7044971 pages
normal bytes: 28179884 kbytes
Signed-off-by: Chegu Vinod
---
arch_init.c | 85 +++
1 files changed, 85 insertions(+), 0 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..69c6c8c 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -104,6 +104,8 @@ int graphic_depth = 15;
#endif
const uint32_t arch_type = QEMU_ARCH;
+static bool mig_throttle_on;
+static void throttle_down_guest_to_converge(void);
/***/
/* ram save/restore */
@@ -378,8 +380,15 @@ static void migration_bitmap_sync(void)
uint64_t num_dirty_pages_init = migration_dirty_pages;
MigrationState *s = migrate_get_current();
static int64_t start_time;
+static int64_t bytes_xfer_prev;
static int64_t num_dirty_pages_period;
int64_t end_time;
+int64_t bytes_xfer_now;
+static int dirty_rate_high_cnt;
+
+if (!bytes_xfer_prev) {
+bytes_xfer_prev = ram_bytes_transferred();
+}
if (!start_time) {
start_time = qemu_get_clock_ms(rt_clock);
@@ -404,6 +413,23 @@ static void migration_bitmap_sync(void)
/* more than 1 second = 1000 millisecons */
if (end_time > start_time + 1000) {
+if (migrate_auto_converge()) {
+/* The following detection logic can be refined later. For now:
+ Check to see if the dirtied bytes is 50% more than the approx.
+ amount of bytes that just got transferred since the last time we
+ were in this routine. If that happens >N times (for now N==4)
+ we turn on the throttle down logic */
+bytes_xfer_now = ram_bytes_transferred();
+if (s->dirty_pages_rate &&
+((num_dirty_pages_period*TARGET_PAGE_SIZE) >
+((bytes_xfer_now - bytes_xfer_prev)/2))) {
+if (dirty_rate_high_cnt++ > 4) {
+DPRINTF("Unable to converge. Throtting down guest\n");
+mig_throttle_on = true;
+}
+ }
+ bytes_xfer_prev = bytes_xfer_now;
+}
s->dirty_pages_rate = num_dirty_pages_period * 1000
/ (end_time - start_time);
s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE;
@@ -628,6 +654,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
}
total_sent += bytes_sent;
acct_info.iterations++;
+throttle_down_guest_to_converge();
/* we want to check in the 1st loop, just in case it was the 1st time
and we had to sync the dirty bitmap.
qemu_get_clock_ns() is a bit expensive, so we only check each some
@@ -1098,3 +1125,61 @@ TargetInfo *qmp_query_target(Error **errp)
return info;
}
+
+static bool throttling_needed(void)
+{
+if (!migrate_auto_converge()) {
+return false;
+}
+return mig_throttle_on;
+}
+
+/* Stub function that's gets run on the vcpu when its brought out of the
+ VM to run inside qemu via async_run_on_cpu()*/
+static void mig_sleep_cpu(void *opq)
+{
+qemu_mutex_unlock_iothread();
+g_usleep(30*1000);
+qemu_mutex_lock_iothread();
+}
+
+/* To reduce the dirty rate explicitly disallow the VCPUs from spending
+ much time in the VM. The migration thread will try to catchup.
+ Workload will experience a performance drop.
+*/
+static void mig_throttle_cpu_down(CPUState *cpu, void *data)
+{
+async_run_on_cpu(cpu, mig_sleep_cpu, NULL);
+}
+
+static void mig_throttle_guest_down(void)
+{
+if (throttling_needed()) {
+qemu_mutex_lock_iothread();
+qemu_for_each_cpu(mig_throttle_cpu_down, NULL);
+
[Qemu-devel] [RFC PATCH v6 1/3] Introduce async_run_on_cpu()
Introduce an asynchronous version of run_on_cpu() i.e. the caller
doesn't have to block till the call back routine finishes execution
on the target vcpu.
Signed-off-by: Chegu Vinod
Reviewed-by: Paolo Bonzini
---
cpus.c| 29 +
include/qemu-common.h |1 +
include/qom/cpu.h | 10 ++
3 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/cpus.c b/cpus.c
index c232265..8cd4eab 100644
--- a/cpus.c
+++ b/cpus.c
@@ -653,6 +653,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data),
void *data)
wi.func = func;
wi.data = data;
+wi.free = false;
if (cpu->queued_work_first == NULL) {
cpu->queued_work_first = &wi;
} else {
@@ -671,6 +672,31 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data),
void *data)
}
}
+void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
+{
+struct qemu_work_item *wi;
+
+if (qemu_cpu_is_self(cpu)) {
+func(data);
+return;
+}
+
+wi = g_malloc0(sizeof(struct qemu_work_item));
+wi->func = func;
+wi->data = data;
+wi->free = true;
+if (cpu->queued_work_first == NULL) {
+cpu->queued_work_first = wi;
+} else {
+cpu->queued_work_last->next = wi;
+}
+cpu->queued_work_last = wi;
+wi->next = NULL;
+wi->done = false;
+
+qemu_cpu_kick(cpu);
+}
+
static void flush_queued_work(CPUState *cpu)
{
struct qemu_work_item *wi;
@@ -683,6 +709,9 @@ static void flush_queued_work(CPUState *cpu)
cpu->queued_work_first = wi->next;
wi->func(wi->data);
wi->done = true;
+if (wi->free) {
+g_free(wi);
+}
}
cpu->queued_work_last = NULL;
qemu_cond_broadcast(&qemu_work_cond);
diff --git a/include/qemu-common.h b/include/qemu-common.h
index ed8b6e2..ac0ed38 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -302,6 +302,7 @@ struct qemu_work_item {
void (*func)(void *data);
void *data;
int done;
+bool free;
};
#ifdef CONFIG_USER_ONLY
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 7cd9442..46465e9 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -265,6 +265,16 @@ bool cpu_is_stopped(CPUState *cpu);
void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
/**
+ * async_run_on_cpu:
+ * @cpu: The vCPU to run on.
+ * @func: The function to be executed.
+ * @data: Data to pass to the function.
+ *
+ * Schedules the function @func for execution on the vCPU @cpu asynchronously.
+ */
+void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
+
+/**
* qemu_for_each_cpu:
* @func: The function to be executed.
* @data: Data to pass to the function.
--
1.7.1
Re: [Qemu-devel] [PATCH] Remove hardcoded xen-platform device initialization
> -Original Message- > From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo > Bonzini > Sent: 14 June 2013 14:53 > To: Paul Durrant > Cc: Stefano Stabellini; qemu-devel@nongnu.org; xen-de...@lists.xen.org > Subject: Re: [PATCH] Remove hardcoded xen-platform device initialization > > Il 14/06/2013 04:50, Paul Durrant ha scritto: > >> > This patch is problematic because we can't know for sure the version of > >> > upstream QEMU that is going to be used with Xen. > >> > If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2, > >> > guests won't be able to use PV drivers. > >> > > > Is there not a compatibility matrix? The hardcoded init. is just blatantly > > the > wrong thing to be doing and it needs to go. > > > > Could my accompanying toolstack patch not be backported to the next 4.2 > release as mitigation? > > I think Ian is right. You should revert the toolstack patch and not > apply this one for now. Then aim at using "-M pc" in 4.4 (and require > 1.6 or 1.7), so that there is no need for a compatibility matrix. > As I said in another mail on the thread, I don't think we'll ever be able to use -M pc, so I'm planning to create a xenfv-2.0 machine type and leave xenfv with the hardcoded device creation semantic. Then I'll add code to libxl to parse qemu's response to -M help and choose xenfv-2.0 if it exists (and specify the -device argument accordingly). Paul
Re: [Qemu-devel] [PATCH 2/2] Add monitor command mem-nodes
On Thu, Jun 13, 2013 at 08:04:00PM -0500, Anthony Liguori wrote: > Eduardo Habkost writes: > > > On Wed, Jun 05, 2013 at 07:57:42AM -0500, Anthony Liguori wrote: > >> Wanlong Gao writes: > >> > >> > Add monitor command mem-nodes to show the huge mapped > >> > memory nodes locations. > >> > > >> > (qemu) info mem-nodes > >> > /proc/14132/fd/13: 2ac0-2aaaeac0: node0 > >> > /proc/14132/fd/13: 2aaaeac0-2aab2ac0: node1 > >> > /proc/14132/fd/14: 2aab2ac0-2aab2b00: node0 > >> > /proc/14132/fd/14: 2aab2b00-2aab2b40: node1 > >> > >> This creates an ABI that we don't currently support. Memory hotplug or > >> a variety of things can break this mapping and then we'd have to provide > >> an interface to describe that the mapping was broken. > > > > What do you mean by "breaking this mapping", exactly? Would the backing > > file of existing guest RAM ever change? (It would require a memory copy > > from one file to another, why would QEMU ever do that?) > > Memory hot-add will change the mapping. hot-remove (if ever > implemented) would break it. So, would the backing-file/offset of existing guest RAM ever change? (It would require a memory copy from one file to another, why would QEMU ever do that?) [...] > > > > Does THP work with tmpfs, already? > > No. OK, that's a real problem. > > If it does, people who doesn't want > > hugetlbfs and want numa tuning to work with THP could just use tmpfs for > > -mem-path. > > > >> > >> I had hoped that we would get proper userspace interfaces for describing > >> memory groups but that appears to have stalled out. > > > > I would love to have it. But while we don't have it, sharing the > > tmpfs/hugetlbfs backing files seem to work just fine as a mechanism to > > let other tools manipulate guest memory policy. We just need to let > > external tools know where the backing files are. > > Is this meant for numad? Wouldn't you want numad to work without > hugetlbfs? > > You have to preallocate pages to hugetlbfs. It's very difficult to use > in practice. If you don't want hugetlbfs you could use tmpfs, and set the policy on the tmpfs files. What I am asking is: why do we need to ask the kernel folks for interfaces to define and set policies on memory groups if we can (in theory) do the exactly same using tmpfs and hugetlbfs files? (But the fact that THP doesn't work with tmpfs is a real problem, as I said above) > >> > >> Does anyone know if this is still on the table? > >> > >> If we can't get a proper kernel interface, then perhaps we need to add > >> full libnuma support but that would really be unfortunate... > > > > Why isn't the "info mem-nodes" solution (I mean: not this version, but a > > proper QMP version that exposes all the information we need) an > > option? > > We're exposing internal QEMU information (the HVA -> GPA mapping) as an > external stable interface. I never wanted to expose the HVA -> GPA mapping. What I want to expose is: * The virtual-NUMA-node -> GPA-range mapping * The GPA -> mem-path file/offset mapping (Alternatively, a simple virtual-NUMA-node -> mem-path file/offset mapping would be enough, too) We could even replace "mem-path file/offset mapping" with "memory groups", if the kernel already had interfaces to deal with memory groups. -- Eduardo
Re: [Qemu-devel] [PATCH v2 03/17] memory: add ref/unref calls
Il 14/06/2013 06:09, Alexey Kardashevskiy ha scritto: > And - crash: > > > #0 object_unref (obj=0xa7a7a7a7a7a7a7a7) at > /home/alexey/pcipassthru/qemu-impreza/qom/object.c:691 Dangling pointer. One ref, two unrefs probably. I'm redoing the series according to Peter's request, it could fix it automatically. Paolo
Re: [Qemu-devel] [PATCH v3 15/16] boot-order-test: Support fw_cfg in I/O space
Markus Armbruster writes:
> Next commit needs it.
>
> Cc: Blue Swirl
> Signed-off-by: Markus Armbruster
> ---
> tests/boot-order-test.c | 24
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index 7b1edc1..d1d99f8 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -133,23 +133,31 @@ static void test_prep_boot_order(void)
> test_boot_orders("prep", read_boot_order_prep, test_cases_prep);
> }
>
> -static void read_fw_cfg(uint64_t cfg_addr, uint16_t cmd,
> +static void read_fw_cfg(uint64_t cfg_addr, bool addr_is_io, uint16_t cmd,
> void *buf, size_t len)
I missed it earlier, but you can use libqos/fw_cfg.h for this.
Regards,
Anthony Liguori
> {
> uint8_t *p = buf;
> size_t i;
>
> -writew(cfg_addr, cmd);
> -for (i = 0; i < len; i++) {
> -p[i] = readb(cfg_addr + 2);
> +if (addr_is_io) {
> +outw(cfg_addr, cmd);
> +for (i = 0; i < len; i++) {
> +p[i] = inb(cfg_addr + 1);
> +}
> +} else {
> +writew(cfg_addr, cmd);
> +for (i = 0; i < len; i++) {
> +p[i] = readb(cfg_addr + 2);
> +}
> }
> }
>
> -static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, uint16_t cmd)
> +static uint16_t read_fw_cfg_i16(uint64_t cfg_addr, bool addr_is_io,
> +uint16_t cmd)
> {
> uint16_t value;
>
> -read_fw_cfg(cfg_addr, cmd, &value, sizeof(value));
> +read_fw_cfg(cfg_addr, addr_is_io, cmd, &value, sizeof(value));
> return le16_to_cpu(value);
> }
>
> @@ -157,7 +165,7 @@ static uint16_t read_fw_cfg_i16(uint64_t cfg_addr,
> uint16_t cmd)
>
> static uint64_t read_boot_order_pmac(void)
> {
> -return read_fw_cfg_i16(PMAC_CFG_ADDR, FW_CFG_BOOT_DEVICE);
> +return read_fw_cfg_i16(PMAC_CFG_ADDR, false, FW_CFG_BOOT_DEVICE);
> }
>
> static const boot_order_test test_cases_fw_cfg[] = {
> @@ -182,7 +190,7 @@ static void test_pmac_newworld_boot_order(void)
>
> static uint64_t read_boot_order_sun4m(void)
> {
> -return read_fw_cfg_i16(SUN4M_CFG_ADDR, FW_CFG_BOOT_DEVICE);
> +return read_fw_cfg_i16(SUN4M_CFG_ADDR, false, FW_CFG_BOOT_DEVICE);
> }
>
> static void test_sun4m_boot_order(void)
> --
> 1.7.11.7
Re: [Qemu-devel] [PATCH v3 11/16] boot-order-test: Better separate target-specific and generic parts
Markus Armbruster writes:
> The initial version did just PC. I didn't bother to separate out
> generic parts, because I don't like to abstract from a single case.
>
> Now we have two cases, PC and PowerMac, and I'm about to add two more.
> Time to do it right.
>
> To ease review, this commit changes the code without in-place, and
> only the next commit reorders it for better readability.
>
> Cc: Andreas Färber
> Cc: Alexander Graf
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster
> ---
> tests/boot-order-test.c | 160
> ++--
> 1 file changed, 99 insertions(+), 61 deletions(-)
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index f6dece6..23edacf 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -15,106 +15,141 @@
> #include "libqtest.h"
> #include "qemu/bswap.h"
>
> -static void test_pc_cmos_byte(int reg, int expected)
> +static uint8_t read_mc146818(uint16_t port, uint8_t reg)
> {
> -int actual;
> -
> -outb(0x70, reg);
> -actual = inb(0x71);
> -g_assert_cmphex(actual, ==, expected);
> +outb(port, reg);
> +return inb(port + 1);
> }
It's worth looking at the rtc-test case and pulling out
cmos_read/write() into libqos I suspect.
Regards,
Anthony Liguori
>
> -static void test_pc_cmos(uint8_t boot1, uint8_t boot2)
> +static uint64_t read_boot_order_pc(void)
> {
> -test_pc_cmos_byte(0x38, boot1);
> -test_pc_cmos_byte(0x3d, boot2);
> +uint8_t b1 = read_mc146818(0x70, 0x38);
> +uint8_t b2 = read_mc146818(0x70, 0x3d);
> +
> +return b1 | (b2 << 8);
> }
>
> -static void test_pc_with_args(const char *test_args,
> - uint8_t boot1, uint8_t boot2,
> - uint8_t reboot1, uint8_t reboot2)
> +static void test_a_boot_order(const char *machine,
> + const char *test_args,
> + uint64_t (*read_boot_order)(void),
> + uint64_t expected_boot,
> + uint64_t expected_reboot)
> {
> -char *args = g_strdup_printf("-nodefaults -display none %s", test_args);
> +char *args;
> +uint64_t actual;
>
> +args = g_strdup_printf("-nodefaults -display none%s%s %s",
> + machine ? " -M " : "",
> + machine ?: "",
> + test_args);
> qtest_start(args);
> -test_pc_cmos(boot1, boot2);
> +actual = read_boot_order();
> +g_assert_cmphex(actual, ==, expected_boot);
> qmp("{ 'execute': 'system_reset' }");
> -test_pc_cmos(reboot1, reboot2);
> +actual = read_boot_order();
> +g_assert_cmphex(actual, ==, expected_reboot);
> qtest_quit(global_qtest);
> g_free(args);
> }
>
> +typedef struct {
> +const char *args;
> +uint64_t expected_boot;
> +uint64_t expected_reboot;
> +} boot_order_test;
> +
> +static void test_boot_orders(const char *machine,
> + uint64_t (*read_boot_order)(void),
> + const boot_order_test *tests)
> +{
> +int i;
> +
> +for (i = 0; tests[i].args; i++) {
> +test_a_boot_order(machine, tests[i].args,
> + read_boot_order,
> + tests[i].expected_boot,
> + tests[i].expected_reboot);
> +}
> +}
> +
> +static const boot_order_test test_cases_pc[] = {
> +{ "",
> + 0x1230, 0x1230 },
> +{ "-no-fd-bootchk",
> + 0x1231, 0x1231 },
> +{ "-boot c",
> + 0x0200, 0x0200 },
> +{ "-boot nda",
> + 0x3410, 0x3410 },
> +{ "-boot order=",
> + 0, 0 },
> +{ "-boot order= -boot order=c",
> + 0x0200, 0x0200 },
> +{ "-boot once=a",
> + 0x0100, 0x1230 },
> +{ "-boot once=a -no-fd-bootchk",
> + 0x0101, 0x1231 },
> +{ "-boot once=a,order=c",
> + 0x0100, 0x0200 },
> +{ "-boot once=d -boot order=nda",
> + 0x0300, 0x3410 },
> +{ "-boot once=a -boot once=b -boot once=c",
> + 0x0200, 0x1230 },
> +{}
> +};
> +
> static void test_pc_boot_order(void)
> {
> -test_pc_with_args("", 0x30, 0x12, 0x30, 0x12);
> -test_pc_with_args("-no-fd-bootchk", 0x31, 0x12, 0x31, 0x12);
> -test_pc_with_args("-boot c", 0, 0x02, 0, 0x02);
> -test_pc_with_args("-boot nda", 0x10, 0x34, 0x10, 0x34);
> -test_pc_with_args("-boot order=", 0, 0, 0, 0);
> -test_pc_with_args("-boot order= -boot order=c", 0, 0x02, 0, 0x02);
> -test_pc_with_args("-boot once=a", 0, 0x01, 0x30, 0x12);
> -test_pc_with_args("-boot once=a -no-fd-bootchk", 0x01, 0x01, 0x31, 0x12);
> -test_pc_with_args("-boot once=a,order=c", 0, 0x01, 0, 0x02);
> -test_pc_with_args("-boot once=d -boot order=nda", 0, 0x03, 0x10, 0x34);
> -test_pc_with_args("-boot once=a -boot once=b -boot once=c",
> - 0, 0x02, 0x30, 0x12);
> +test_boot_orders(NULL, read_boot
Re: [Qemu-devel] [PATCH] Remove hardcoded xen-platform device initialization
Il 14/06/2013 04:50, Paul Durrant ha scritto: >> > This patch is problematic because we can't know for sure the version of >> > upstream QEMU that is going to be used with Xen. >> > If we apply this patch and QEMU 1.5 is going to be used with Xen 4.2, >> > guests won't be able to use PV drivers. >> > > Is there not a compatibility matrix? The hardcoded init. is just blatantly > the wrong thing to be doing and it needs to go. > > Could my accompanying toolstack patch not be backported to the next 4.2 > release as mitigation? I think Ian is right. You should revert the toolstack patch and not apply this one for now. Then aim at using "-M pc" in 4.4 (and require 1.6 or 1.7), so that there is no need for a compatibility matrix. Paolo
Re: [Qemu-devel] [Xen-devel] [PATCH] Remove hardcoded xen-platform device initialization
Il 14/06/2013 06:38, Paul Durrant ha scritto: I think the right solution for this is to move towards using the normal "-M pc" machine. libxl can simply use "-M pc -machine accel=xen -device xen-platform-pv"; older versions that use the xenfv machine will still work. And if you do this, you will also get the benefit of versioned machine types. >> >> Thanks. I'll have a look at that. > > It's a little more complicated than I thought. There are two machine > types, xenpv and xenfv (i.e. HVM), and they share the accel=xen > option. Yes, I was talking of xenfv only. > Thus QEMU attaches to the VM in the machine code rather than > the accelerator init code. Using -M pc is therefore not an option, > unless we perhaps have separate accel options. You're talking about xen_hvm_init, right? IIRC there is a hypercall that lets you know if a domain is PV or FV so you could move large parts of it to accelerator init. What's left can be done in "if (xen_enabled())" (especially those parts that have matching TCG/KVM code in normal "-M pc" initialization, and have that code currently disabled for Xen: unifying the two or at least doing an if/else would be nicer). > Either way this > doesn't address the backwards compatibility issue. I think QEMU is > going to need to know which version of the Xen toolstack invoked it > unless we specify a compatibility matrix. Yes, "xenfv" needs to stay as legacy for compatibility purposes. But if you move the toolchain and QEMU towards using "-M pc" (requiring new QEMU for new toolchains that do) it would be a very nice cleanup. It is also needed if you ever want to support Q35/PCIe. Paolo
Re: [Qemu-devel] [PATCH v3 10/16] boot-order-test: Cover -boot once in ppc tests
Markus Armbruster writes: > Cc: Andreas Färber > Cc: Alexander Graf > Cc: qemu-...@nongnu.org > Signed-off-by: Markus Armbruster Reviewed-by: Anthony Liguori Regards, Anthony Liguori > --- > tests/boot-order-test.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c > index c1cc2a6..f6dece6 100644 > --- a/tests/boot-order-test.c > +++ b/tests/boot-order-test.c > @@ -113,6 +113,7 @@ static void test_powermac_boot_order(void) > test_powermac_with_args(newworld, "", 'c', 'c'); > test_powermac_with_args(newworld, "-boot c", 'c', 'c'); > test_powermac_with_args(newworld, "-boot d", 'd', 'd'); > +test_powermac_with_args(newworld, "-boot once=d,order=c", 'd', 'c'); > } > } > > -- > 1.7.11.7
Re: [Qemu-devel] [PATCH v3 09/16] boot-order-test: Add tests for PowerMacs
Markus Armbruster writes:
> From: Andreas Färber
>
> They set the boot device via fw_cfg, which is then translated to a boot
> path of "hd" or "cd" in OpenBIOS.
>
> Signed-off-by: Andreas Färber
> Cc: Alexander Graf
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster
> ---
> tests/Makefile | 2 ++
> tests/boot-order-test.c | 66
> -
> 2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 394e029..9653fce 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -67,6 +67,8 @@ gcov-files-sparc-y += hw/m48t59.c
> gcov-files-sparc64-y += hw/m48t59.c
> check-qtest-arm-y = tests/tmp105-test$(EXESUF)
> gcov-files-arm-y += hw/tmp105.c
> +check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
> +check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
>
> GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h
> tests/test-qmp-commands.h
>
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> index 2215710..c1cc2a6 100644
> --- a/tests/boot-order-test.c
> +++ b/tests/boot-order-test.c
> @@ -10,8 +10,10 @@
> * See the COPYING file in the top-level directory.
> */
>
> +#include
> #include
> #include "libqtest.h"
> +#include "qemu/bswap.h"
>
> static void test_pc_cmos_byte(int reg, int expected)
> {
> @@ -58,11 +60,73 @@ static void test_pc_boot_order(void)
>0, 0x02, 0x30, 0x12);
> }
>
> +#define G3BEIGE_CFG_ADDR 0xf510
> +#define MAC99_CFG_ADDR 0xf510
> +
> +#define NO_QEMU_PROTOS
> +#include "hw/nvram/fw_cfg.h"
> +#undef NO_QEMU_PROTOS
> +
> +static void powermac_fw_cfg_read(bool newworld, uint16_t cmd,
> + uint8_t *buf, unsigned int len)
> +{
> +unsigned int i;
> +
> +writew(newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR, cmd);
> +for (i = 0; i < len; i++) {
> +buf[i] = readb((newworld ? MAC99_CFG_ADDR : G3BEIGE_CFG_ADDR) + 2);
> +}
> +}
> +
> +static uint16_t powermac_fw_cfg_read16(bool newworld, uint16_t cmd)
> +{
> +uint16_t value;
> +
> +powermac_fw_cfg_read(newworld, cmd, (uint8_t *)&value, sizeof(value));
> +return le16_to_cpu(value);
> +}
> +
> +static void test_powermac_with_args(bool newworld, const char *extra_args,
> +uint16_t expected_boot,
> +uint16_t expected_reboot)
> +{
> +char *args = g_strdup_printf("-nodefaults -display none -machine %s %s",
> + newworld ? "mac99" : "g3beige", extra_args);
> +uint16_t actual;
> +qtest_start(args);
> +actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE);
> +g_assert_cmphex(actual, ==, expected_boot);
> +qmp("{ 'execute': 'system_reset' }");
Same concern here but otherwise looks good.
Regards,
Anthony Liguori
> +actual = powermac_fw_cfg_read16(newworld, FW_CFG_BOOT_DEVICE);
> +g_assert_cmphex(actual, ==, expected_reboot);
> +qtest_quit(global_qtest);
> +g_free(args);
> +}
> +
> +static void test_powermac_boot_order(void)
> +{
> +int i;
> +
> +for (i = 0; i < 2; i++) {
> +bool newworld = (i == 1);
> +
> +test_powermac_with_args(newworld, "", 'c', 'c');
> +test_powermac_with_args(newworld, "-boot c", 'c', 'c');
> +test_powermac_with_args(newworld, "-boot d", 'd', 'd');
> +}
> +}
> +
> int main(int argc, char *argv[])
> {
> +const char *arch = qtest_get_arch();
> +
> g_test_init(&argc, &argv, NULL);
>
> -qtest_add_func("boot-order/pc", test_pc_boot_order);
> +if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +qtest_add_func("boot-order/pc", test_pc_boot_order);
> +} else if (strcmp(arch, "ppc") == 0 || strcmp(arch, "ppc64") == 0) {
> +qtest_add_func("boot-order/powermac", test_powermac_boot_order);
> +}
>
> return g_test_run();
> }
> --
> 1.7.11.7
Re: [Qemu-devel] [PATCH v3 08/16] boot-order-test: New; covering just PC for now
Markus Armbruster writes:
> Signed-off-by: Markus Armbruster
> ---
> tests/Makefile | 2 ++
> tests/boot-order-test.c | 68
> +
> 2 files changed, 70 insertions(+)
> create mode 100644 tests/boot-order-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index c107489..394e029 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -54,6 +54,7 @@ gcov-files-i386-y = hw/fdc.c
> check-qtest-i386-y += tests/ide-test$(EXESUF)
> check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
> gcov-files-i386-y += hw/hd-geometry.c
> +check-qtest-i386-y += tests/boot-order-test$(EXESUF)
> check-qtest-i386-y += tests/rtc-test$(EXESUF)
> check-qtest-i386-y += tests/i440fx-test$(EXESUF)
> check-qtest-i386-y += tests/fw_cfg-test$(EXESUF)
> @@ -130,6 +131,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o
> tests/fdc-test$(EXESUF): tests/fdc-test.o
> tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
> tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
> +tests/boot-order-test$(EXESUF): tests/boot-order-test.o
> tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(libqos-omap-obj-y)
> tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
> tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
> diff --git a/tests/boot-order-test.c b/tests/boot-order-test.c
> new file mode 100644
> index 000..2215710
> --- /dev/null
> +++ b/tests/boot-order-test.c
> @@ -0,0 +1,68 @@
> +/*
> + * Boot order test cases.
> + *
> + * Copyright (c) 2013 Red Hat Inc.
> + *
> + * Authors:
> + * Markus Armbruster ,
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include
> +#include "libqtest.h"
> +
> +static void test_pc_cmos_byte(int reg, int expected)
> +{
> +int actual;
> +
> +outb(0x70, reg);
> +actual = inb(0x71);
> +g_assert_cmphex(actual, ==, expected);
> +}
> +
> +static void test_pc_cmos(uint8_t boot1, uint8_t boot2)
> +{
> +test_pc_cmos_byte(0x38, boot1);
> +test_pc_cmos_byte(0x3d, boot2);
> +}
> +
> +static void test_pc_with_args(const char *test_args,
> + uint8_t boot1, uint8_t boot2,
> + uint8_t reboot1, uint8_t reboot2)
> +{
> +char *args = g_strdup_printf("-nodefaults -display none %s", test_args);
> +
> +qtest_start(args);
> +test_pc_cmos(boot1, boot2);
> +qmp("{ 'execute': 'system_reset' }");
I think this races. I'd suggest doing a tight loop of this test and
running it a few thousand times to see if you can catch it.
qmp_system_reset() calls qemu_system_reset_requested() which stops all
CPUs but let's control fall back to the main loop which actually does
the device reset.
I think there's a tiny window where this command could return while the
reset routines have not been actually called yet.
Technically speaking, I think it's necessary to wait for a reset event
to know that the device model has been reset.
Regards,
Anthony Liguori
> +test_pc_cmos(reboot1, reboot2);
> +qtest_quit(global_qtest);
> +g_free(args);
> +}
> +
> +static void test_pc_boot_order(void)
> +{
> +test_pc_with_args("", 0x30, 0x12, 0x30, 0x12);
> +test_pc_with_args("-no-fd-bootchk", 0x31, 0x12, 0x31, 0x12);
> +test_pc_with_args("-boot c", 0, 0x02, 0, 0x02);
> +test_pc_with_args("-boot nda", 0x10, 0x34, 0x10, 0x34);
> +test_pc_with_args("-boot order=", 0, 0, 0, 0);
> +test_pc_with_args("-boot order= -boot order=c", 0, 0x02, 0, 0x02);
> +test_pc_with_args("-boot once=a", 0, 0x01, 0x30, 0x12);
> +test_pc_with_args("-boot once=a -no-fd-bootchk", 0x01, 0x01, 0x31, 0x12);
> +test_pc_with_args("-boot once=a,order=c", 0, 0x01, 0, 0x02);
> +test_pc_with_args("-boot once=d -boot order=nda", 0, 0x03, 0x10, 0x34);
> +test_pc_with_args("-boot once=a -boot once=b -boot once=c",
> + 0, 0x02, 0x30, 0x12);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +g_test_init(&argc, &argv, NULL);
> +
> +qtest_add_func("boot-order/pc", test_pc_boot_order);
> +
> +return g_test_run();
> +}
> --
> 1.7.11.7
Re: [Qemu-devel] [PATCH v3 07/16] qtest: Don't reset on qtest chardev connect
Markus Armbruster writes:
> libqtest's qtest_init() connecting to the qtest socket triggers reset.
> This was coded in the hope we could use the same QEMU process for
> multiple tests that way. Never used. Injects an extra reset even
> when it's not used, and that can mess up tests such as the one of
> -boot once I'm about to add. Drop it.
>
> Signed-off-by: Markus Armbruster
We could always add a reset qtest command. Probably makes more sense really.
Reviewed-by: Anthony Liguori
Regards,
Anthony Liguori
> ---
> qtest.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/qtest.c b/qtest.c
> index 07a9612..74f1842 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -472,7 +472,12 @@ static void qtest_event(void *opaque, int event)
>
> switch (event) {
> case CHR_EVENT_OPENED:
> -qemu_system_reset(false);
> +/*
> + * We used to call qemu_system_reset() here, hoping we could
> + * use the same process for multiple tests that way. Never
> + * used. Injects an extra reset even when it's not used, and
> + * that can mess up tests, e.g. -boot once.
> + */
> for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
> irq_levels[i] = 0;
> }
> --
> 1.7.11.7
