Re: [PATCH v1 1/1] riscv: Pass RISCVHartArrayState by pointer

[Philippe Mathieu-Daudé]

On 1/16/21 12:00 AM, Alistair Francis wrote:
> We were accidently passing RISCVHartArrayState by value instead of
> pointer. The type is 824 bytes long so let's correct that and pass it by
> pointer instead.
> 
> Fixes: Coverity CID 1438099
> Fixes: Coverity CID 1438100
> Fixes: Coverity CID 1438101
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/riscv/boot.h |  6 +++---
>  hw/riscv/boot.c         |  8 ++++----
>  hw/riscv/sifive_u.c     | 10 +++++-----
>  hw/riscv/spike.c        |  8 ++++----
>  hw/riscv/virt.c         |  8 ++++----
>  5 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 20ff5fe5e5..11a21dd584 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -24,9 +24,9 @@
>  #include "hw/loader.h"
>  #include "hw/riscv/riscv_hart.h"
>  
> -bool riscv_is_32bit(RISCVHartArrayState harts);
> +bool riscv_is_32bit(RISCVHartArrayState *harts);
>  
> -target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts,
> +target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState *harts,
>                                            target_ulong firmware_end_addr);
>  target_ulong riscv_find_and_load_firmware(MachineState *machine,
>                                            const char 
> *default_machine_firmware,
> @@ -42,7 +42,7 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
>  hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
>                           uint64_t kernel_entry, hwaddr *start);
>  uint32_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
> -void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
> harts,
> +void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
> *harts,
>                                 hwaddr saddr,
>                                 hwaddr rom_base, hwaddr rom_size,
>                                 uint64_t kernel_entry,
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 83586aef41..acf77675b2 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -33,14 +33,14 @@
>  
>  #include <libfdt.h>
>  
> -bool riscv_is_32bit(RISCVHartArrayState harts)
> +bool riscv_is_32bit(RISCVHartArrayState *harts)
>  {
> -    RISCVCPU hart = harts.harts[0];
> +    RISCVCPU hart = harts->harts[0];

This doesn't look improved. Maybe you want:

       return riscv_cpu_is_32bit(&harts->harts[0].env);

>  
>      return riscv_cpu_is_32bit(&hart.env);
>  }