[RFC,v2,09/21] ppc/xive: extend the interrupt presenter model for XIVE

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.

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 <clg@kaod.org>
---
 hw/intc/xics.c        | 4 ++++
 include/hw/ppc/xics.h | 6 ++++++
 2 files changed, 10 insertions(+)

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.
The TIMA really needs to be managed by a different object than the ICP.

> 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 <clg@kaod.org>
> ---
>  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"

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 <clg@kaod.org>
>> ---
>>  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"
>

On Tue, Sep 19, 2017 at 09:28:45PM +0200, Cédric Le Goater wrote:
> 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 ? 

Or individual TIMA objects which the cpus point to using their intc
pointers.

> 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.

No, it shouldn't be migrated.  It's set up during machine construction.

On 09/22/2017 12:58 PM, David Gibson wrote:
> On Tue, Sep 19, 2017 at 09:28:45PM +0200, Cédric Le Goater wrote:
>> 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 ? 
> 
> Or individual TIMA objects which the cpus point to using their intc
> pointers.

ah ok. We really are splitting the two worlds.

C.
 
>> 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.
> 
> No, it shouldn't be migrated.  It's set up during machine construction.
>