Re: [RFC PATCH v2 2/3] hw/intc: Support 32/64-bit mtimecmp and mtime accesses in RISC-V ACLINT

[Alistair Francis]

On Thu, Feb 10, 2022 at 4:22 PM <frank.chang@sifive.com> wrote:
>
> From: Frank Chang <frank.chang@sifive.com>
>
> RISC-V privilege spec defines that:
>
> * In RV32, memory-mapped writes to mtimecmp modify only one 32-bit part
>   of the register.
> * For RV64, naturally aligned 64-bit memory accesses to the mtime and
>   mtimecmp registers are additionally supported and are atomic.
>
> It's possible to perform both 32/64-bit read/write accesses to both
> mtimecmp and mtime registers.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/intc/riscv_aclint.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index 3b598d8a7e..e7b103e83a 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -126,9 +126,9 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, 
> hwaddr addr,
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "aclint-mtimer: invalid hartid: %zu", hartid);
>          } else if ((addr & 0x7) == 0) {
> -            /* timecmp_lo */
> +            /* timecmp_lo for RV32/RV64 or timecmp for RV64 */
>              uint64_t timecmp = env->timecmp;
> -            return timecmp & 0xFFFFFFFF;
> +            return (size == 4) ? (timecmp & 0xFFFFFFFF) : timecmp;
>          } else if ((addr & 0x7) == 4) {
>              /* timecmp_hi */
>              uint64_t timecmp = env->timecmp;
> @@ -139,8 +139,9 @@ static uint64_t riscv_aclint_mtimer_read(void *opaque, 
> hwaddr addr,
>              return 0;
>          }
>      } else if (addr == mtimer->time_base) {
> -        /* time_lo */
> -        return cpu_riscv_read_rtc(mtimer->timebase_freq) & 0xFFFFFFFF;
> +        /* time_lo for RV32/RV64 or timecmp for RV64 */
> +        uint64_t rtc = cpu_riscv_read_rtc(mtimer->timebase_freq);
> +        return (size == 4) ? (rtc & 0xFFFFFFFF) : rtc;
>      } else if (addr == mtimer->time_base + 4) {
>          /* time_hi */
>          return (cpu_riscv_read_rtc(mtimer->timebase_freq) >> 32) & 
> 0xFFFFFFFF;
> @@ -167,12 +168,17 @@ static void riscv_aclint_mtimer_write(void *opaque, 
> hwaddr addr,
>              qemu_log_mask(LOG_GUEST_ERROR,
>                            "aclint-mtimer: invalid hartid: %zu", hartid);
>          } else if ((addr & 0x7) == 0) {
> -            /* timecmp_lo */
> -            uint64_t timecmp_hi = env->timecmp >> 32;
> -            riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), hartid,
> -                timecmp_hi << 32 | (value & 0xFFFFFFFF),
> -                mtimer->timebase_freq);
> -            return;
> +            if (size == 4) {
> +                /* timecmp_lo for RV32/RV64 */
> +                uint64_t timecmp_hi = env->timecmp >> 32;
> +                riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> hartid,
> +                    timecmp_hi << 32 | (value & 0xFFFFFFFF),
> +                    mtimer->timebase_freq);
> +            } else {
> +                /* timecmp for RV64 */
> +                riscv_aclint_mtimer_write_timecmp(mtimer, RISCV_CPU(cpu), 
> hartid,
> +                                                  value, 
> mtimer->timebase_freq);
> +            }
>          } else if ((addr & 0x7) == 4) {
>              /* timecmp_hi */
>              uint64_t timecmp_lo = env->timecmp;
> --
> 2.31.1
>
>