[PATCH v4] target/arm/monitor: query-cpu-model-expansion crashed qemu when using machine type none

[PATCH v4] target/arm/monitor: query-cpu-model-expansion crashed qemu when using machine type none

[Liang Yan]

Commit e19afd566781 mentioned that target-arm only supports queryable
cpu models 'max', 'host', and the current type when KVM is in use.
The logic works well until using machine type none.

For machine type none, cpu_type will be null if cpu option is not
set by command line, strlen(cpu_type) will terminate process.
So We add a check above it.

This won't affect i386 and s390x since they do not use current_cpu.

Signed-off-by: Liang Yan <lyan@suse.com>
---
 v4: change code style based on the review from Andrew Jones
 v3: change git commit message
 v2: fix code style issue
---
 target/arm/monitor.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 9725dfff16..c2dc7908de 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -137,17 +137,20 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
     }
 
     if (kvm_enabled()) {
-        const char *cpu_type = current_machine->cpu_type;
-        int len = strlen(cpu_type) - strlen(ARM_CPU_TYPE_SUFFIX);
         bool supported = false;
 
         if (!strcmp(model->name, "host") || !strcmp(model->name, "max")) {
             /* These are kvmarm's recommended cpu types */
             supported = true;
-        } else if (strlen(model->name) == len &&
-                   !strncmp(model->name, cpu_type, len)) {
-            /* KVM is enabled and we're using this type, so it works. */
-            supported = true;
+        } else if (current_machine->cpu_type) {
+            const char *cpu_type = current_machine->cpu_type;
+            int len = strlen(cpu_type) - strlen(ARM_CPU_TYPE_SUFFIX);
+
+            if (strlen(model->name) == len &&
+                !strncmp(model->name, cpu_type, len)) {
+                /* KVM is enabled and we're using this type, so it works. */
+                supported = true;
+            }
         }
         if (!supported) {
             error_setg(errp, "We cannot guarantee the CPU type '%s' works "
-- 
2.25.0

Re: [PATCH v4] target/arm/monitor: query-cpu-model-expansion crashed qemu when using machine type none

[Peter Maydell]

On Mon, 3 Feb 2020 at 13:44, Liang Yan <lyan@suse.com> wrote:
>
> Commit e19afd566781 mentioned that target-arm only supports queryable
> cpu models 'max', 'host', and the current type when KVM is in use.
> The logic works well until using machine type none.
>
> For machine type none, cpu_type will be null if cpu option is not
> set by command line, strlen(cpu_type) will terminate process.
> So We add a check above it.
>
> This won't affect i386 and s390x since they do not use current_cpu.
>
> Signed-off-by: Liang Yan <lyan@suse.com>
> ---
>  v4: change code style based on the review from Andrew Jones
>  v3: change git commit message
>  v2: fix code style issue

If a reviewer says "with these changes, reviewed-by:", or
"otherwise, reviewed-by...", then you should add those tags to
your commit message, assuming you've only made the changes
they asked for. That saves them having to look at and reply
to the patchset again.

In this case I'll just add them as I add this patch to
target-arm.next, but if you could handle tags across versions
for future patchset submissions it makes life a little
easier for us.

Applied to target-arm.next, thanks.

thanks
-- PMM

Re: [PATCH v4] target/arm/monitor: query-cpu-model-expansion crashed qemu when using machine type none

[Liang Yan]

On 2/3/20 8:54 AM, Peter Maydell wrote:
> On Mon, 3 Feb 2020 at 13:44, Liang Yan <lyan@suse.com> wrote:
>>
>> Commit e19afd566781 mentioned that target-arm only supports queryable
>> cpu models 'max', 'host', and the current type when KVM is in use.
>> The logic works well until using machine type none.
>>
>> For machine type none, cpu_type will be null if cpu option is not
>> set by command line, strlen(cpu_type) will terminate process.
>> So We add a check above it.
>>
>> This won't affect i386 and s390x since they do not use current_cpu.
>>
>> Signed-off-by: Liang Yan <lyan@suse.com>
>> ---
>>  v4: change code style based on the review from Andrew Jones
>>  v3: change git commit message
>>  v2: fix code style issue
> 
> If a reviewer says "with these changes, reviewed-by:", or
> "otherwise, reviewed-by...", then you should add those tags to
> your commit message, assuming you've only made the changes
> they asked for. That saves them having to look at and reply
> to the patchset again.
> 
> In this case I'll just add them as I add this patch to
> target-arm.next, but if you could handle tags across versions
> for future patchset submissions it makes life a little
> easier for us.
> 
Thanks for the tips, will definitely do in the future.

Best,
Liang

> Applied to target-arm.next, thanks.
> 
> thanks
> -- PMM
>