Cédric Le Goater <c...@kaod.org> writes: > On 09/20/2016 08:29 AM, Nikunj A Dadhania wrote: >> Cédric Le Goater <c...@kaod.org> writes: >> >>> On 09/20/2016 08:02 AM, Nikunj A Dadhania wrote: >>>> Cédric Le Goater <c...@kaod.org> writes: >>>> >>>>> On 09/19/2016 08:29 AM, Nikunj A Dadhania wrote: >>>>>> From: Benjamin Herrenschmidt <b...@kernel.crashing.org> >>>>>> >>>>>> 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. >>>>> >>>>> A couple of issues below regarding the class helpers, >>>>> >>>>>> Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> >>>>>> Signed-off-by: Nikunj A Dadhania <nik...@linux.vnet.ibm.com> >>>>>> --- >>>>>> hw/intc/trace-events | 10 ++-- >>>>>> hw/intc/xics.c | 143 >>>>>> +++++++++++++++++++++++++++++++------------------- >>>>>> hw/intc/xics_kvm.c | 10 ++-- >>>>>> hw/intc/xics_spapr.c | 28 +++++----- >>>>>> include/hw/ppc/xics.h | 23 +++++--- >>>>>> 5 files changed, 131 insertions(+), 83 deletions(-) > >>>>>> @@ -224,9 +224,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, int server); >>>>>> -static void ics_eoi(ICSState *ics, int nr); >>>>>> +static void ics_reject(ICSState *ics, uint32_t nr) >>>>>> +{ >>>>>> + ICSStateClass *k = ICS_GET_CLASS(ics); >>>>> >>>>> Shouldn't that be ICS_BASE_GET_CLASS() >>>> >>>> The class hierarchy is something like this: >>>> >>>> ICS_BASE -> ICS_SIMPLE -> ICS_KVM >>> >>> yes. but if we called ics_* with an instance of an ics class which is >>> not a ICS_SIMPLE class that will break. >> >> Correct >> >>> ICSStateClass is the baseclass, whenever we call methods on a ICSState* >>> object, we should use the method defines in ICSStateClass. >> >> Hmm, in that case I need to initialize base class methods in >> instance_init of ics_simple > > yes but this is done, no ? I see : > > static void ics_simple_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > ICSStateClass *isc = ICS_BASE_CLASS(klass);
Right. Currently, we have this: + ICSStateClass *isc = ICS_CLASS(klass); > > dc->realize = ics_simple_realize; > dc->vmsd = &vmstate_ics_simple; > dc->reset = ics_simple_reset; > isc->post_load = ics_simple_post_load; > isc->reject = ics_simple_reject; > isc->resend = ics_simple_resend; > isc->eoi = ics_simple_eoi; Regards Nikunj