From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58098) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRz42-0005KK-Uq for qemu-devel@nongnu.org; Tue, 26 Jul 2016 05:54:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bRz3y-0005y7-2Z for qemu-devel@nongnu.org; Tue, 26 Jul 2016 05:54:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bRz3x-0005xk-Qx for qemu-devel@nongnu.org; Tue, 26 Jul 2016 05:54:10 -0400 Date: Tue, 26 Jul 2016 10:54:05 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20160726095404.GA3609@work-vm> References: <1469119676-32373-1-git-send-email-lvivier@redhat.com> <20160722064311.GU15941@voom.fritz.box> <20160723063031.GZ15941@voom.fritz.box> <01797e6a-de08-4073-debe-fab7e4795b80@redhat.com> <8a0c41cc-ed63-befa-f78e-7319b3689655@redhat.com> <4ab69528-5fe7-1f44-078a-04af2a1985e6@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4ab69528-5fe7-1f44-078a-04af2a1985e6@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: Thomas Huth , David Gibson , dgibson@redhat.com, qemu-devel@nongnu.org * Laurent Vivier (lvivier@redhat.com) wrote: > > > On 26/07/2016 11:39, Laurent Vivier wrote: > > > > > > On 26/07/2016 11:28, Thomas Huth wrote: > >> On 26.07.2016 11:23, Laurent Vivier wrote: > >>> > >>> > >>> On 23/07/2016 08:30, David Gibson wrote: > >>>> On Fri, Jul 22, 2016 at 09:28:58AM +0200, Laurent Vivier wrote: > >>>>> > >>>>> > >>>>> On 22/07/2016 08:43, David Gibson wrote: > >>>>>> On Thu, Jul 21, 2016 at 06:47:56PM +0200, Laurent Vivier wrote: > >>>>>>> As userfaultfd syscall is available on powerpc, migration > >>>>>>> postcopy can be used. > >>>>>>> > >>>>>>> This patch adds the support needed to test this on powerpc, > >>>>>>> instead of using a bootsector to run code to modify memory, > >>>>>>> we use a FORTH script in "boot-command" property. > >>>>>>> > >>>>>>> As spapr machine doesn't support "-prom-env" argument > >>>>>>> (the nvram is initialized by SLOF and not by QEMU), > >>>>>>> "boot-command" is provided to SLOF via a file mapped nvram > >>>>>>> (with "-drive file=...,if=pflash") > >>>>>>> > >>>>>>> Signed-off-by: Laurent Vivier > >>>>>>> --- > >>>>>>> v2: move FORTH script directly in sprintf() > >>>>>>> use openbios_firmware_abi.h > >>>>>>> remove useless "default" case > >>>>>>> > >>>>>>> tests/Makefile.include | 1 + > >>>>>>> tests/postcopy-test.c | 116 +++++++++++++++++++++++++++++++++++++++++-------- > >>>>>>> 2 files changed, 98 insertions(+), 19 deletions(-) > >>>>>> > >>>>>> There's a mostly cosmetic problem with this. If you run make check > >>>>>> for a ppc64 target on an x86 machine, you get: > >>>>>> > >>>>>> GTESTER check-qtest-ppc64 > >>>>>> "kvm" accelerator not found. > >>>>>> "kvm" accelerator not found. > >>>>> > >>>>> I think this is because of "-machine accel=kvm:tcg", it tries to use kvm > >>>>> and fall back to tcg. > >>>>> > >>>>> accel.c: > >>>>> > >>>>> 80 void configure_accelerator(MachineState *ms) > >>>>> 81 { > >>>>> ... > >>>>> 100 acc = accel_find(buf); > >>>>> 101 if (!acc) { > >>>>> 102 fprintf(stderr, "\"%s\" accelerator not found.\n", buf); > >>>>> 103 continue; > >>>>> 104 } > >>>>> > >>>>> We can remove the "-machine" argument to use the default instead (tcg or > >>>>> kvm). > >>>> > >>>> That sounds like a good option for a general test. > >>> > >>> In fact, we can't: we need to add a "-machine accel=XXXX" to our command > >>> line to override the "-machine accel=qtest" provided by the qtest > >>> framework. If we don't override it, the machine doesn't start. > >> > >> Would it work if you'd added some magic with "#ifdef CONFIG_KVM" here? > > > > I think it needs to be dynamic as the same binary test is used on x86 to > > test x86 and ppc64, and vice-versa. I'm going to check if we have > > something like "qtest_get_accel()"... > > Something like that should work: > > --- a/tests/postcopy-test.c > +++ b/tests/postcopy-test.c > @@ -380,12 +380,17 @@ static void test_migrate(void) > tmpfs, bootpath, uri); > } else if (strcmp(arch, "ppc64") == 0) { > init_bootfile_ppc(bootpath); > - cmd_src = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > +#ifdef _ARCH_PPC64 > +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg" > +#else > +#define QEMU_CMD_ACCEL "-machine accel=tcg" > +#endif > + cmd_src = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M" > " -name pcsource,debug-threads=on" > " -serial file:%s/src_serial" > " -drive file=%s,if=pflash,format=raw", > tmpfs, bootpath); > - cmd_dst = g_strdup_printf("-machine accel=kvm:tcg -m 256M" > + cmd_dst = g_strdup_printf(QEMU_CMD_ACCEL " -m 256M" > " -name pcdest,debug-threads=on" > " -serial file:%s/dest_serial" > " -incoming %s", > > Laurent Is it worth the hastle to just get rid of the two warnings? Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK