Re: [PATCH-for-8.0 4/4] hw/ppc/spapr_ovec: Avoid target_ulong spapr_ovec_parse_vector()

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 12/13/22 09:35, Philippe Mathieu-Daudé wrote:
> spapr_ovec.c is a device, but it uses target_ulong which is
> target specific. The hwaddr type (declared in "exec/hwaddr.h")
> better fits hardware addresses.

As said by Harsh, spapr_ovec is in fact a data structure that stores platform
options that are supported by the guest.

That doesn't mean that I oppose the change made here. Aside from semantics - which
I also don't have a strong opinion about it - I don't believe it matters that
much - spapr is 64 bit only, so hwaddr will always be == target_ulong.

Cedric/David/Greg, let me know if you have any restriction/thoughts about this.
I'm inclined to accept it as is.


Daniel

> 
> Change spapr_ovec_parse_vector() to take a hwaddr argument,
> allowing the removal of "cpu.h" in a device header.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/ppc/spapr_ovec.c         | 3 ++-
>   include/hw/ppc/spapr_ovec.h | 4 ++--
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> index b2567caa5c..a18a751b57 100644
> --- a/hw/ppc/spapr_ovec.c
> +++ b/hw/ppc/spapr_ovec.c
> @@ -19,6 +19,7 @@
>   #include "qemu/error-report.h"
>   #include "trace.h"
>   #include <libfdt.h>
> +#include "cpu.h"
>   
>   #define OV_MAXBYTES 256 /* not including length byte */
>   #define OV_MAXBITS (OV_MAXBYTES * BITS_PER_BYTE)
> @@ -176,7 +177,7 @@ static target_ulong vector_addr(target_ulong table_addr, int vector)
>       return table_addr;
>   }
>   
> -SpaprOptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector)
> +SpaprOptionVector *spapr_ovec_parse_vector(hwaddr table_addr, int vector)
>   {
>       SpaprOptionVector *ov;
>       target_ulong addr;
> diff --git a/include/hw/ppc/spapr_ovec.h b/include/hw/ppc/spapr_ovec.h
> index c3e8b98e7e..d756b916e4 100644
> --- a/include/hw/ppc/spapr_ovec.h
> +++ b/include/hw/ppc/spapr_ovec.h
> @@ -37,7 +37,7 @@
>   #ifndef SPAPR_OVEC_H
>   #define SPAPR_OVEC_H
>   
> -#include "cpu.h"
> +#include "exec/hwaddr.h"
>   
>   typedef struct SpaprOptionVector SpaprOptionVector;
>   
> @@ -73,7 +73,7 @@ void spapr_ovec_set(SpaprOptionVector *ov, long bitnr);
>   void spapr_ovec_clear(SpaprOptionVector *ov, long bitnr);
>   bool spapr_ovec_test(SpaprOptionVector *ov, long bitnr);
>   bool spapr_ovec_empty(SpaprOptionVector *ov);
> -SpaprOptionVector *spapr_ovec_parse_vector(target_ulong table_addr, int vector);
> +SpaprOptionVector *spapr_ovec_parse_vector(hwaddr table_addr, int vector);
>   int spapr_dt_ovec(void *fdt, int fdt_offset,
>                     SpaprOptionVector *ov, const char *name);
>