Re: [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class

2016-06-30 Thread Nikunj A Dadhania 
Cédric Le Goater  writes:

> On 06/28/2016 09:05 PM, Nikunj A Dadhania wrote:
>> From: Benjamin Herrenschmidt 
>> 
>> The existing implementation remains same and ics-base is introduced. The
>> type name "ics" is retained, and all the related functions renamed as
>> ics_simple_*
>> 
>> This will allow different implementations for the source controllers
>> such as the MSI support of PHB3 on Power8 which uses in-memory state
>> tables for example.
>> 
>> Signed-off-by: Benjamin Herrenschmidt 
>> Signed-off-by: Nikunj A Dadhania 

>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index bbdba84..39928d9 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
>>  {
>>  ICSState *ics;
>>  
>> -ics = ICS(object_new(TYPE_ICS));
>> +ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));
>
> Should not that be ICS_BASE() ? 

No, we renamed ICS => ICS_SIMPLE (retaining the typename "ics" for
migration compatibility). ICS_SIMPLE is a child of ICS_BASE

>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 8433bf9..bc10c16 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -118,22 +118,29 @@ struct ICPState {
>>  bool cap_irq_xics_enabled;
>>  };
>>  
>> -#define TYPE_ICS "ics"
>> -#define ICS(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS)
>> +#define TYPE_ICS_BASE "ics-base"
>> +#define ICS_BASE(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SIMPLE)
>
> should not that be TYPE_ICS_BASE ?

Oops, you are right.

>>  #define ICS_CLASS(klass) \
>> - OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS)
>> + OBJECT_CLASS_CHECK(ICSStateClass, (klass), TYPE_ICS_SIMPLE)
>
> and here  ? 
>
>
>>  #define ICS_GET_CLASS(obj) \
>> - OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS)
>> + OBJECT_GET_CLASS(ICSStateClass, (obj), TYPE_ICS_SIMPLE)
>
> and here  ? 
>
> but I might be confusing all these ICS :)

Earlier there were two
1) TYPE_ICS
2) TYPE_ICS_KVM (parent: TYPE_ICS)

Now:
1) TYPE_ICS_BASE - new
2) TYPE_ICS_SIMPLE - was known as TYPE_ICS
3) TYPE_ICS_KVM - same as before (parent: TYPE_ICS_SIMPLE)

Regards
Nikunj

Re: [Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class

2016-06-30 Thread Cédric Le Goater 
On 06/28/2016 09:05 PM, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt 
> 
> The existing implementation remains same and ics-base is introduced. The
> type name "ics" is retained, and all the related functions renamed as
> ics_simple_*
> 
> This will allow different implementations for the source controllers
> such as the MSI support of PHB3 on Power8 which uses in-memory state
> tables for example.
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Signed-off-by: Nikunj A Dadhania 
> ---
>  hw/intc/trace-events  |  10 ++--
>  hw/intc/xics.c| 146 
> +++---
>  hw/intc/xics_kvm.c|  10 ++--
>  hw/intc/xics_spapr.c  |  28 +-
>  include/hw/ppc/xics.h |  23 +---
>  5 files changed, 132 insertions(+), 85 deletions(-)
> 
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 5f0f783..e5e7ec7 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) 
> "icp_accept: XIRR %#"PRIx3
>  xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server 
> %d given XIRR %#"PRIx32" new XIRR %#"PRIx32
>  xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver 
> irq %#"PRIx32" priority %#x"
>  xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new 
> XIRR=%#x new pending priority=%#x"
> -xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
> +xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 
> %#x]"
>  xics_masked_pending(void) "set_irq_msi: masked pending"
> -xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
> -xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) 
> "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
> -xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
> -xics_ics_eoi(int nr) "ics_eoi: irq %#x"
> +xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 
> %#x]"
> +xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) 
> "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
> +xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
> +xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
>  xics_alloc(int irq) "irq %d"
>  xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d 
> irqs, lsi=%d, alignnum %d"
>  xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index bbdba84..39928d9 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
>  {
>  ICSState *ics;
>  
> -ics = ICS(object_new(TYPE_ICS));
> +ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));

Should not that be ICS_BASE() ? 


>  object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
>  ics->xics = xics;
>  QLIST_INSERT_HEAD(>ics, ics, list);
> @@ -223,9 +223,32 @@ static const TypeInfo xics_common_info = {
>  #define XISR(ss)   (((ss)->xirr) & XISR_MASK)
>  #define CPPR(ss)   (((ss)->xirr) >> 24)
>  
> -static void ics_reject(ICSState *ics, int nr);
> -static void ics_resend(ICSState *ics);
> -static void ics_eoi(ICSState *ics, int nr);
> +static void ics_reject(ICSState *ics, uint32_t nr)
> +{
> +ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +if (k->reject) {
> +k->reject(ics, nr);
> +}
> +}
> +
> +static void ics_resend(ICSState *ics)
> +{
> +ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +if (k->resend) {
> +k->resend(ics);
> +}
> +}
> +
> +static void ics_eoi(ICSState *ics, int nr)
> +{
> +ICSStateClass *k = ICS_GET_CLASS(ics);
> +
> +if (k->eoi) {
> +k->eoi(ics, nr);
> +}
> +}
>  
>  static void icp_check_ipi(ICPState *ss)
>  {
> @@ -428,7 +451,7 @@ static const TypeInfo icp_info = {
>  /*
>   * ICS: Source layer
>   */
> -static void resend_msi(ICSState *ics, int srcno)
> +static void ics_simple_resend_msi(ICSState *ics, int srcno)
>  {
>  ICSIRQState *irq = ics->irqs + srcno;
>  
> @@ -441,7 +464,7 @@ static void resend_msi(ICSState *ics, int srcno)
>  }
>  }
>  
> -static void resend_lsi(ICSState *ics, int srcno)
> +static void ics_simple_resend_lsi(ICSState *ics, int srcno)
>  {
>  ICSIRQState *irq = ics->irqs + srcno;
>  
> @@ -453,11 +476,11 @@ static void resend_lsi(ICSState *ics, int srcno)
>  }
>  }
>  
> -static void set_irq_msi(ICSState *ics, int srcno, int val)
> +static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val)
>  {
>  ICSIRQState *irq = ics->irqs + srcno;
>  
> -trace_xics_set_irq_msi(srcno, srcno + ics->offset);
> +trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset);
>  
>  if (val) {
>  if (irq->priority == 0xff) {
> @@ -469,31 +492,31 @@ static void set_irq_msi(ICSState *ics, int srcno, int 
>

[Qemu-devel] [PATCH v2 8/8] ppc/xics: Split ICS into ics-base and ics class

2016-06-28 Thread Nikunj A Dadhania 
From: Benjamin Herrenschmidt 

The existing implementation remains same and ics-base is introduced. The
type name "ics" is retained, and all the related functions renamed as
ics_simple_*

This will allow different implementations for the source controllers
such as the MSI support of PHB3 on Power8 which uses in-memory state
tables for example.

Signed-off-by: Benjamin Herrenschmidt 
Signed-off-by: Nikunj A Dadhania 
---
 hw/intc/trace-events  |  10 ++--
 hw/intc/xics.c| 146 +++---
 hw/intc/xics_kvm.c|  10 ++--
 hw/intc/xics_spapr.c  |  28 +-
 include/hw/ppc/xics.h |  23 +---
 5 files changed, 132 insertions(+), 85 deletions(-)

diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 5f0f783..e5e7ec7 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -50,12 +50,12 @@ xics_icp_accept(uint32_t old_xirr, uint32_t new_xirr) 
"icp_accept: XIRR %#"PRIx3
 xics_icp_eoi(int server, uint32_t xirr, uint32_t new_xirr) "icp_eoi: server %d 
given XIRR %#"PRIx32" new XIRR %#"PRIx32
 xics_icp_irq(int server, int nr, uint8_t priority) "cpu %d trying to deliver 
irq %#"PRIx32" priority %#x"
 xics_icp_raise(uint32_t xirr, uint8_t pending_priority) "raising IRQ new 
XIRR=%#x new pending priority=%#x"
-xics_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq %#x]"
+xics_ics_simple_set_irq_msi(int srcno, int nr) "set_irq_msi: srcno %d [irq 
%#x]"
 xics_masked_pending(void) "set_irq_msi: masked pending"
-xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq %#x]"
-xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) 
"ics_write_xive: irq %#x [src %d] server %#x prio %#x"
-xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
-xics_ics_eoi(int nr) "ics_eoi: irq %#x"
+xics_ics_simple_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno %d [irq 
%#x]"
+xics_ics_simple_write_xive(int nr, int srcno, int server, uint8_t priority) 
"ics_write_xive: irq %#x [src %d] server %#x prio %#x"
+xics_ics_simple_reject(int nr, int srcno) "reject irq %#x [src %d]"
+xics_ics_simple_eoi(int nr) "ics_eoi: irq %#x"
 xics_alloc(int irq) "irq %d"
 xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d 
irqs, lsi=%d, alignnum %d"
 xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index bbdba84..39928d9 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -112,7 +112,7 @@ void xics_add_ics(XICSState *xics)
 {
 ICSState *ics;
 
-ics = ICS(object_new(TYPE_ICS));
+ics = ICS_SIMPLE(object_new(TYPE_ICS_SIMPLE));
 object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL);
 ics->xics = xics;
 QLIST_INSERT_HEAD(>ics, ics, list);
@@ -223,9 +223,32 @@ static const TypeInfo xics_common_info = {
 #define XISR(ss)   (((ss)->xirr) & XISR_MASK)
 #define CPPR(ss)   (((ss)->xirr) >> 24)
 
-static void ics_reject(ICSState *ics, int nr);
-static void ics_resend(ICSState *ics);
-static void ics_eoi(ICSState *ics, int nr);
+static void ics_reject(ICSState *ics, uint32_t nr)
+{
+ICSStateClass *k = ICS_GET_CLASS(ics);
+
+if (k->reject) {
+k->reject(ics, nr);
+}
+}
+
+static void ics_resend(ICSState *ics)
+{
+ICSStateClass *k = ICS_GET_CLASS(ics);
+
+if (k->resend) {
+k->resend(ics);
+}
+}
+
+static void ics_eoi(ICSState *ics, int nr)
+{
+ICSStateClass *k = ICS_GET_CLASS(ics);
+
+if (k->eoi) {
+k->eoi(ics, nr);
+}
+}
 
 static void icp_check_ipi(ICPState *ss)
 {
@@ -428,7 +451,7 @@ static const TypeInfo icp_info = {
 /*
  * ICS: Source layer
  */
-static void resend_msi(ICSState *ics, int srcno)
+static void ics_simple_resend_msi(ICSState *ics, int srcno)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
@@ -441,7 +464,7 @@ static void resend_msi(ICSState *ics, int srcno)
 }
 }
 
-static void resend_lsi(ICSState *ics, int srcno)
+static void ics_simple_resend_lsi(ICSState *ics, int srcno)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
@@ -453,11 +476,11 @@ static void resend_lsi(ICSState *ics, int srcno)
 }
 }
 
-static void set_irq_msi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_msi(ICSState *ics, int srcno, int val)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
-trace_xics_set_irq_msi(srcno, srcno + ics->offset);
+trace_xics_ics_simple_set_irq_msi(srcno, srcno + ics->offset);
 
 if (val) {
 if (irq->priority == 0xff) {
@@ -469,31 +492,31 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
 }
 }
 
-static void set_irq_lsi(ICSState *ics, int srcno, int val)
+static void ics_simple_set_irq_lsi(ICSState *ics, int srcno, int val)
 {
 ICSIRQState *irq = ics->irqs + srcno;
 
-trace_xics_set_irq_lsi(srcno, srcno + ics->offset);
+trace_xics_ics_simple_set_irq_lsi(srcno, srcno + ics->offset);
 if (val) {
 irq->status