qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64


From: Laurent Vivier
Subject: Re: [Qemu-devel] [PATCH v2] test: port postcopy test to ppc64
Date: Tue, 26 Jul 2016 11:58:17 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1


On 26/07/2016 11:54, Dr. David Alan Gilbert wrote:
> * Laurent Vivier (address@hidden) 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 <address@hidden>
>>>>>>>>> ---
>>>>>>>>> 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?

I don't know, it's why I'd like to have the opinion of David.

Laurent



reply via email to

[Prev in Thread] Current Thread [Next in Thread]