Re: [Qemu-devel] [PATCH v1 06/11] ppc/xics: Make the ICSState a list

[Nikunj A Dadhania]

David Gibson <address@hidden> writes:

> [ Unknown signature status ]
> On Mon, Jun 27, 2016 at 10:26:42AM +0530, Nikunj A Dadhania wrote:
>> David Gibson <address@hidden> writes:
>> 
>> > [ Unknown signature status ]
>> > On Thu, Jun 23, 2016 at 11:17:25PM +0530, Nikunj A Dadhania wrote:
>> >> From: Benjamin Herrenschmidt <address@hidden>
>> >> 
>> >> Instead of an array of fixed sized blocks, use a list, as we will need
>> >> to have sources with variable number of interrupts. SPAPR only uses
>> >> a single entry. Native will create more. If performance becomes an
>> >> issue we can add some hashed lookup but for now this will do fine.
>> >> 
>> >> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
>> >> Signed-off-by: Nikunj A Dadhania <address@hidden>
>> >> ---
>> >>  hw/intc/xics.c        | 78 
>> >> +++++++++++++++++++++++++++---------------------
>> >>  hw/intc/xics_kvm.c    | 29 +++++++++++++-----
>> >>  hw/intc/xics_spapr.c  | 82 
>> >> +++++++++++++++++++++++++++++----------------------
>> >>  hw/ppc/spapr_events.c |  2 +-
>> >>  hw/ppc/spapr_pci.c    |  5 ++--
>> >>  hw/ppc/spapr_vio.c    |  2 +-
>> >>  include/hw/ppc/xics.h | 13 ++++----
>> >>  7 files changed, 124 insertions(+), 87 deletions(-)
>> >> 
>> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> >> index 38e51fc..ef2a1e4 100644
>> >> --- a/hw/intc/xics.c
>> >> +++ b/hw/intc/xics.c
>> >> @@ -96,13 +96,16 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
>> >>  static void xics_common_reset(DeviceState *d)
>> >>  {
>> >>      XICSState *xics = XICS_COMMON(d);
>> >> +    ICSState *ics;
>> >>      int i;
>> >>  
>> >>      for (i = 0; i < xics->nr_servers; i++) {
>> >>          device_reset(DEVICE(&xics->ss[i]));
>> >>      }
>> >>  
>> >> -    device_reset(DEVICE(xics->ics));
>> >> +    QLIST_FOREACH(ics, &xics->ics, list) {
>> >> +        device_reset(DEVICE(ics));
>> >> +    }
>> >>  }
>> >>  
>> >>  static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char 
>> >> *name,
>> >> @@ -134,7 +137,6 @@ static void xics_prop_set_nr_irqs(Object *obj, 
>> >> Visitor *v, const char *name,
>> >>      }
>> >>  
>> >>      assert(info->set_nr_irqs);
>> >> -    assert(xics->ics);
>> >>      info->set_nr_irqs(xics, value, errp);
>> >>  }
>> >>  
>> >> @@ -212,33 +214,35 @@ static void ics_reject(ICSState *ics, int nr);
>> >>  static void ics_resend(ICSState *ics);
>> >>  static void ics_eoi(ICSState *ics, int nr);
>> >>  
>> >> -static void icp_check_ipi(XICSState *xics, int server)
>> >> +static void icp_check_ipi(ICPState *ss, int server)
>> >
>> > Since you're now passing ICPState, you don't need the server
>> > parameter, since its only purpose is to locate the right ICPState in
>> > the XICSState.
>> 
>> Right, i had retained this as there was a trace function using it. Maybe
>> we can then move the trace to the caller.
>
> Or just change the trace template.

Sure.

Regards
Nikunj