On 2021/6/26 下午11:20, Frank Chang wrote:
> LIU Zhiwei <zhiwei_liu@c-sky.com <mailto:zhiwei_liu@c-sky.com>> 於 
> 2021年4月9日 週五 下午3:57寫道：
>
>
>     +
>     +/*
>     + * riscv_clic_create:
>     + *
>     + * @addr: base address of M-Mode CLIC memory-mapped registers
>     + * @prv_s: have smode region
>     + * @prv_u: have umode region
>     + * @num_harts: number of CPU harts
>     + * @num_sources: number of interrupts supporting by each aperture
>     + * @clicintctlbits: bits are actually implemented in the
>     clicintctl registers
>     + * @version: clic version, such as "v0.9"
>     + *
>     + * Returns: the device object
>     + */
>     +DeviceState *riscv_clic_create(hwaddr addr, bool prv_s, bool prv_u,
>     +                               uint32_t num_harts, uint32_t
>     num_sources,
>     +                               uint8_t clicintctlbits,
>     +                               const char *version)
>     +{
>     +    DeviceState *dev = qdev_new(TYPE_RISCV_CLIC);
>     +
>     +    assert(num_sources <= 4096);
>     +    assert(num_harts <= 1024);
>     +    assert(clicintctlbits <= 8);
>     +    assert(!strcmp(version, "v0.8") || !strcmp(version, "v0.9"));
>     +
>     +    qdev_prop_set_bit(dev, "prv-s", prv_s);
>     +    qdev_prop_set_bit(dev, "prv-u", prv_u);
>     +    qdev_prop_set_uint32(dev, "num-harts", num_harts);
>     +    qdev_prop_set_uint32(dev, "num-sources", num_sources);
>     +    qdev_prop_set_uint32(dev, "clicintctlbits", clicintctlbits);
>     +    qdev_prop_set_uint64(dev, "mclicbase", addr);
>
>
> According to spec:
>   Since the CLIC memory map must be aligned at a 4KiB boundary,
>   the mclicbase CSR has its 12 least-significant bits hardwired to zero.
>   It is used to inform software about the location of CLIC memory 
> mappped registers.
>
> I think it's better to add another addr check to ensure it's 4KiB aligned.
>
Agree.

Thanks,
Zhiwei

> Thanks,
> Frank Chang
>
>     +    qdev_prop_set_string(dev, "version", version);
>     +
>     +    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
>     +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, addr);
>     +    return dev;
>     +}
>     +
>