Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 3/9] ppc/xics: Make the ICSState a list

[Nikunj A Dadhania]

Cédric Le Goater <address@hidden> writes:

> On 09/19/2016 09:02 AM, Nikunj A Dadhania wrote:
>> Cédric Le Goater <address@hidden> writes:
>> 
>>>  
>>>> +static int icp_post_load(ICPState *ss, int version_id)
>>>> +{
>>>> +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>>>> +    XICSState *xics = spapr->xics;
>>>> +    uint32_t irq, i;
>>>> +    static uint32_t server_no;
>>>> +
>>>> +    server_no++;
>>>> +    irq = XISR(ss);
>>>> +    if (!ss->cs || !irq) {
>>>> +        goto icpend;
>>>> +    }
>>>> +
>>>> +    /* Restore the xirr_owner */
>>>> +    ss->xirr_owner = xics_find_source(xics, irq);
>>>> +
>>>> + icpend:
>>>> +    trace_xics_icp_post_load(server_no, ss->xirr, 
>>>> (uint64_t)ss->xirr_owner,
>>>> +                             ss->pending_priority);
>>>> +    if (xics->nr_servers != server_no) {
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    /* All the ICPs are processed, now restore all the state */
>>>> +    for (i = 0; i < xics->nr_servers; i++) {
>>>> +        icp_resend(xics, i);
>>>> +    }
>>>> +    server_no = 0;
>>>> +    return 0;
>>>> +}
>>>
>>> Is the issue this change is trying to fix related to ICSState becoming 
>>> a list ? If not, It should be in another patch I think.
>> 
>> Yes, and we introduced xirr_owner to optimize. This needs to regenerated
>> at the destination after migration.
>
> Ah. this is because of the previous patch. ok. I am not sure 
> I understand the complete issue but it would better if it was 
> a bit more isolated. This patch is mixing different things and 
> doing in xics.c :
>
>       #include "hw/ppc/spapr.h"
>
> seems wrong.

Not sure, Why?

> I think David suggested to redesign the xics migration state.

That is not needed, as I have solved the problem in the previous patch.
The migration changes was needed for the issue that I had reported.

Regards
Nikunj