On 16.03.2017 15:46, Philippe Mathieu-Daudé wrote: > Hi Stefan, Richard. > > On 03/16/2017 10:28 AM, David Hildenbrand wrote: >> On 02.03.2017 20:56, David Hildenbrand wrote: >>> Am 30.01.2017 um 14:15 schrieb Stefan Weil: >>>> Returning NULL from get_max_cpu_model results in a SIGSEGV runtime error. >>>> >>>> Signed-off-by: Stefan Weil <s...@weilnetz.de> >>>> --- >>>> >>>> v2: Re-sent as v1 was damaged by my mailer. >>>> >>>> This is also broken in Debian. >>>> >>>> In addition, there is no default CPU ("any"), so binfmt and related >>>> actions currently don't work. I hacked my local installation by >>>> duplicating the "qemu" cpu definition for "any", but maybe there is >>>> a better solution. >>> >>> That should then already work. >>> >>>> >>>> Regards >>>> Stefan >>>> >>>> target/s390x/cpu_models.c | 2 -- >>>> 1 file changed, 2 deletions(-) >>>> >>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c >>>> index 2a894ee..6e34763 100644 >>>> --- a/target/s390x/cpu_models.c >>>> +++ b/target/s390x/cpu_models.c >>>> @@ -660,7 +660,6 @@ static void check_compatibility(const S390CPUModel >>>> *max_model, >>>> >>>> static S390CPUModel *get_max_cpu_model(Error **errp) >>>> { >>>> -#ifndef CONFIG_USER_ONLY >>>> static S390CPUModel max_model; >>>> static bool cached; >>>> >>>> @@ -680,7 +679,6 @@ static S390CPUModel *get_max_cpu_model(Error **errp) >>>> cached = true; >>>> return &max_model; >>>> } >>>> -#endif >>>> return NULL; >>>> } >>>> >>>> >>> >>> I thought this was already picked up for stable... >>> But looks like only the "any" model fixup got. >>> >>> So >>> >>> Acked-by: David Hildenbrand <da...@redhat.com> >>> >> >> Ping, can somebody please pick this up? > > I see at least two possible changes: > > the nasty one to catch bug: > > @@ -681,7 +681,7 @@ static S390CPUModel *get_max_cpu_model(Error **errp) > return &max_model; > } > #endif > - return NULL; > + abort(); > } > > and the nicer one to avoid SIGSEGV: > > @@ -734,8 +734,10 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp) > } > > max_model = get_max_cpu_model(errp); > - if (*errp) { > - error_prepend(errp, "CPU models are not available: "); > + if (max_model == NULL) { > + if (*errp) { > + error_prepend(errp, "CPU models are not available: "); > + } else { > + error_setg(errp, "CPU models are not available"); > + } > return; > } >
As already discussed, user mode should behave just like tcg. No special handling. So the original patch here is just fine in my opinion. -- Thanks, David