On 2/3/20 8:08 AM, Andrew Jones wrote:
> On Fri, Jan 31, 2020 at 10:46:49PM -0500, Liang Yan wrote:
>> Commit e19afd56 mentioned that target-arm only supports queryable
>
> Please use more hexdigits. I'm not sure QEMU has a policy for that,
> but I'd go with 12.
>
>> 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 <l...@suse.com>
>> ---
>> v3: change git commit message
>> v2: fix code style issue
>> ---
>> target/arm/monitor.c | 14 ++++++++------
>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/target/arm/monitor.c b/target/arm/monitor.c
>> index 9725dfff16..3350cd65d0 100644
>> --- a/target/arm/monitor.c
>> +++ b/target/arm/monitor.c
>> @@ -137,17 +137,19 @@ 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);
>
> Need a blank line here.
>
>> + if (strlen(model->name) == len &&
>> + !strncmp(model->name, cpu_type, len)) {
>
> Four spaces of indent too many on the line above.
>
>> + /* 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
>>
>>
>
> With the three changes above
>
>
> Reviewed-by: Andrew Jones <drjo...@redhat.com>
> Tested-by: Andrew Jones <drjo...@redhat.com>
>
Thanks for the review, I will update soon.
>
> It'd be nice to extend tests/qtest/arm-cpu-features.c to also do
> some checks with machine='none' with and without KVM.
>
Will do it later, thanks for the suggestion.
Best,
Liang
> Thanks,
> drew
>
