[PATCH v2 02/11] hw/ppc/spapr: Remove SpaprMachineClass::nr_xirqs field

Philippe Mathieu-Daudé posted 11 patches 3 weeks, 3 days ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Chinmay Rath <rathc@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH v2 02/11] hw/ppc/spapr: Remove SpaprMachineClass::nr_xirqs field
Posted by Philippe Mathieu-Daudé 3 weeks, 3 days ago
The SpaprMachineClass::nr_xirqs field was only used by the
pseries-3.0 machine, which got removed. Remove it as now unused.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/ppc/spapr.h |  1 -
 hw/ppc/spapr.c         |  1 -
 hw/ppc/spapr_irq.c     | 22 +++++++---------------
 3 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 0c1e5132de2..494367fb99a 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -145,7 +145,6 @@ struct SpaprMachineClass {
     /*< public >*/
     bool dr_phb_enabled;       /* enable dynamic-reconfig/hotplug of PHBs */
     bool update_dt_enabled;    /* enable KVMPPC_H_UPDATE_DT */
-    uint32_t nr_xirqs;
     bool broken_host_serial_model; /* present real host info to the guest */
     bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
     bool linux_pci_probe;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 426a778d3e8..b5d20bc1756 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4691,7 +4691,6 @@ static void spapr_machine_class_init(ObjectClass *oc, const void *data)
     smc->dr_phb_enabled = true;
     smc->linux_pci_probe = true;
     smc->smp_threads_vsmt = true;
-    smc->nr_xirqs = SPAPR_NR_XIRQS;
     xfc->match_nvt = spapr_match_nvt;
     vmc->client_architecture_support = spapr_vof_client_architecture_support;
     vmc->quiesce = spapr_vof_quiesce;
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 317d57a3802..2ce323457be 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -279,15 +279,11 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers,
 
 uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr)
 {
-    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
-
-    return SPAPR_XIRQ_BASE + smc->nr_xirqs - SPAPR_IRQ_MSI;
+    return SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE - SPAPR_IRQ_MSI;
 }
 
 void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
 {
-    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
-
     if (kvm_enabled() && kvm_kernel_irqchip_split()) {
         error_setg(errp, "kernel_irqchip split mode not supported on pseries");
         return;
@@ -308,7 +304,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
         object_property_add_child(OBJECT(spapr), "ics", obj);
         object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
                                  &error_abort);
-        object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
+        object_property_set_int(obj, "nr-irqs", SPAPR_NR_XIRQS, &error_abort);
         if (!qdev_realize(DEVICE(obj), NULL, errp)) {
             return;
         }
@@ -322,7 +318,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
         int i;
 
         dev = qdev_new(TYPE_SPAPR_XIVE);
-        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_IRQ_NR_IPIS);
+        qdev_prop_set_uint32(dev, "nr-irqs", SPAPR_NR_XIRQS + SPAPR_IRQ_NR_IPIS);
         /*
          * 8 XIVE END structures per CPU. One for each available
          * priority
@@ -349,7 +345,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
     }
 
     spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
-                                      smc->nr_xirqs + SPAPR_IRQ_NR_IPIS);
+                                      SPAPR_NR_XIRQS + SPAPR_IRQ_NR_IPIS);
 
     /*
      * Mostly we don't actually need this until reset, except that not
@@ -364,11 +360,10 @@ int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp)
 {
     SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
     int i;
-    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
     int rc;
 
     assert(irq >= SPAPR_XIRQ_BASE);
-    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
+    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
 
     for (i = 0; i < ARRAY_SIZE(intcs); i++) {
         SpaprInterruptController *intc = intcs[i];
@@ -388,10 +383,9 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
 {
     SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
     int i, j;
-    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
 
     assert(irq >= SPAPR_XIRQ_BASE);
-    assert((irq + num) <= (smc->nr_xirqs + SPAPR_XIRQ_BASE));
+    assert((irq + num) <= (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
 
     for (i = irq; i < (irq + num); i++) {
         for (j = 0; j < ARRAY_SIZE(intcs); j++) {
@@ -408,8 +402,6 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
 
 qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
 {
-    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
-
     /*
      * This interface is basically for VIO and PHB devices to find the
      * right qemu_irq to manipulate, so we only allow access to the
@@ -418,7 +410,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
      * interfaces, we can change this if we need to in future.
      */
     assert(irq >= SPAPR_XIRQ_BASE);
-    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
+    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
 
     if (spapr->ics) {
         assert(ics_valid_irq(spapr->ics, irq));
-- 
2.51.0


Re: [PATCH v2 02/11] hw/ppc/spapr: Remove SpaprMachineClass::nr_xirqs field
Posted by Chinmay Rath 3 weeks, 3 days ago
On 10/21/25 14:13, Philippe Mathieu-Daudé wrote:
> The SpaprMachineClass::nr_xirqs field was only used by the
> pseries-3.0 machine, which got removed. Remove it as now unused.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/ppc/spapr.h |  1 -
>   hw/ppc/spapr.c         |  1 -
>   hw/ppc/spapr_irq.c     | 22 +++++++---------------
>   3 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 0c1e5132de2..494367fb99a 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -145,7 +145,6 @@ struct SpaprMachineClass {
>       /*< public >*/
>       bool dr_phb_enabled;       /* enable dynamic-reconfig/hotplug of PHBs */
>       bool update_dt_enabled;    /* enable KVMPPC_H_UPDATE_DT */
> -    uint32_t nr_xirqs;
>       bool broken_host_serial_model; /* present real host info to the guest */
>       bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
>       bool linux_pci_probe;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 426a778d3e8..b5d20bc1756 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -4691,7 +4691,6 @@ static void spapr_machine_class_init(ObjectClass *oc, const void *data)
>       smc->dr_phb_enabled = true;
>       smc->linux_pci_probe = true;
>       smc->smp_threads_vsmt = true;
> -    smc->nr_xirqs = SPAPR_NR_XIRQS;
>       xfc->match_nvt = spapr_match_nvt;
>       vmc->client_architecture_support = spapr_vof_client_architecture_support;
>       vmc->quiesce = spapr_vof_quiesce;
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 317d57a3802..2ce323457be 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -279,15 +279,11 @@ void spapr_irq_dt(SpaprMachineState *spapr, uint32_t nr_servers,
>   
>   uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr)
>   {
> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
> -    return SPAPR_XIRQ_BASE + smc->nr_xirqs - SPAPR_IRQ_MSI;
> +    return SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE - SPAPR_IRQ_MSI;
>   }
Hey,
With this cleanup, I think we can get rid of the SpaprMachineState 
argument passed to spapr_irq_nr_msis function since it is not used 
anymore ^.

Regards,
Chinmay
>   
>   void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>   {
> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
>       if (kvm_enabled() && kvm_kernel_irqchip_split()) {
>           error_setg(errp, "kernel_irqchip split mode not supported on pseries");
>           return;
> @@ -308,7 +304,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>           object_property_add_child(OBJECT(spapr), "ics", obj);
>           object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
>                                    &error_abort);
> -        object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, &error_abort);
> +        object_property_set_int(obj, "nr-irqs", SPAPR_NR_XIRQS, &error_abort);
>           if (!qdev_realize(DEVICE(obj), NULL, errp)) {
>               return;
>           }
> @@ -322,7 +318,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>           int i;
>   
>           dev = qdev_new(TYPE_SPAPR_XIVE);
> -        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_IRQ_NR_IPIS);
> +        qdev_prop_set_uint32(dev, "nr-irqs", SPAPR_NR_XIRQS + SPAPR_IRQ_NR_IPIS);
>           /*
>            * 8 XIVE END structures per CPU. One for each available
>            * priority
> @@ -349,7 +345,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>       }
>   
>       spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
> -                                      smc->nr_xirqs + SPAPR_IRQ_NR_IPIS);
> +                                      SPAPR_NR_XIRQS + SPAPR_IRQ_NR_IPIS);
>   
>       /*
>        * Mostly we don't actually need this until reset, except that not
> @@ -364,11 +360,10 @@ int spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error **errp)
>   {
>       SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
>       int i;
> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>       int rc;
>   
>       assert(irq >= SPAPR_XIRQ_BASE);
> -    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
> +    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>   
>       for (i = 0; i < ARRAY_SIZE(intcs); i++) {
>           SpaprInterruptController *intc = intcs[i];
> @@ -388,10 +383,9 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
>   {
>       SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
>       int i, j;
> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>   
>       assert(irq >= SPAPR_XIRQ_BASE);
> -    assert((irq + num) <= (smc->nr_xirqs + SPAPR_XIRQ_BASE));
> +    assert((irq + num) <= (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>   
>       for (i = irq; i < (irq + num); i++) {
>           for (j = 0; j < ARRAY_SIZE(intcs); j++) {
> @@ -408,8 +402,6 @@ void spapr_irq_free(SpaprMachineState *spapr, int irq, int num)
>   
>   qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>   {
> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
>       /*
>        * This interface is basically for VIO and PHB devices to find the
>        * right qemu_irq to manipulate, so we only allow access to the
> @@ -418,7 +410,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>        * interfaces, we can change this if we need to in future.
>        */
>       assert(irq >= SPAPR_XIRQ_BASE);
> -    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
> +    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>   
>       if (spapr->ics) {
>           assert(ics_valid_irq(spapr->ics, irq));

Re: [PATCH v2 02/11] hw/ppc/spapr: Remove SpaprMachineClass::nr_xirqs field
Posted by Chinmay Rath 3 weeks, 2 days ago
On 10/21/25 16:05, Chinmay Rath wrote:
>
> On 10/21/25 14:13, Philippe Mathieu-Daudé wrote:
>> The SpaprMachineClass::nr_xirqs field was only used by the
>> pseries-3.0 machine, which got removed. Remove it as now unused.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/ppc/spapr.h |  1 -
>>   hw/ppc/spapr.c         |  1 -
>>   hw/ppc/spapr_irq.c     | 22 +++++++---------------
>>   3 files changed, 7 insertions(+), 17 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 0c1e5132de2..494367fb99a 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -145,7 +145,6 @@ struct SpaprMachineClass {
>>       /*< public >*/
>>       bool dr_phb_enabled;       /* enable dynamic-reconfig/hotplug 
>> of PHBs */
>>       bool update_dt_enabled;    /* enable KVMPPC_H_UPDATE_DT */
>> -    uint32_t nr_xirqs;
>>       bool broken_host_serial_model; /* present real host info to the 
>> guest */
>>       bool pre_4_1_migration; /* don't migrate hpt-max-page-size */
>>       bool linux_pci_probe;
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 426a778d3e8..b5d20bc1756 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -4691,7 +4691,6 @@ static void 
>> spapr_machine_class_init(ObjectClass *oc, const void *data)
>>       smc->dr_phb_enabled = true;
>>       smc->linux_pci_probe = true;
>>       smc->smp_threads_vsmt = true;
>> -    smc->nr_xirqs = SPAPR_NR_XIRQS;
>>       xfc->match_nvt = spapr_match_nvt;
>>       vmc->client_architecture_support = 
>> spapr_vof_client_architecture_support;
>>       vmc->quiesce = spapr_vof_quiesce;
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 317d57a3802..2ce323457be 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -279,15 +279,11 @@ void spapr_irq_dt(SpaprMachineState *spapr, 
>> uint32_t nr_servers,
>>     uint32_t spapr_irq_nr_msis(SpaprMachineState *spapr)
>>   {
>> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> -
>> -    return SPAPR_XIRQ_BASE + smc->nr_xirqs - SPAPR_IRQ_MSI;
>> +    return SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE - SPAPR_IRQ_MSI;
>>   }
> Hey,
> With this cleanup, I think we can get rid of the SpaprMachineState 
> argument passed to spapr_irq_nr_msis function since it is not used 
> anymore ^.
>
> Regards,
> Chinmay
On second thoughts, since the functions is just returning a constant 
value now, would it make sense to get rid of the functions all together 
and replace the calls with the respective constant ?

Regards,
Chinmay

>>     void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>>   {
>> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> -
>>       if (kvm_enabled() && kvm_kernel_irqchip_split()) {
>>           error_setg(errp, "kernel_irqchip split mode not supported 
>> on pseries");
>>           return;
>> @@ -308,7 +304,7 @@ void spapr_irq_init(SpaprMachineState *spapr, 
>> Error **errp)
>>           object_property_add_child(OBJECT(spapr), "ics", obj);
>>           object_property_set_link(obj, ICS_PROP_XICS, OBJECT(spapr),
>>                                    &error_abort);
>> -        object_property_set_int(obj, "nr-irqs", smc->nr_xirqs, 
>> &error_abort);
>> +        object_property_set_int(obj, "nr-irqs", SPAPR_NR_XIRQS, 
>> &error_abort);
>>           if (!qdev_realize(DEVICE(obj), NULL, errp)) {
>>               return;
>>           }
>> @@ -322,7 +318,7 @@ void spapr_irq_init(SpaprMachineState *spapr, 
>> Error **errp)
>>           int i;
>>             dev = qdev_new(TYPE_SPAPR_XIVE);
>> -        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + 
>> SPAPR_IRQ_NR_IPIS);
>> +        qdev_prop_set_uint32(dev, "nr-irqs", SPAPR_NR_XIRQS + 
>> SPAPR_IRQ_NR_IPIS);
>>           /*
>>            * 8 XIVE END structures per CPU. One for each available
>>            * priority
>> @@ -349,7 +345,7 @@ void spapr_irq_init(SpaprMachineState *spapr, 
>> Error **errp)
>>       }
>>         spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
>> -                                      smc->nr_xirqs + 
>> SPAPR_IRQ_NR_IPIS);
>> +                                      SPAPR_NR_XIRQS + 
>> SPAPR_IRQ_NR_IPIS);
>>         /*
>>        * Mostly we don't actually need this until reset, except that not
>> @@ -364,11 +360,10 @@ int spapr_irq_claim(SpaprMachineState *spapr, 
>> int irq, bool lsi, Error **errp)
>>   {
>>       SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
>>       int i;
>> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>       int rc;
>>         assert(irq >= SPAPR_XIRQ_BASE);
>> -    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
>> +    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>>         for (i = 0; i < ARRAY_SIZE(intcs); i++) {
>>           SpaprInterruptController *intc = intcs[i];
>> @@ -388,10 +383,9 @@ void spapr_irq_free(SpaprMachineState *spapr, 
>> int irq, int num)
>>   {
>>       SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
>>       int i, j;
>> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>>         assert(irq >= SPAPR_XIRQ_BASE);
>> -    assert((irq + num) <= (smc->nr_xirqs + SPAPR_XIRQ_BASE));
>> +    assert((irq + num) <= (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>>         for (i = irq; i < (irq + num); i++) {
>>           for (j = 0; j < ARRAY_SIZE(intcs); j++) {
>> @@ -408,8 +402,6 @@ void spapr_irq_free(SpaprMachineState *spapr, int 
>> irq, int num)
>>     qemu_irq spapr_qirq(SpaprMachineState *spapr, int irq)
>>   {
>> -    SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>> -
>>       /*
>>        * This interface is basically for VIO and PHB devices to find the
>>        * right qemu_irq to manipulate, so we only allow access to the
>> @@ -418,7 +410,7 @@ qemu_irq spapr_qirq(SpaprMachineState *spapr, int 
>> irq)
>>        * interfaces, we can change this if we need to in future.
>>        */
>>       assert(irq >= SPAPR_XIRQ_BASE);
>> -    assert(irq < (smc->nr_xirqs + SPAPR_XIRQ_BASE));
>> +    assert(irq < (SPAPR_NR_XIRQS + SPAPR_XIRQ_BASE));
>>         if (spapr->ics) {
>>           assert(ics_valid_irq(spapr->ics, irq));
>