[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.