Re: [Qemu-devel] [RFC PATCH v2 09/21] ppc/xive: extend the interrupt presenter model for XIVE

[Cédric Le Goater]

On 09/19/2017 09:36 AM, David Gibson wrote:
> On Mon, Sep 11, 2017 at 07:12:23PM +0200, Cédric Le Goater wrote:
>> The XIVE interrupt presenter exposes a set of Thread Interrupt
>> Management Areas, also called rings, one per different level of
>> privilege (four in all). This area is used to handle priority
>> management and interrupt acknowledgment among other things.
>>
>> We extend the ICPState object with a cache of the register data for
>> XIVE. The integration with the sPAPR machine is much easier and we
>> need a common framework to switch from one controller model to
>> another: XICS <-> XIVE.
> 
> This sounds like an even worse idea than referencing the ICS state.

ok ok.

> The TIMA really needs to be managed by a different object than the ICP.

like an array under the machine indexed by the cpu index ? 

at some point, we will need to :

    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
    ICPState *icp = ICP(cpu->intc);

and 

    icp = xics_icp_get(xive->ics->xics, target);


isn't the cpu->intc pointer  the best option to hold that information ? 
and it is migrated.

C. 


>> The next patch will introduce the MMIO handlers to interact with the
>> TIMA, OS only, which is required for the sPAPR support.
>>
>> Signed-off-by: Cédric Le Goater <address@hidden>
>> ---
>>  hw/intc/xics.c        | 4 ++++
>>  include/hw/ppc/xics.h | 6 ++++++
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>> index a84ba51ad8ff..927d4fec966a 100644
>> --- a/hw/intc/xics.c
>> +++ b/hw/intc/xics.c
>> @@ -274,6 +274,7 @@ static const VMStateDescription vmstate_icp_server = {
>>          VMSTATE_UINT32(xirr, ICPState),
>>          VMSTATE_UINT8(pending_priority, ICPState),
>>          VMSTATE_UINT8(mfrr, ICPState),
>> +        VMSTATE_UINT8_ARRAY(tima, ICPState, 0x40),
>>          VMSTATE_END_OF_LIST()
>>      },
>>  };
>> @@ -293,6 +294,7 @@ static void icp_reset(void *dev)
>>      if (icpc->reset) {
>>          icpc->reset(icp);
>>      }
>> +    memset(icp->tima, 0, sizeof(icp->tima));
>>  }
>>  
>>  static void icp_realize(DeviceState *dev, Error **errp)
>> @@ -343,6 +345,8 @@ static void icp_realize(DeviceState *dev, Error **errp)
>>          icpc->realize(icp, errp);
>>      }
>>  
>> +    icp->tima_os = &icp->tima[0x10];
>> +
>>      qemu_register_reset(icp_reset, dev);
>>      vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
>>  }
>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>> index 28d248abad61..c835997303c4 100644
>> --- a/include/hw/ppc/xics.h
>> +++ b/include/hw/ppc/xics.h
>> @@ -83,6 +83,12 @@ struct ICPState {
>>      qemu_irq output;
>>  
>>      XICSFabric *xics;
>> +
>> +    /* XIVE section */
>> +#define XIVE_TM_RING_COUNT 4
>> +
>> +    uint8_t tima[XIVE_TM_RING_COUNT * 0x10];
>> +    uint8_t *tima_os;
>>  };
>>  
>>  #define ICP_PROP_XICS "xics"
>