diff mbox

linux-user: Exclude more cpu model code when building s390x linux-user

Message ID 20170302193300.21188-1-brogers@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bruce Rogers March 2, 2017, 7:33 p.m. UTC
Currently qemu-s390x segfaults. Avoid cpu model code which doesn't
apply in linux user case.
---
 target/s390x/cpu_models.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

David Hildenbrand March 2, 2017, 7:50 p.m. UTC | #1
Am 02.03.2017 um 20:33 schrieb Bruce Rogers:
> Currently qemu-s390x segfaults. Avoid cpu model code which doesn't
> apply in linux user case.

That was also my impression, but it does apply. However, for now it
doesn't matter, as also tcg isn't properly wired up.

In general, linux-user can support _at least_ the same features as tcg,
in theory even more (all that only apply to kernel space).

We had a proper fix on this list by Richard Henderson, wonder what
happened to that?

This file should contain no CONFIG_USER_ONLY apart from the QMP
interface stuff.

> ---
>  target/s390x/cpu_models.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
> index 2a894eec65..97e4e75f68 100644
> --- a/target/s390x/cpu_models.c
> +++ b/target/s390x/cpu_models.c
> @@ -590,7 +590,6 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
>      cpu_info_from_model(baseline_info->model, &model, true);
>      return baseline_info;
>  }
> -#endif
>  
>  static void check_consistency(const S390CPUModel *model)
>  {
> @@ -660,7 +659,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,9 +678,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>          cached = true;
>          return &max_model;
>      }
> -#endif
>      return NULL;
>  }
> +#endif
>  
>  static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
>  {
> @@ -718,6 +716,7 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
>  
>  void s390_realize_cpu_model(CPUState *cs, Error **errp)
>  {
> +#ifndef CONFIG_USER_ONLY
>      S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
>      S390CPU *cpu = S390_CPU(cs);
>      const S390CPUModel *max_model;
> @@ -751,6 +750,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
>      }
>  
>      apply_cpu_model(cpu->model, errp);
> +#endif
>  }
>  
>  static void get_feature(Object *obj, Visitor *v, const char *name,
>
David Hildenbrand March 2, 2017, 7:53 p.m. UTC | #2
Am 02.03.2017 um 20:50 schrieb David Hildenbrand:
> Am 02.03.2017 um 20:33 schrieb Bruce Rogers:
>> Currently qemu-s390x segfaults. Avoid cpu model code which doesn't
>> apply in linux user case.
> 
> That was also my impression, but it does apply. However, for now it
> doesn't matter, as also tcg isn't properly wired up.
> 
> In general, linux-user can support _at least_ the same features as tcg,
> in theory even more (all that only apply to kernel space).
> 
> We had a proper fix on this list by Richard Henderson, wonder what
> happened to that?

Actually it was Stefan Weil :) cc-ing him.

> 
> This file should contain no CONFIG_USER_ONLY apart from the QMP
> interface stuff.
> 
>> ---
>>  target/s390x/cpu_models.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>> index 2a894eec65..97e4e75f68 100644
>> --- a/target/s390x/cpu_models.c
>> +++ b/target/s390x/cpu_models.c
>> @@ -590,7 +590,6 @@ CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
>>      cpu_info_from_model(baseline_info->model, &model, true);
>>      return baseline_info;
>>  }
>> -#endif
>>  
>>  static void check_consistency(const S390CPUModel *model)
>>  {
>> @@ -660,7 +659,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,9 +678,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>          cached = true;
>>          return &max_model;
>>      }
>> -#endif
>>      return NULL;
>>  }
>> +#endif
>>  
>>  static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
>>  {
>> @@ -718,6 +716,7 @@ static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
>>  
>>  void s390_realize_cpu_model(CPUState *cs, Error **errp)
>>  {
>> +#ifndef CONFIG_USER_ONLY
>>      S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
>>      S390CPU *cpu = S390_CPU(cs);
>>      const S390CPUModel *max_model;
>> @@ -751,6 +750,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
>>      }
>>  
>>      apply_cpu_model(cpu->model, errp);
>> +#endif
>>  }
>>  
>>  static void get_feature(Object *obj, Visitor *v, const char *name,
>>
> 
>
Bruce Rogers March 2, 2017, 7:59 p.m. UTC | #3
>>> On 3/2/2017 at 12:53 PM, <david@redhat.com> wrote:
> Am 02.03.2017 um 20:50 schrieb David Hildenbrand:
>> Am 02.03.2017 um 20:33 schrieb Bruce Rogers:
>>> Currently qemu-s390x segfaults. Avoid cpu model code which doesn't
>>> apply in linux user case.
>> 
>> That was also my impression, but it does apply. However, for now it
>> doesn't matter, as also tcg isn't properly wired up.
>> 
>> In general, linux-user can support _at least_ the same features as tcg,
>> in theory even more (all that only apply to kernel space).
>> 
>> We had a proper fix on this list by Richard Henderson, wonder what
>> happened to that?
> 
> Actually it was Stefan Weil :) cc-ing him.

I'm not all so familiar with this code, I only saw a failure and fixed it the
most obvious way for me. I'm happy if there is a better fix out there.

BTW: I resent the patch again with SOB.

Brue

> 
>> 
>> This file should contain no CONFIG_USER_ONLY apart from the QMP
>> interface stuff.
>> 
>>> ---
>>>  target/s390x/cpu_models.c | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
>>> index 2a894eec65..97e4e75f68 100644
>>> --- a/target/s390x/cpu_models.c
>>> +++ b/target/s390x/cpu_models.c
>>> @@ -590,7 +590,6 @@ CpuModelBaselineInfo 
> *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
>>>      cpu_info_from_model(baseline_info->model, &model, true);
>>>      return baseline_info;
>>>  }
>>> -#endif
>>>  
>>>  static void check_consistency(const S390CPUModel *model)
>>>  {
>>> @@ -660,7 +659,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,9 +678,9 @@ static S390CPUModel *get_max_cpu_model(Error **errp)
>>>          cached = true;
>>>          return &max_model;
>>>      }
>>> -#endif
>>>      return NULL;
>>>  }
>>> +#endif
>>>  
>>>  static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
>>>  {
>>> @@ -718,6 +716,7 @@ static inline void apply_cpu_model(const S390CPUModel 
> *model, Error **errp)
>>>  
>>>  void s390_realize_cpu_model(CPUState *cs, Error **errp)
>>>  {
>>> +#ifndef CONFIG_USER_ONLY
>>>      S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
>>>      S390CPU *cpu = S390_CPU(cs);
>>>      const S390CPUModel *max_model;
>>> @@ -751,6 +750,7 @@ void s390_realize_cpu_model(CPUState *cs, Error **errp)
>>>      }
>>>  
>>>      apply_cpu_model(cpu->model, errp);
>>> +#endif
>>>  }
>>>  
>>>  static void get_feature(Object *obj, Visitor *v, const char *name,
>>>
>> 
>> 
> 
> 
> -- 
> Thanks,
> 
> David
David Hildenbrand March 2, 2017, 8:03 p.m. UTC | #4
Am 02.03.2017 um 20:59 schrieb Bruce Rogers:
>>>> On 3/2/2017 at 12:53 PM, <david@redhat.com> wrote:
>> Am 02.03.2017 um 20:50 schrieb David Hildenbrand:
>>> Am 02.03.2017 um 20:33 schrieb Bruce Rogers:
>>>> Currently qemu-s390x segfaults. Avoid cpu model code which doesn't
>>>> apply in linux user case.
>>>
>>> That was also my impression, but it does apply. However, for now it
>>> doesn't matter, as also tcg isn't properly wired up.
>>>
>>> In general, linux-user can support _at least_ the same features as tcg,
>>> in theory even more (all that only apply to kernel space).
>>>
>>> We had a proper fix on this list by Richard Henderson, wonder what
>>> happened to that?
>>
>> Actually it was Stefan Weil :) cc-ing him.
> 
> I'm not all so familiar with this code, I only saw a failure and fixed it the
> most obvious way for me. I'm happy if there is a better fix out there.

Thanks for taking care, seems like the other patch somehow got stuck, so
thanks for bringing that up! :)

In my original series (when I thought linux-user wouldn't care about CPU
models), your patch would have been the correct fix!

> 
> BTW: I resent the patch again with SOB.
> 
> Brue
>
diff mbox

Patch

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2a894eec65..97e4e75f68 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -590,7 +590,6 @@  CpuModelBaselineInfo *arch_query_cpu_model_baseline(CpuModelInfo *infoa,
     cpu_info_from_model(baseline_info->model, &model, true);
     return baseline_info;
 }
-#endif
 
 static void check_consistency(const S390CPUModel *model)
 {
@@ -660,7 +659,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,9 +678,9 @@  static S390CPUModel *get_max_cpu_model(Error **errp)
         cached = true;
         return &max_model;
     }
-#endif
     return NULL;
 }
+#endif
 
 static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
 {
@@ -718,6 +716,7 @@  static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
 
 void s390_realize_cpu_model(CPUState *cs, Error **errp)
 {
+#ifndef CONFIG_USER_ONLY
     S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
     S390CPU *cpu = S390_CPU(cs);
     const S390CPUModel *max_model;
@@ -751,6 +750,7 @@  void s390_realize_cpu_model(CPUState *cs, Error **errp)
     }
 
     apply_cpu_model(cpu->model, errp);
+#endif
 }
 
 static void get_feature(Object *obj, Visitor *v, const char *name,