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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/21/22 06:46, Cédric Le Goater wrote:
On 12/16/22 17:47, Daniel Henrique Barboza wrote:


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.

Well, I am not sure.

The vector table variable is the result of a ppc64_phys_to_real() conversion
of the CAS hcall parameter, which is a target_ulong, but ppc64_phys_to_real()
returns a uint64_t.

The code is not consistent in another places :

   hw/ppc/spapr_tpm_proxy.c uses a uint64_t
   hw/ppc/spapr_hcall.c, a target_ulong
   hw/ppc/spapr_rtas.c, a hwaddr
   hw/ppc/spapr_drc.c, a hwaddr indirectly

Should we change ppc64_phys_to_real() to return an hwaddr (also) ?

It makes sense to me a function called ppc64_phys_to_real() returning
a hwaddr type.

Daniel



C.




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@xxxxxxxxxx>
---
  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);




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux