Re: [Qemu-ppc] [PATCH v4 6/9] ppc/xics: Split ICS into ics-base and ics class

[Cédric Le Goater]

On 09/20/2016 11:41 AM, Nikunj A Dadhania wrote:
> Cédric Le Goater <address@hidden> writes:
>> On 09/20/2016 10:10 AM, Nikunj A Dadhania wrote:
>>>>>>>>> -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);
>>
>> oh yes and same in the kvm ICS. The name are bit confusing as we are 
>> introducing SIMPLE but not the associated macros.
>>
>> You can check on my 2.8 github branch, I got something working there :
>>
>>      
>> https://github.com/legoater/qemu/commit/d0b492707debc7192f0860e2c5fc2daa9fd453ac
> 
> Cool. :-)
> 
> Thanks for testing.

I did some tests for pnv, pseries tcg and kvm. 

It didn't see any migration issues, yet. Do you have some scenario 
to reproduce the fix you are trying to add ? That's under tcg with
a e1000 ? 

Thanks,

C.