From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41678) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS1xx-0002X4-T6 for qemu-devel@nongnu.org; Tue, 26 Jul 2016 09:00:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bS1xr-0004X7-SA for qemu-devel@nongnu.org; Tue, 26 Jul 2016 09:00:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43408) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bS1xr-0004Wz-K7 for qemu-devel@nongnu.org; Tue, 26 Jul 2016 09:00:03 -0400 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> <2f0f75ea-2566-83e2-0d78-6e0b761ed44c@redhat.com> From: Laurent Vivier Message-ID: Date: Tue, 26 Jul 2016 14:59:59 +0200 MIME-Version: 1.0 In-Reply-To: <2f0f75ea-2566-83e2-0d78-6e0b761ed44c@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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: Thomas Huth , David Gibson Cc: dgibson@redhat.com, qemu-devel@nongnu.org, dgilbert@redhat.com On 26/07/2016 14:53, Laurent Vivier wrote: > > > On 26/07/2016 12:02, Thomas Huth wrote: >> On 26.07.2016 11:53, Laurent Vivier 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 >> >> I think you'd need to test CONFIG_KVM, too, since it could also have >> been disabled on on PPC, couldn't it? > > Sure. > >>> +#define QEMU_CMD_ACCEL "-machine accel=kvm:tcg" >>> +#else >>> +#define QEMU_CMD_ACCEL "-machine accel=tcg" >>> +#endif >> >> Alternatively, what about shutting up the message in accel.c by changing >> it like that: >> >> if (!qtest_enabled()) { >> error_report("\"%s\" accelerator not found.\n", buf); >> } >> > > I've tried that, and we always get the messages in the "make check" output. No, I'm wrong: I didn't add the "qtest_enabled()", only replace the fprintf() by an "error_report()"... it should work. Laurent