Looking at htif_mm_ops[] read/write handlers, we notice they
expect 32-bit values to accumulate into to the 'fromhost' and
'tohost' 64-bit variables. Explicit by setting the .impl
min/max fields.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Notes

1/ these variables belong to HTIFState but are declared statically!

  static uint64_t fromhost_addr, tohost_addr, begin_sig_addr, end_sig_addr;

2/ I believe a 64-bit implementation would simplify the logic.

3/ This is a non-QOM device model!

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/riscv_htif.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 3f84d8d6738..db69b5e3ca7 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -325,6 +325,10 @@ static const MemoryRegionOps htif_mm_ops = {
     .read = htif_mm_read,
     .write = htif_mm_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
 };
 
 HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
-- 
2.45.2

On Sat, Nov 30, 2024 at 12:45 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Looking at htif_mm_ops[] read/write handlers, we notice they
> expect 32-bit values to accumulate into to the 'fromhost' and
> 'tohost' 64-bit variables. Explicit by setting the .impl
> min/max fields.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

Alistair

> ---
> Notes
>
> 1/ these variables belong to HTIFState but are declared statically!
>
>   static uint64_t fromhost_addr, tohost_addr, begin_sig_addr, end_sig_addr;
>
> 2/ I believe a 64-bit implementation would simplify the logic.
>
> 3/ This is a non-QOM device model!
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/char/riscv_htif.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 3f84d8d6738..db69b5e3ca7 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -325,6 +325,10 @@ static const MemoryRegionOps htif_mm_ops = {
>      .read = htif_mm_read,
>      .write = htif_mm_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
>  };
>
>  HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
> --
> 2.45.2
>
>

On 11/29/24 12:43 PM, Philippe Mathieu-Daudé wrote:
> Looking at htif_mm_ops[] read/write handlers, we notice they
> expect 32-bit values to accumulate into to the 'fromhost' and
> 'tohost' 64-bit variables. Explicit by setting the .impl
> min/max fields.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Notes
> 
> 1/ these variables belong to HTIFState but are declared statically!
> 
>    static uint64_t fromhost_addr, tohost_addr, begin_sig_addr, end_sig_addr;
> 
> 2/ I believe a 64-bit implementation would simplify the logic.


Perhaps. We would need to check with the protocol first. There might be
a reason why we're using 64 bit read/writes internally while doing 32 bits
r/w in the device.


For now:

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

> 
> 3/ This is a non-QOM device model!
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/char/riscv_htif.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 3f84d8d6738..db69b5e3ca7 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -325,6 +325,10 @@ static const MemoryRegionOps htif_mm_ops = {
>       .read = htif_mm_read,
>       .write = htif_mm_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
>   };
>   
>   HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,