Re: [Qemu-ppc] [Qemu-devel] [PATCH v3 2/8] xics: add pre_save/post_load/cpu_setup dispatchers

[Andreas Färber]

Am 29.08.2013 10:25, schrieb Alexey Kardashevskiy:
> On 08/23/2013 09:38 PM, Andreas Färber wrote:
>> Am 23.08.2013 05:39, schrieb Alexey Kardashevskiy:
>>> On 08/19/2013 11:54 PM, Andreas Färber wrote:
>>>> Am 19.08.2013 07:55, schrieb Alexey Kardashevskiy:
>>>>> The upcoming support of in-kernel XICS will redefine migration callbacks
>>>>> for both ICS and ICP so classes and callback pointers are added.
>>>>>
>>>>> This adds a cpu_setup callback to the XICS device class (as XICS-KVM
>>>>> will do it different) and xics_dispatch_cpu_setup(). This also moves
>>>>> the place where xics_dispatch_cpu_setup() is called a bit further to
>>>>> have VCPU initialized (required for XICS-KVM).
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>>> ---
>>>>> Changes:
>>>>> v3:
>>>>> * fixed local variables names
>>>>> ---
>>>>>  hw/intc/xics.c        | 61 
>>>>> +++++++++++++++++++++++++++++++++++++++++++++++----
>>>>>  hw/ppc/spapr.c        |  4 ++--
>>>>>  include/hw/ppc/xics.h | 46 +++++++++++++++++++++++++++++++++++++-
>>>>>  3 files changed, 104 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>>>>> index 6b3c071..e3a957d 100644
>>>>> --- a/hw/intc/xics.c
>>>>> +++ b/hw/intc/xics.c
>>>> [...]
>>>>> @@ -674,10 +724,12 @@ static Property xics_properties[] = {
>>>>>  static void xics_class_init(ObjectClass *oc, void *data)
>>>>>  {
>>>>>      DeviceClass *dc = DEVICE_CLASS(oc);
>>>>> +    XICSStateClass *k = XICS_CLASS(oc);
>>>>>  
>>>>>      dc->realize = xics_realize;
>>>>>      dc->props = xics_properties;
>>>>>      dc->reset = xics_reset;
>>>>> +    k->cpu_setup = xics_cpu_setup;
>>>>>  
>>>>>      spapr_rtas_register("ibm,set-xive", rtas_set_xive);
>>>>>      spapr_rtas_register("ibm,get-xive", rtas_get_xive);
>>>>
>>>> This hunk is fixed up in 4/8, please squash that bit here.
>>>
>>> Thanks for the review, fixed this.
>>>
>>>
>>>> Otherwise looks good.
>>>
>>> What exactly looks good? This patch only?
>>
>> Yes. And another one has by Rb already I believe. I hope Alex can start
>> cherry-picking some more patches when he gets back to NUE.
> 
>> I know you had some open questions I still need to reply to. Among
>> others, yes, I concur with mst that ppc/spapr_pci.c could be moved to
>> pci-host/spapr.c, but remember to update MAINTAINERS for you guys to
>> still get CC'ed on it then.
>>
>> I had asked Anthony to reply to the KVM XICS patch
>> because he said that
> 
> Do we really expect Anthony to reply? He seems to disappear from this
> conversation and Alex already Rb'ed that patch.
> 
>> accessing the parent's method seemed wrong to him and the parent
>> implement should be put in charge of calling rather than the derived.
>> I.e., I will be dropping my proposed OBJECT_GET_PARENT_CLASS() macro and
>> started redoing several series. Haven't looked into how exactly your
>> code (CPU setup I think?) may need to change.
> 
> 
> xics_dispatch_cpu_setup() should call XICS common's cpu_setup which must
> call then KVM-XICS's cpu_setup? I do not follow you here, sorry.

I assume Anthony meant to have xics_dispatch_cpu_setup() or whatever to
contain the common logic and call cpu_setup hook *only* for the
emulation/KVM specific logic. But that's what he was/is supposed to
explain himself. :) He is CC'ed and if he doesn't reply, ping him.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg