Re: [qemu-s390x] [PATCH] s390x/cpumodel: fix segmentation fault when baselining models

[Christian Borntraeger]

On 07/18/2018 10:40 AM, David Hildenbrand wrote:
> On 18.07.2018 10:39, Christian Borntraeger wrote:
>>
>>
>> On 07/18/2018 10:24 AM, David Hildenbrand wrote:
>>> Usually, when baselining two CPU models, whereby one of them has base
>>> CPU features disabled (e.g. z14-base,msa=off), we fallback to an older
>>> model that did not have these features in the base model. We always try to
>>> create a "sane" CPU model (as far as possible), and one part of it is that
>>> removing base features is no good and to be avoided.
>>>
>>> Now, if we disable base features that were part of a z900, we're out of
>>> luck. We won't find a CPU model and QEMU will segfault. This is a
>>> scenario that should never happen in real life, but it can be used to
>>> crash QEMU.
>>>
>>> So let's make something like this:
>>>
>>> { "execute": "query-cpu-model-baseline",
>>>   "arguments" : { "modela": { "name": "z14-base", "props": {"esan3" : 
>>> false}},
>>>                   "modelb": { "name": "z14"}} }
>>>
>>> Produce:
>>>
>>> {"return": {"model": {"name": "z900-base", "props": {"esan3": false}}}}
>>>
>>> Instead of segfaulting.
>>>
>>> This could of course be improved (e.g. to z14-base,esan3=false), however
>>> as this ususally won't happen, let's just avoid crashes.
>>>
>>> Signed-off-by: David Hildenbrand <address@hidden>
>>> ---
>>>  target/s390x/cpu_models.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index cfdbccf46d..13a5d4f095 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -716,6 +716,12 @@ CpuModelBaselineInfo 
>>> *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
>>>  
>>>      model.def = s390_find_cpu_def(cpu_type, max_gen, max_gen_ga,
>>>                                    model.features);
>>> +
>>> +    /* models without early base features (esan3) are bad - fallback to 
>>> z900 */
>>> +    if (!model.def) {
>>> +        model.def = s390_find_cpu_def(0x2064, 7, 1, NULL);
>>> +    }
>>> +
>>
>> Is there a way to not even return z900 but retuning an empty model (e.g. no 
>> model that 
>> matches) ?
> 
> An error would be an alternative.

As N3 means new in zarch (and backported to ESA390 mode), there is no machine 
without N3.
So I would prefer an error.