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);
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;
}
Thanks,
C.
