qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH] hw/arm/virt: fix cmdline parsing bug with CPU o


From: Ard Biesheuvel
Subject: Re: [Qemu-devel] [PATCH] hw/arm/virt: fix cmdline parsing bug with CPU options and smp > 1
Date: Wed, 4 Mar 2015 07:10:46 +0000

On 3 March 2015 at 19:25, Greg Bellows <address@hidden> wrote:
>
>
> On Tue, Mar 3, 2015 at 11:06 AM, Ard Biesheuvel <address@hidden>
> wrote:
>>
>> The recently introduced feature that allows 32 bit guests to be
>> executed under KVM on a 64-bit host incorrectly handles the case
>> where more than 1 cpu is specified using '-smp N'
>>
>> For instance, this invocation of qemu
>>
>>   qemu-system-aarch64 -M virt -cpu cortex-a57,aarch64=off -smp 2
>>

This is incorrect: it is the command line I used for reproducing the
issue while working on the bug, after removing the KVM check.
So this should read

>>   qemu-system-aarch64 -M virt -enable-kvm -cpu host,aarch64=off -smp 2


>> produces the following error
>>
>>   qemu-system-aarch64: Expected key=value format, found aarch64
>>
>> which is caused by the destructive parsing performed by
>> cpu_common_parse_features(), resulting in subsequent attempts
>> to parse the CPU option string (for each additional CPU) to fail.
>>
>> So duplicate the string before parsing it, and free it directly
>> afterwards.
>>
>> Signed-off-by: Ard Biesheuvel <address@hidden>
>> ---
>>  hw/arm/virt.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 69f51ac0da58..f8a6c46323dc 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -758,6 +758,7 @@ static void machvirt_init(MachineState *machine)
>>          CPUClass *cc = CPU_CLASS(oc);
>>          Object *cpuobj;
>>          Error *err = NULL;
>> +        char *cpuopts = g_strdup(cpustr[1]);
>>
>>          if (!oc) {
>>              fprintf(stderr, "Unable to find CPU definition\n");
>> @@ -766,7 +767,8 @@ static void machvirt_init(MachineState *machine)
>>          cpuobj = object_new(object_class_get_name(oc));
>>
>>          /* Handle any CPU options specified by the user */
>> -        cc->parse_features(CPU(cpuobj), cpustr[1], &err);
>> +        cc->parse_features(CPU(cpuobj), cpuopts, &err);
>> +        g_free(cpuopts);
>>          if (err) {
>>              error_report("%s", error_get_pretty(err));
>>              exit(1);
>> --
>> 1.8.3.2
>>
> Saw your patch after I sent mine out, roughly same fix...
>
> Reviewed-by: Greg Bellows <address@hidden>
>

Thanks.

Let's leave it up to Peter which version he is most happy with

-- 
Ard.



reply via email to

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