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.
Liks this:
https://patchew.org/QEMU/20220108091419.2027710-1-yangxiaoj...@loongson.cn/20220108091419.2027710-12-yangxiaoj...@loongson.cn/
static bool loongarch_cpu_has_work(CPUState *cs)
{
+#ifdef CONFIG_USER_ONLY
return true;
+#else
+ LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+ CPULoongArchState *env = &cpu->env;
+ bool has_work = false;
+
+ if ((cs->interrupt_request & CPU_INTERRUPT_HARD) &&
+ cpu_loongarch_hw_interrupts_pending(env)) {
+ has_work = true;
+ }
+
+ return has_work;
+#endif
}
Prefer positive tests over negative tests, so
#ifdef CONFIG_USER_ONLY
return true;
#else
#error
#endif
+ data = FIELD_DP32(data, CPUCFG2, LSPW, 1);
Do you support the SPW extension in this series? If not you probably
don't want to set this bit.
Correct, you can't expose features that you don't implement.
Accept this suggesstions.
Thanks
Song
