1. Patchew
  2. QEMU
  3. [PATCH RESEND v2 0/2] Introduce SPAPR_NR_IPIS and fix max-cpus
  4. View patch

[PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.

Harsh Prateek Bora posted 2 patches 1 year ago
Maintainers: Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>
[PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Harsh Prateek Bora 1 year ago 
spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
the range of CPU IPIs during initialization of nr-irqs property.
It is more appropriate to have its own define which can be further
reused as appropriate for correct interpretation.

Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Suggested-by: Cedric Le Goater <clg@kaod.org>
---
 hw/ppc/spapr_irq.c         | 4 ++--
 include/hw/ppc/spapr_irq.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..0c5db6b161 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -329,7 +329,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_XIRQ_BASE);
+        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_NR_IPIS);
         /*
          * 8 XIVE END structures per CPU. One for each available
          * priority
@@ -356,7 +356,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
     }
 
     spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
-                                      smc->nr_xirqs + SPAPR_XIRQ_BASE);
+                                      smc->nr_xirqs + SPAPR_NR_IPIS);
 
     /*
      * Mostly we don't actually need this until reset, except that not
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index c22a72c9e2..e7a80a8349 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -28,6 +28,7 @@
 #define SPAPR_IRQ_MSI        (SPAPR_XIRQ_BASE + 0x0300)
 
 #define SPAPR_NR_XIRQS       0x1000
+#define SPAPR_NR_IPIS        0x1000
 
 struct SpaprMachineState;
 
-- 
2.39.3
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Cédric Le Goater 1 year ago 
Hello Harsh,

Please add to your .git/config file:

[diff]
	orderFile = /path/to/qemu/scripts/git.orderfile


On 11/22/23 10:28, Harsh Prateek Bora wrote:
> spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
> the range of CPU IPIs during initialization of nr-irqs property.
> It is more appropriate to have its own define which can be further
> reused as appropriate for correct interpretation.
> 
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Suggested-by: Cedric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr_irq.c         | 4 ++--
>   include/hw/ppc/spapr_irq.h | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index a0d1e1298e..0c5db6b161 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -329,7 +329,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_XIRQ_BASE);
> +        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_NR_IPIS);

SPAPR_IRQ_NR_IPIS ?

>           /*
>            * 8 XIVE END structures per CPU. One for each available
>            * priority
> @@ -356,7 +356,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>       }
>   
>       spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
> -                                      smc->nr_xirqs + SPAPR_XIRQ_BASE);
> +                                      smc->nr_xirqs + SPAPR_NR_IPIS);
>   
>       /*
>        * Mostly we don't actually need this until reset, except that not
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index c22a72c9e2..e7a80a8349 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -28,6 +28,7 @@

In include/hw/ppc/spapr_irq.h, we should describe the ranges a bit more.
See commit dcc345b61ebe and ad8de98636e7 for more info. Something like :

   /*
    * The XIVE IRQ backend uses the same layout as the XICS backend but
    * covers the full range of the IRQ number space. The IRQ numbers for
    * the CPU IPIs are allocated at the bottom of this space, below 4K,
    * to preserve compatibility with XICS which does not use that range.
    */

   /*
    * CPU IPI range (XIVE only)
    */
   #define SPAPR_IRQ_IPI        0x0
   #define SPAPR_IRQ_NR_IPIS    0x1000

   /*
    * IRQ range offsets per device type
    */
   #define SPAPR_XIRQ_BASE      XICS_IRQ_BASE /* 0x1000 */


And to make sure the ranges don't overlap, let's add :

   QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE)


Thanks,

C.
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Harsh Prateek Bora 1 year ago 

On 11/22/23 17:01, Cédric Le Goater wrote:
> Hello Harsh,
> 
> Please add to your .git/config file:
> 
> [diff]
>      orderFile = /path/to/qemu/scripts/git.orderfile
> 

Sure, thanks for the suggestion.

> 
> On 11/22/23 10:28, Harsh Prateek Bora wrote:
>> spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
>> the range of CPU IPIs during initialization of nr-irqs property.
>> It is more appropriate to have its own define which can be further
>> reused as appropriate for correct interpretation.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> Suggested-by: Cedric Le Goater <clg@kaod.org>
>> ---
>>   hw/ppc/spapr_irq.c         | 4 ++--
>>   include/hw/ppc/spapr_irq.h | 1 +
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index a0d1e1298e..0c5db6b161 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -329,7 +329,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_XIRQ_BASE);
>> +        qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + 
>> SPAPR_NR_IPIS);
> 
> SPAPR_IRQ_NR_IPIS ?
> 
>>           /*
>>            * 8 XIVE END structures per CPU. One for each available
>>            * priority
>> @@ -356,7 +356,7 @@ void spapr_irq_init(SpaprMachineState *spapr, 
>> Error **errp)
>>       }
>>       spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
>> -                                      smc->nr_xirqs + SPAPR_XIRQ_BASE);
>> +                                      smc->nr_xirqs + SPAPR_NR_IPIS);
>>       /*
>>        * Mostly we don't actually need this until reset, except that not
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index c22a72c9e2..e7a80a8349 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -28,6 +28,7 @@
> 
> In include/hw/ppc/spapr_irq.h, we should describe the ranges a bit more.
> See commit dcc345b61ebe and ad8de98636e7 for more info. Something like :
> 
>    /*
>     * The XIVE IRQ backend uses the same layout as the XICS backend but
>     * covers the full range of the IRQ number space. The IRQ numbers for
>     * the CPU IPIs are allocated at the bottom of this space, below 4K,
>     * to preserve compatibility with XICS which does not use that range.
>     */
> 
>    /*
>     * CPU IPI range (XIVE only)
>     */
>    #define SPAPR_IRQ_IPI        0x0
>    #define SPAPR_IRQ_NR_IPIS    0x1000
> 
>    /*
>     * IRQ range offsets per device type
>     */
>    #define SPAPR_XIRQ_BASE      XICS_IRQ_BASE /* 0x1000 */
> 
> 
> And to make sure the ranges don't overlap, let's add :
> 
>    QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE)
> 

Yeh, this looks much better. Will update and post.

regards,
Harsh
> 
> Thanks,
> 
> C.
> 
>
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Philippe Mathieu-Daudé 1 year ago 
Hi Harsh,

On 22/11/23 10:28, Harsh Prateek Bora wrote:
> spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
> the range of CPU IPIs during initialization of nr-irqs property.
> It is more appropriate to have its own define which can be further
> reused as appropriate for correct interpretation.
> 
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Suggested-by: Cedric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr_irq.c         | 4 ++--
>   include/hw/ppc/spapr_irq.h | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)


> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index c22a72c9e2..e7a80a8349 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -28,6 +28,7 @@
>   #define SPAPR_IRQ_MSI        (SPAPR_XIRQ_BASE + 0x0300)
>   
>   #define SPAPR_NR_XIRQS       0x1000
> +#define SPAPR_NR_IPIS        0x1000

BTW why hexadecimal and not decimal?

Regards,

Phil.
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Cédric Le Goater 1 year ago 
On 11/22/23 12:13, Philippe Mathieu-Daudé wrote:
> Hi Harsh,
> 
> On 22/11/23 10:28, Harsh Prateek Bora wrote:
>> spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
>> the range of CPU IPIs during initialization of nr-irqs property.
>> It is more appropriate to have its own define which can be further
>> reused as appropriate for correct interpretation.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> Suggested-by: Cedric Le Goater <clg@kaod.org>
>> ---
>>   hw/ppc/spapr_irq.c         | 4 ++--
>>   include/hw/ppc/spapr_irq.h | 1 +
>>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> 
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index c22a72c9e2..e7a80a8349 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -28,6 +28,7 @@
>>   #define SPAPR_IRQ_MSI        (SPAPR_XIRQ_BASE + 0x0300)
>>   #define SPAPR_NR_XIRQS       0x1000
>> +#define SPAPR_NR_IPIS        0x1000
> 
> BTW why hexadecimal and not decimal?

I think because the HW IRQs are displayed in hex under Linux. Probably.
It has been a while.

Thanks,

C.
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
Posted by Philippe Mathieu-Daudé 1 year ago 
On 22/11/23 10:28, Harsh Prateek Bora wrote:
> spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
> the range of CPU IPIs during initialization of nr-irqs property.
> It is more appropriate to have its own define which can be further
> reused as appropriate for correct interpretation.
> 
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> Suggested-by: Cedric Le Goater <clg@kaod.org>
> ---
>   hw/ppc/spapr_irq.c         | 4 ++--
>   include/hw/ppc/spapr_irq.h | 1 +
>   2 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.