On 3/28/22 06:57, Xiaojuan Yang wrote:
+static uint64_t loongarch_qemu_read(void *opaque, hwaddr addr, unsigned size) +{ + uint64_t feature = 0UL; + + switch (addr) { + case FEATURE_REG: + feature |= 1UL << IOCSRF_MSI | 1UL << IOCSRF_EXTIOI | + 1UL << IOCSRF_CSRIPI; + return feature ;
What's the point of the feature variable?
+ case VENDOR_REG: + return *(uint64_t *)"Loongson"; + case CPUNAME_REG: + return *(uint64_t *)"3A5000";
This is definitely wrong, as (1) it depends on host endianness, and (2) you're reading 8 bytes from a 7 byte string.
+static const MemoryRegionOps loongarch_qemu_ops = { + .read = loongarch_qemu_read, + .write = loongarch_qemu_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 4, + .max_access_size = 8, + }, + .impl = { + .min_access_size = 4, + .max_access_size = 8, + },
The implementation above doesn't actually support access size 4; it only supports 8. It doesn't seem like this should be a io region at all, but a ROM.
+static void loongarch_cpu_init(LoongArchCPU *la_cpu, int cpu_num) +{ + CPULoongArchState *env; + env = &la_cpu->env; + + memory_region_init_io(&env->system_iocsr, OBJECT(la_cpu), NULL, + env, "iocsr", UINT64_MAX); + address_space_init(&env->address_space_iocsr, &env->system_iocsr, "IOCSR"); + + timer_init_ns(&la_cpu->timer, QEMU_CLOCK_VIRTUAL, + &loongarch_constant_timer_cb, la_cpu);
This timer belongs to the cpu, not the board model. This init belongs over in target/loongarch/. r~