| Message ID | 20200203134251.12986-1-lyan@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] target/arm/monitor: query-cpu-model-expansion crashed qemu when using machine type none | expand |
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
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 >
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 "
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(-)