Re: [PATCH v7 02/21] target/loongarch: Add core definition

From: WANG Xuerui <i.qemu@xen0n.name>
To: "Song Gao" <gaosong@loongson.cn>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, thuth@redhat.com,
	richard.henderson@linaro.org, yangxiaojuan@loongson.cn,
	peterx@redhat.com, laurent@vivier.eu, alistair.francis@wdc.com,
	maobibo@loongson.cn, pbonzini@redhat.com, bmeng.cn@gmail.com,
	alex.bennee@linaro.org, chenhuacai@loongson.cn
Subject: Re: [PATCH v7 02/21] target/loongarch: Add core definition
Date: Wed, 20 Oct 2021 20:00:41 +0800	[thread overview]
Message-ID: <74bbac65-2cd4-dac7-86d1-e18cfa3f3f1f@xen0n.name> (raw)
In-Reply-To: <aa983155-85ed-f3e3-47b2-b7138125e8d0@loongson.cn>

On 2021/10/20 16:54, Song Gao wrote:

> On 10/19/2021 01:38 AM, Philippe Mathieu-Daudé wrote:
>> On 10/18/21 18:06, WANG Xuerui wrote:
>>
>> On 10/18/21 20:47, Song Gao wrote:
>>>> +static void set_loongarch_cpucfg(CPULoongArchState *env)
>>>> +{
>>>> +    int i;
>>>> +
>>>> +    for (i = 0; i < 49; i++) {
>>>> +        env->cpucfg[i] = 0x0;
>>>> +    }
>>>> +    env->cpucfg[0] = 0x14c010;
>>>> +    env->cpucfg[1] = 0x3f2f2fe;
>>>> +    env->cpucfg[2] = 0x60c3cf;
>>>> +    env->cpucfg[3] = 0xcff;
>>>> +    env->cpucfg[4] = 0x5f5e100;
>>>> +    env->cpucfg[5] = 0x10001;
>>>> +    env->cpucfg[16] = 0x2c3d;
>>>> +    env->cpucfg[17] = 0x6080003;
>>>> +    env->cpucfg[18] = 0x6080003;
>>>> +    env->cpucfg[19] = 0x60800f;
>>>> +    env->cpucfg[20] = 0x60f000f;
>>> I know these values are taken from a real 3A5000 chip, since I have such
>>> a machine and I've done the experiment, but others likely wouldn't
>>> notice so quickly. Maybe put some comment on top of this function to
>>> illustrate this?
>> Simpler: ...
>>
> On linux-user emulation. We don't care about the value of cpucfg[i]. 
> I think we only need to set cpucfg[i] to 0. and set value on system emulations, is that better? 

I'm afraid that wouldn't be better; actually it would be *wrong* to just
return zeroes for user-space CPUCFG accesses. CPUCFG is designed to
provide runtime feature probing like CPUID, and is usable from
user-space. So, one can only assume this data is being used, and
properly return data.

I've heard that kernel could choose to not actually enable all features
for which CPUCFG indicate support, while not configuring CPUCFG values
to reflect that, thus making CPUCFG values unreliable; that's not a
proper reason to return zeroes. Kernel should be fixed to properly
configure CPUCFG instead. Because otherwise you wouldn't make such an
instruction an unprivileged one in the first place...