Re: [PATCH v14 02/26] target/loongarch: Add core definition

[WANG Xuerui]

Hi,

On 1/10/22 20:34, gaosong wrote:

Hi,

On 2022/1/10 上午2:49, Richard Henderson wrote:

+static bool loongarch_cpu_has_work(CPUState *cs)
+{
+    return true;

Note: this is only applicable to CONFIG_USER_ONLY, and needs to be changed in the following commits adding system emulation. To better convey your intention it may be better to use an #ifdef guard, something like this:

#ifndef CONFIG_USER_ONLY
#error System emulation TODO
#else
     return true;
#endif

(I'm not sure if this is okay in QEMU coding style, so please correct me if this isn't the case.)

In my opinion, we don't need to do this. As you pointed out below, SPW shouldn't appear in this series. All CONFIG_USER_ONLY  macors should appear in the system emulation series.

IMO, preemptively including the guard serves as "paving the road" to the full system emulation; in your argument, I feel we're deferring too much information to the context (this patch being inside the "linux-user" series, and getting merged earlier than full system emulation), while context would gradually fade out in the months/years ahead; being precise in writing down what you mean doesn't do any harm.

Additionally, we might have to re-order the patches in order to unblock as much progress as possible, and in that case, this patch and the other patch adding the !CONFIG_USER_ONLY part would probably get squashed into one, so this code has to be touched too.