Re: [PATCH v1 2/3] target/loongarch: Allow user enable/disable LSX/LASX features

[gaosong <gaosong@loongson.cn>]

在 2023/10/19 上午7:47, Richard Henderson 写道:
> On 10/18/23 01:59, Song Gao wrote:
>> Some users may not need LSX/LASX, this patch allows the user
>> enable/disable LSX/LASX features.
>>
>>   e.g
>>   '-cpu max,lsx=on,lasx=on'   (default);
>>   '-cpu max,lsx=on,lasx=off'  (enabled LSX);
>>   '-cpu max,lsx=off,lasx=on'  (error, need lsx=on);
>>   '-cpu max,lsx=off'          (disable LSX and LASX).
>
> ...
>
>> +    /* CPU has LSX */
>> +    bool has_lsx;
>> +    /* CPU has  LASX */
>> +    bool has_lasx;
>
> Why do you need these variables?
>
> I suspect that you've copied them from one of the more complex Arm 
> cases where we need to resolve multiple properties simultaneously 
> during realize.
>
> You'll get identical behaviour in your current code if you drop these 
> and rely only on the CPUCFG2 bits.
>
> If you wanted to do something more complex, you could use OnOffAuto, 
> so that you can detect conflicting settings (such as #3 above), but 
> not generate an error for
>
>   -cpu foo,lasx=on
>
Got it, thanks for you suggestion.
> where 'foo' is some cpu model which does *no* default lsx=on.  You 
> would see that has_lsx==AUTO && has_lasx==ON and then set lsx=ON.
>
Some cpu model not support lasx or lsx feature,  we should't allow user 
set lsx=on or lasx=on.

I think we need env->features. set the feature when the cpu model 
support this feature.
If the cpu model support the feature,  we allow user set CPUCFG to 
enable/disable this feature.

Do you have more suggestion?

Thanks.
Song Gao