Re: [PATCH-for-9.0 v2 5/8] hw: Prefer qdev_prop_set_bit over object_property_set_bool for QDev

2023-12-12 Thread Peter Maydell 
On Thu, 23 Nov 2023 at 14:38, Philippe Mathieu-Daudé  wrote:
>
> The QOM API is lower level than the QDev one. When an instance is
> QDev and setting the property can not fail (using _abort),
> prefer qdev_prop_set_bit() over object_property_set_bool().
>
> Mechanical transformation using the following coccinelle patch:
>
>   @@
>   expression o, p, v;
>   @@
>   -object_property_set_bool(OBJECT(o), p, v, _abort)
>   +qdev_prop_set_bit(DEVICE(o), p, v)
>   
>   -object_property_set_bool(o, p, v, _abort)
>   +qdev_prop_set_bit(DEVICE(o), p, v)
>
> manually adding the missing "hw/qdev-properties.h" header.
>
> In hw/arm/armsse.c we use the available 'cpudev' instead of 'cpuobj'.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Philippe Mathieu-Daudé 

> @@ -1287,8 +1288,7 @@ void arm_load_kernel(ARMCPU *cpu, MachineState *ms, 
> struct arm_boot_info *info)
>   * CPU.
>   */
>  if (cs != first_cpu) {
> -object_property_set_bool(cpuobj, "start-powered-off", true,
> - _abort);
> +qdev_prop_set_bit(DEVICE(cpuobj), "start-powered-off", true);
>  }
>  }
>  }

This makes this code look a bit weird. Currently we have a loop
which has an "Object *cpuobj" which it uses to set properties,
in both cases using the object_property_* APIs. With this change,
we do half the job using a QOM API and the other half using
a qdev API. It would be good to follow up by converting the
other property-set so we can have a local Device * instead of
the Object *.


> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index eace854335..6733652120 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -263,20 +263,13 @@ static void pc_init1(MachineState *machine,
>  size_t i;
>
>  pci_dev = pci_new_multifunction(-1, pcms->south_bridge);
> -object_property_set_bool(OBJECT(pci_dev), "has-usb",
> - machine_usb(machine), _abort);
> -object_property_set_bool(OBJECT(pci_dev), "has-acpi",
> - x86_machine_is_acpi_enabled(x86ms),
> - _abort);
> -object_property_set_bool(OBJECT(pci_dev), "has-pic", false,
> - _abort);
> -object_property_set_bool(OBJECT(pci_dev), "has-pit", false,
> - _abort);
> -qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);
> -object_property_set_bool(OBJECT(pci_dev), "smm-enabled",
> - x86_machine_is_smm_enabled(x86ms),
> - _abort);
>  dev = DEVICE(pci_dev);
> +qdev_prop_set_bit(dev, "has-usb", machine_usb(machine));
> +qdev_prop_set_bit(dev, "has-acpi", 
> x86_machine_is_acpi_enabled(x86ms));
> +qdev_prop_set_bit(dev, "has-pic", false);
> +qdev_prop_set_bit(dev, "has-pit", false);
> +qdev_prop_set_uint32(DEVICE(pci_dev), "smb_io_base", 0xb100);

This line also can just use "dev".

> +qdev_prop_set_bit(dev, "smm-enabled", 
> x86_machine_is_smm_enabled(x86ms));
>  for (i = 0; i < ISA_NUM_IRQS; i++) {
>  qdev_connect_gpio_out_named(dev, "isa-irqs", i, x86ms->gsi[i]);
>  }

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH-for-9.0 v2 5/8] hw: Prefer qdev_prop_set_bit over object_property_set_bool for QDev

2023-11-26 Thread Harsh Prateek Bora 




On 11/23/23 20:08, Philippe Mathieu-Daudé wrote:

The QOM API is lower level than the QDev one. When an instance is
QDev and setting the property can not fail (using _abort),
prefer qdev_prop_set_bit() over object_property_set_bool().

Mechanical transformation using the following coccinelle patch:

   @@
   expression o, p, v;
   @@
   -object_property_set_bool(OBJECT(o), p, v, _abort)
   +qdev_prop_set_bit(DEVICE(o), p, v)
   
   -object_property_set_bool(o, p, v, _abort)
   +qdev_prop_set_bit(DEVICE(o), p, v)

manually adding the missing "hw/qdev-properties.h" header.

In hw/arm/armsse.c we use the available 'cpudev' instead of 'cpuobj'.

Suggested-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/acpi/cpu_hotplug.c   |  7 +++
  hw/acpi/ich9.c  |  4 ++--
  hw/acpi/piix4.c |  4 ++--
  hw/arm/armsse.c |  3 +--
  hw/arm/armv7m.c |  3 +--
  hw/arm/aspeed_ast2400.c |  3 +--
  hw/arm/aspeed_ast2600.c |  9 +++--
  hw/arm/bcm2835_peripherals.c|  3 +--
  hw/arm/bcm2836.c|  4 ++--
  hw/arm/boot.c   |  4 ++--
  hw/arm/fsl-imx25.c  |  3 +--
  hw/arm/fsl-imx31.c  |  3 +--
  hw/arm/fsl-imx6.c   | 12 
  hw/arm/fsl-imx6ul.c |  8 
  hw/arm/fsl-imx7.c   | 10 --
  hw/arm/npcm7xx.c|  9 +++--
  hw/arm/xlnx-versal.c|  9 +++--
  hw/arm/xlnx-zynqmp.c|  9 +++--
  hw/core/bus.c   |  2 +-
  hw/core/qdev.c  |  2 +-
  hw/i386/pc_piix.c   | 19 ++-
  hw/microblaze/petalogix_ml605_mmu.c |  5 ++---
  hw/microblaze/xlnx-zynqmp-pmu.c | 18 +++---
  hw/mips/cps.c   |  3 +--
  hw/ppc/e500.c   |  3 +--
  hw/ppc/spapr_pci.c  |  3 +--


For spapr:
Reviewed-by: Harsh Prateek Bora 


  hw/rx/rx-gdbsim.c   |  4 ++--
  hw/sparc/sun4m.c|  3 +--
  28 files changed, 64 insertions(+), 105 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 634bbecb31..1338c037b5 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -12,6 +12,7 @@
  #include "qemu/osdep.h"
  #include "hw/acpi/cpu_hotplug.h"
  #include "qapi/error.h"
+#include "hw/qdev-properties.h"
  #include "hw/core/cpu.h"
  #include "hw/i386/pc.h"
  #include "hw/pci/pci.h"
@@ -41,8 +42,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, 
uint64_t data,
   */
  if (addr == 0 && data == 0) {
  AcpiCpuHotplug *cpus = opaque;
-object_property_set_bool(cpus->device, "cpu-hotplug-legacy", false,
- _abort);
+qdev_prop_set_bit(DEVICE(cpus->device), "cpu-hotplug-legacy", false);
  }
  }
  
@@ -66,8 +66,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
  
  cpu_id = k->get_arch_id(cpu);

  if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
-object_property_set_bool(g->device, "cpu-hotplug-legacy", false,
- _abort);
+qdev_prop_set_bit(DEVICE(g->device), "cpu-hotplug-legacy", false);
  return;
  }
  
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c

index 25e2c7243e..64b00673fe 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -30,6 +30,7 @@
  #include "hw/pci/pci.h"
  #include "migration/vmstate.h"
  #include "qemu/timer.h"
+#include "hw/qdev-properties.h"
  #include "hw/core/cpu.h"
  #include "sysemu/reset.h"
  #include "sysemu/runstate.h"
@@ -197,8 +198,7 @@ static bool vmstate_test_use_cpuhp(void *opaque)
  static int vmstate_cpuhp_pre_load(void *opaque)
  {
  ICH9LPCPMRegs *s = opaque;
-Object *obj = OBJECT(s->gpe_cpu.device);
-object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
+qdev_prop_set_bit(DEVICE(s->gpe_cpu.device), "cpu-hotplug-legacy", false);
  return 0;
  }
  
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c

index dd523d2e4c..215929ac6a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -203,8 +203,8 @@ static bool vmstate_test_use_cpuhp(void *opaque)
  
  static int vmstate_cpuhp_pre_load(void *opaque)

  {
-Object *obj = OBJECT(opaque);
-object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
+DeviceState *dev = DEVICE(opaque);
+qdev_prop_set_bit(dev, "cpu-hotplug-legacy", false);
  return 0;
  }
  
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c

index 4672df180f..546b15e658 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1022,8 +1022,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
   * later if necessary.
   */
  if (extract32(info->cpuwait_rst, i, 1)) {
-

Re: [PATCH-for-9.0 v2 5/8] hw: Prefer qdev_prop_set_bit over object_property_set_bool for QDev

2023-11-24 Thread Ani Sinha 



> On 23-Nov-2023, at 8:08 PM, Philippe Mathieu-Daudé  wrote:
> 
> The QOM API is lower level than the QDev one. When an instance is
> QDev and setting the property can not fail (using _abort),
> prefer qdev_prop_set_bit() over object_property_set_bool().
> 
> Mechanical transformation using the following coccinelle patch:
> 
>  @@
>  expression o, p, v;
>  @@
>  -object_property_set_bool(OBJECT(o), p, v, _abort)
>  +qdev_prop_set_bit(DEVICE(o), p, v)
>  
>  -object_property_set_bool(o, p, v, _abort)
>  +qdev_prop_set_bit(DEVICE(o), p, v)
> 
> manually adding the missing "hw/qdev-properties.h" header.
> 
> In hw/arm/armsse.c we use the available 'cpudev' instead of 'cpuobj'.
> 
> Suggested-by: Markus Armbruster 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> hw/acpi/cpu_hotplug.c   |  7 +++
> hw/acpi/ich9.c  |  4 ++--
> hw/acpi/piix4.c |  4 ++--

For the acpi bits,
Reviewed-by: Ani Sinha 

> hw/arm/armsse.c |  3 +--
> hw/arm/armv7m.c |  3 +--
> hw/arm/aspeed_ast2400.c |  3 +--
> hw/arm/aspeed_ast2600.c |  9 +++--
> hw/arm/bcm2835_peripherals.c|  3 +--
> hw/arm/bcm2836.c|  4 ++--
> hw/arm/boot.c   |  4 ++--
> hw/arm/fsl-imx25.c  |  3 +--
> hw/arm/fsl-imx31.c  |  3 +--
> hw/arm/fsl-imx6.c   | 12 
> hw/arm/fsl-imx6ul.c |  8 
> hw/arm/fsl-imx7.c   | 10 --
> hw/arm/npcm7xx.c|  9 +++--
> hw/arm/xlnx-versal.c|  9 +++--
> hw/arm/xlnx-zynqmp.c|  9 +++--
> hw/core/bus.c   |  2 +-
> hw/core/qdev.c  |  2 +-
> hw/i386/pc_piix.c   | 19 ++-
> hw/microblaze/petalogix_ml605_mmu.c |  5 ++---
> hw/microblaze/xlnx-zynqmp-pmu.c | 18 +++---
> hw/mips/cps.c   |  3 +--
> hw/ppc/e500.c   |  3 +--
> hw/ppc/spapr_pci.c  |  3 +--
> hw/rx/rx-gdbsim.c   |  4 ++--
> hw/sparc/sun4m.c|  3 +--
> 28 files changed, 64 insertions(+), 105 deletions(-)
> 
> diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
> index 634bbecb31..1338c037b5 100644
> --- a/hw/acpi/cpu_hotplug.c
> +++ b/hw/acpi/cpu_hotplug.c
> @@ -12,6 +12,7 @@
> #include "qemu/osdep.h"
> #include "hw/acpi/cpu_hotplug.h"
> #include "qapi/error.h"
> +#include "hw/qdev-properties.h"
> #include "hw/core/cpu.h"
> #include "hw/i386/pc.h"
> #include "hw/pci/pci.h"
> @@ -41,8 +42,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, 
> uint64_t data,
>  */
> if (addr == 0 && data == 0) {
> AcpiCpuHotplug *cpus = opaque;
> -object_property_set_bool(cpus->device, "cpu-hotplug-legacy", false,
> - _abort);
> +qdev_prop_set_bit(DEVICE(cpus->device), "cpu-hotplug-legacy", false);
> }
> }
> 
> @@ -66,8 +66,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
> CPUState *cpu)
> 
> cpu_id = k->get_arch_id(cpu);
> if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
> -object_property_set_bool(g->device, "cpu-hotplug-legacy", false,
> - _abort);
> +qdev_prop_set_bit(DEVICE(g->device), "cpu-hotplug-legacy", false);
> return;
> }
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 25e2c7243e..64b00673fe 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -30,6 +30,7 @@
> #include "hw/pci/pci.h"
> #include "migration/vmstate.h"
> #include "qemu/timer.h"
> +#include "hw/qdev-properties.h"
> #include "hw/core/cpu.h"
> #include "sysemu/reset.h"
> #include "sysemu/runstate.h"
> @@ -197,8 +198,7 @@ static bool vmstate_test_use_cpuhp(void *opaque)
> static int vmstate_cpuhp_pre_load(void *opaque)
> {
> ICH9LPCPMRegs *s = opaque;
> -Object *obj = OBJECT(s->gpe_cpu.device);
> -object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
> +qdev_prop_set_bit(DEVICE(s->gpe_cpu.device), "cpu-hotplug-legacy", 
> false);
> return 0;
> }
> 
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index dd523d2e4c..215929ac6a 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -203,8 +203,8 @@ static bool vmstate_test_use_cpuhp(void *opaque)
> 
> static int vmstate_cpuhp_pre_load(void *opaque)
> {
> -Object *obj = OBJECT(opaque);
> -object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
> +DeviceState *dev = DEVICE(opaque);
> +qdev_prop_set_bit(dev, "cpu-hotplug-legacy", false);
> return 0;
> }
> 
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 4672df180f..546b15e658 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -1022,8 +1022,7 @@ static void armsse_realize(DeviceState *dev, Error 
> **errp)
>  *

[PATCH-for-9.0 v2 5/8] hw: Prefer qdev_prop_set_bit over object_property_set_bool for QDev

2023-11-23 Thread Philippe Mathieu-Daudé 
The QOM API is lower level than the QDev one. When an instance is
QDev and setting the property can not fail (using _abort),
prefer qdev_prop_set_bit() over object_property_set_bool().

Mechanical transformation using the following coccinelle patch:

  @@
  expression o, p, v;
  @@
  -object_property_set_bool(OBJECT(o), p, v, _abort)
  +qdev_prop_set_bit(DEVICE(o), p, v)
  
  -object_property_set_bool(o, p, v, _abort)
  +qdev_prop_set_bit(DEVICE(o), p, v)

manually adding the missing "hw/qdev-properties.h" header.

In hw/arm/armsse.c we use the available 'cpudev' instead of 'cpuobj'.

Suggested-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/acpi/cpu_hotplug.c   |  7 +++
 hw/acpi/ich9.c  |  4 ++--
 hw/acpi/piix4.c |  4 ++--
 hw/arm/armsse.c |  3 +--
 hw/arm/armv7m.c |  3 +--
 hw/arm/aspeed_ast2400.c |  3 +--
 hw/arm/aspeed_ast2600.c |  9 +++--
 hw/arm/bcm2835_peripherals.c|  3 +--
 hw/arm/bcm2836.c|  4 ++--
 hw/arm/boot.c   |  4 ++--
 hw/arm/fsl-imx25.c  |  3 +--
 hw/arm/fsl-imx31.c  |  3 +--
 hw/arm/fsl-imx6.c   | 12 
 hw/arm/fsl-imx6ul.c |  8 
 hw/arm/fsl-imx7.c   | 10 --
 hw/arm/npcm7xx.c|  9 +++--
 hw/arm/xlnx-versal.c|  9 +++--
 hw/arm/xlnx-zynqmp.c|  9 +++--
 hw/core/bus.c   |  2 +-
 hw/core/qdev.c  |  2 +-
 hw/i386/pc_piix.c   | 19 ++-
 hw/microblaze/petalogix_ml605_mmu.c |  5 ++---
 hw/microblaze/xlnx-zynqmp-pmu.c | 18 +++---
 hw/mips/cps.c   |  3 +--
 hw/ppc/e500.c   |  3 +--
 hw/ppc/spapr_pci.c  |  3 +--
 hw/rx/rx-gdbsim.c   |  4 ++--
 hw/sparc/sun4m.c|  3 +--
 28 files changed, 64 insertions(+), 105 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 634bbecb31..1338c037b5 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -12,6 +12,7 @@
 #include "qemu/osdep.h"
 #include "hw/acpi/cpu_hotplug.h"
 #include "qapi/error.h"
+#include "hw/qdev-properties.h"
 #include "hw/core/cpu.h"
 #include "hw/i386/pc.h"
 #include "hw/pci/pci.h"
@@ -41,8 +42,7 @@ static void cpu_status_write(void *opaque, hwaddr addr, 
uint64_t data,
  */
 if (addr == 0 && data == 0) {
 AcpiCpuHotplug *cpus = opaque;
-object_property_set_bool(cpus->device, "cpu-hotplug-legacy", false,
- _abort);
+qdev_prop_set_bit(DEVICE(cpus->device), "cpu-hotplug-legacy", false);
 }
 }
 
@@ -66,8 +66,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, 
CPUState *cpu)
 
 cpu_id = k->get_arch_id(cpu);
 if ((cpu_id / 8) >= ACPI_GPE_PROC_LEN) {
-object_property_set_bool(g->device, "cpu-hotplug-legacy", false,
- _abort);
+qdev_prop_set_bit(DEVICE(g->device), "cpu-hotplug-legacy", false);
 return;
 }
 
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 25e2c7243e..64b00673fe 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -30,6 +30,7 @@
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qemu/timer.h"
+#include "hw/qdev-properties.h"
 #include "hw/core/cpu.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
@@ -197,8 +198,7 @@ static bool vmstate_test_use_cpuhp(void *opaque)
 static int vmstate_cpuhp_pre_load(void *opaque)
 {
 ICH9LPCPMRegs *s = opaque;
-Object *obj = OBJECT(s->gpe_cpu.device);
-object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
+qdev_prop_set_bit(DEVICE(s->gpe_cpu.device), "cpu-hotplug-legacy", false);
 return 0;
 }
 
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index dd523d2e4c..215929ac6a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -203,8 +203,8 @@ static bool vmstate_test_use_cpuhp(void *opaque)
 
 static int vmstate_cpuhp_pre_load(void *opaque)
 {
-Object *obj = OBJECT(opaque);
-object_property_set_bool(obj, "cpu-hotplug-legacy", false, _abort);
+DeviceState *dev = DEVICE(opaque);
+qdev_prop_set_bit(dev, "cpu-hotplug-legacy", false);
 return 0;
 }
 
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 4672df180f..546b15e658 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1022,8 +1022,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
  * later if necessary.
  */
 if (extract32(info->cpuwait_rst, i, 1)) {
-object_property_set_bool(cpuobj, "start-powered-off", true,
- _abort);
+qdev_prop_set_bit(cpudev, "start-powered-off", true);
 }
 if