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

2021-01-19 Thread Eric Blake 
On 1/17/21 10:52 AM, Philippe Mathieu-Daudé wrote:
> On 1/16/21 11:38 PM, Alistair Francis wrote:
>> On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé  
>> wrote:
>>>
>>> On 1/16/21 12:00 AM, Alistair Francis wrote:
 We were accidently passing RISCVHartArrayState by value instead of

accidentally

 pointer. The type is 824 bytes long so let's correct that and pass it by
 pointer instead.

 -bool riscv_is_32bit(RISCVHartArrayState harts)
 +bool riscv_is_32bit(RISCVHartArrayState *harts)

Definitely better,

  {
 -RISCVCPU hart = harts.harts[0];
 +RISCVCPU hart = harts->harts[0];

but yeah, this still results in a copy (unless the compiler optimizes it).

>>>
>>> This doesn't look improved. Maybe you want:
>>>
>>>return riscv_cpu_is_32bit(>harts[0].env);

Whereas this is obviously a pointer into the original without relying on
the compiler to elide a copy.

>>
>> I suspect this ends up generating the same code.
> 
> If the compiler is smart enough, but I'm not sure it can figure out
> only 1 element from the structure is accessed...
> My understanding is "first copy the content pointed at '*harts' in
> 'hart' on the stack", then only use "env".
> 
> Cc'ing Eric/Richard to double check.

I agree that relying on the compiler optimization is not as
straightforward as writing the code to directly access the correct
pointer from the get-go.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

2021-01-18 Thread Richard Henderson 
On 1/15/21 1:00 PM, Alistair Francis wrote:
> +bool riscv_is_32bit(RISCVHartArrayState *harts)
>  {
> -RISCVCPU hart = harts.harts[0];
> +RISCVCPU hart = harts->harts[0];

This is a large structure copy as well.


>  return riscv_cpu_is_32bit();
>  }

Better as

  >harts[0].env

surely.


r~

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

2021-01-17 Thread Philippe Mathieu-Daudé 
On 1/16/21 11:38 PM, Alistair Francis wrote:
> On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé  
> wrote:
>>
>> 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 
>>> ---
>>>  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/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 
>>>
>>> -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[0].env);
> 
> I suspect this ends up generating the same code.

If the compiler is smart enough, but I'm not sure it can figure out
only 1 element from the structure is accessed...
My understanding is "first copy the content pointed at '*harts' in
'hart' on the stack", then only use "env".

Cc'ing Eric/Richard to double check.

> 
> Either way, good point I have just squashed this change into the patch.

Thanks,

Phil.

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

2021-01-16 Thread Alistair Francis 
On Sat, Jan 16, 2021 at 2:32 PM Philippe Mathieu-Daudé  wrote:
>
> 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 
> > ---
> >  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 
> >
> > -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[0].env);

I suspect this ends up generating the same code.

Either way, good point I have just squashed this change into the patch.

Alistair

>
> >
> >  return riscv_cpu_is_32bit();
> >  }

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

2021-01-16 Thread 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 
> ---
>  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 
>  
> -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[0].env);

>  
>  return riscv_cpu_is_32bit();
>  }

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

2021-01-16 Thread Alistair Francis 
On Fri, Jan 15, 2021 at 3:00 PM 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 

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  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 
>
> -bool riscv_is_32bit(RISCVHartArrayState harts)
> +bool riscv_is_32bit(RISCVHartArrayState *harts)
>  {
> -RISCVCPU hart = harts.harts[0];
> +RISCVCPU hart = harts->harts[0];
>
>  return riscv_cpu_is_32bit();
>  }
>
> -target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts,
> +target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState *harts,
>target_ulong firmware_end_addr) {
>  if (riscv_is_32bit(harts)) {
>  return QEMU_ALIGN_UP(firmware_end_addr, 4 * MiB);
> @@ -247,7 +247,7 @@ void riscv_rom_copy_firmware_info(MachineState *machine, 
> hwaddr rom_base,
> _space_memory);
>  }
>
> -void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
> harts,
> +void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
> *harts,
> hwaddr start_addr,
> hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index f5c400dd44..d23f349b4e 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -466,7 +466,7 @@ static void sifive_u_machine_init(MachineState *machine)
>
>  /* create device tree */
>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> -   riscv_is_32bit(s->soc.u_cpus));
> +   riscv_is_32bit(>soc.u_cpus));
>
>  if (s->start_in_flash) {
>  /*
> @@ -495,7 +495,7 @@ static void sifive_u_machine_init(MachineState *machine)
>  break;
>  }
>
> -if (riscv_is_32bit(s->soc.u_cpus)) {
> +if (riscv_is_32bit(>soc.u_cpus)) {
>  firmware_end_addr = riscv_find_and_load_firmware(machine,
>  "opensbi-riscv32-generic-fw_dynamic.bin",
>  start_addr, NULL);
> @@ -506,7 +506,7 @@ static void sifive_u_machine_init(MachineState *machine)
>  }
>
>  if (machine->kernel_filename) {
> -kernel_start_addr = riscv_calc_kernel_start_addr(s->soc.u_cpus,
> +kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
>   firmware_end_addr);
>
>  kernel_entry = riscv_load_kernel(machine->kernel_filename,
> @@ -533,7 +533,7 @@ static void sifive_u_machine_init(MachineState *machine)
>  /* Compute the fdt load address in dram */
>  fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
> machine->ram_size, s->fdt);
> -if (!riscv_is_32bit(s->soc.u_cpus)) {
> +if (!riscv_is_32bit(>soc.u_cpus)) {
>  start_addr_hi32 = (uint64_t)start_addr >> 32;
>

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

2021-01-16 Thread Alistair Francis 
On Sat, Jan 16, 2021 at 8:30 AM Bin Meng  wrote:
>
> On Sat, Jan 16, 2021 at 7: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
>
> Where can I look at the Coverity report for QEMU?

I don't think you can. I think there are only a few people who can see
them and they just report them to everyone else.

>
> > Signed-off-by: Alistair Francis 
> > ---
> >  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(-)
> >
>
> Reviewed-by: Bin Meng 

Thanks!

Alistair

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

2021-01-16 Thread Bin Meng 
On Sat, Jan 16, 2021 at 7: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

Where can I look at the Coverity report for QEMU?

> Signed-off-by: Alistair Francis 
> ---
>  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(-)
>

Reviewed-by: Bin Meng

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

2021-01-15 Thread Palmer Dabbelt 

On Fri, 15 Jan 2021 15:00:27 PST (-0800), 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 
---
 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 

-bool riscv_is_32bit(RISCVHartArrayState harts)
+bool riscv_is_32bit(RISCVHartArrayState *harts)
 {
-RISCVCPU hart = harts.harts[0];
+RISCVCPU hart = harts->harts[0];

 return riscv_cpu_is_32bit();
 }

-target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts,
+target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState *harts,
   target_ulong firmware_end_addr) {
 if (riscv_is_32bit(harts)) {
 return QEMU_ALIGN_UP(firmware_end_addr, 4 * MiB);
@@ -247,7 +247,7 @@ void riscv_rom_copy_firmware_info(MachineState *machine, 
hwaddr rom_base,
_space_memory);
 }

-void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
harts,
+void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
*harts,
hwaddr start_addr,
hwaddr rom_base, hwaddr rom_size,
uint64_t kernel_entry,
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index f5c400dd44..d23f349b4e 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -466,7 +466,7 @@ static void sifive_u_machine_init(MachineState *machine)

 /* create device tree */
 create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
-   riscv_is_32bit(s->soc.u_cpus));
+   riscv_is_32bit(>soc.u_cpus));

 if (s->start_in_flash) {
 /*
@@ -495,7 +495,7 @@ static void sifive_u_machine_init(MachineState *machine)
 break;
 }

-if (riscv_is_32bit(s->soc.u_cpus)) {
+if (riscv_is_32bit(>soc.u_cpus)) {
 firmware_end_addr = riscv_find_and_load_firmware(machine,
 "opensbi-riscv32-generic-fw_dynamic.bin",
 start_addr, NULL);
@@ -506,7 +506,7 @@ static void sifive_u_machine_init(MachineState *machine)
 }

 if (machine->kernel_filename) {
-kernel_start_addr = riscv_calc_kernel_start_addr(s->soc.u_cpus,
+kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
  firmware_end_addr);

 kernel_entry = riscv_load_kernel(machine->kernel_filename,
@@ -533,7 +533,7 @@ static void sifive_u_machine_init(MachineState *machine)
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
machine->ram_size, s->fdt);
-if (!riscv_is_32bit(s->soc.u_cpus)) {
+if (!riscv_is_32bit(>soc.u_cpus)) {
 start_addr_hi32 = (uint64_t)start_addr >> 32;
 }

@@ -552,7 +552,7 @@ static void sifive_u_machine_init(MachineState *machine)
 0x,
/* fw_dyn: */
 };
-if (riscv_is_32bit(s->soc.u_cpus)) {
+if (riscv_is_32bit(>soc.u_cpus)) {

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

2021-01-15 Thread Alistair Francis 
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 
---
 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 
 
-bool riscv_is_32bit(RISCVHartArrayState harts)
+bool riscv_is_32bit(RISCVHartArrayState *harts)
 {
-RISCVCPU hart = harts.harts[0];
+RISCVCPU hart = harts->harts[0];
 
 return riscv_cpu_is_32bit();
 }
 
-target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState harts,
+target_ulong riscv_calc_kernel_start_addr(RISCVHartArrayState *harts,
   target_ulong firmware_end_addr) {
 if (riscv_is_32bit(harts)) {
 return QEMU_ALIGN_UP(firmware_end_addr, 4 * MiB);
@@ -247,7 +247,7 @@ void riscv_rom_copy_firmware_info(MachineState *machine, 
hwaddr rom_base,
_space_memory);
 }
 
-void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
harts,
+void riscv_setup_rom_reset_vec(MachineState *machine, RISCVHartArrayState 
*harts,
hwaddr start_addr,
hwaddr rom_base, hwaddr rom_size,
uint64_t kernel_entry,
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index f5c400dd44..d23f349b4e 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -466,7 +466,7 @@ static void sifive_u_machine_init(MachineState *machine)
 
 /* create device tree */
 create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
-   riscv_is_32bit(s->soc.u_cpus));
+   riscv_is_32bit(>soc.u_cpus));
 
 if (s->start_in_flash) {
 /*
@@ -495,7 +495,7 @@ static void sifive_u_machine_init(MachineState *machine)
 break;
 }
 
-if (riscv_is_32bit(s->soc.u_cpus)) {
+if (riscv_is_32bit(>soc.u_cpus)) {
 firmware_end_addr = riscv_find_and_load_firmware(machine,
 "opensbi-riscv32-generic-fw_dynamic.bin",
 start_addr, NULL);
@@ -506,7 +506,7 @@ static void sifive_u_machine_init(MachineState *machine)
 }
 
 if (machine->kernel_filename) {
-kernel_start_addr = riscv_calc_kernel_start_addr(s->soc.u_cpus,
+kernel_start_addr = riscv_calc_kernel_start_addr(>soc.u_cpus,
  firmware_end_addr);
 
 kernel_entry = riscv_load_kernel(machine->kernel_filename,
@@ -533,7 +533,7 @@ static void sifive_u_machine_init(MachineState *machine)
 /* Compute the fdt load address in dram */
 fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
machine->ram_size, s->fdt);
-if (!riscv_is_32bit(s->soc.u_cpus)) {
+if (!riscv_is_32bit(>soc.u_cpus)) {
 start_addr_hi32 = (uint64_t)start_addr >> 32;
 }
 
@@ -552,7 +552,7 @@ static void sifive_u_machine_init(MachineState *machine)
 0x,
/* fw_dyn: */
 };
-if (riscv_is_32bit(s->soc.u_cpus)) {
+if (riscv_is_32bit(>soc.u_cpus)) {
 reset_vec[4] = 0x0202a583; /* lw a1, 32(t0) */