diff mbox series

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

Message ID 20220210061737.1171-3-frank.chang@sifive.com (mailing list archive)
State New, archived
Headers show
Series Support ACLINT 32/64-bit mtimecmp/mtime read/write accesses | expand

Commit Message

Frank Chang Feb. 10, 2022, 6:17 a.m. UTC 
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>
---
 hw/intc/riscv_aclint.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

Comments

Alistair Francis Feb. 21, 2022, 9:49 p.m. UTC | #1 
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
>
>
Alistair Francis Feb. 21, 2022, 9:56 p.m. UTC | #2 
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>
> ---
>  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;

Should we check the other accesses to make sure the size == 4?

Alistair

> --
> 2.31.1
>
>
diff mbox series

Patch

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;