On 11/23/2017 12:07 PM, David Gibson wrote:
> On Fri, Nov 17, 2017 at 08:16:47AM +0100, Cédric Le Goater wrote:
>> On 11/17/2017 05:48 AM, David Gibson wrote:
>>> On Fri, Nov 10, 2017 at 03:20:09PM +0000, Cédric Le Goater wrote:
>>>> Currently, the ICSState 'ics' object of the sPAPR machine acts as the
>>>> global interrupt source handler and also as the IRQ number allocator
>>>> for the machine. Some IRQ numbers are allocated very early in the
>>>> machine initialization sequence to populate the device tree, and this
>>>> is a problem to introduce the new POWER XIVE interrupt model, as it
>>>> needs to share the IRQ numbers with the older model.
>>>>
>>>> To prepare ground for XIVE, here is a set of new XICSFabric operations
>>>> to let the machine handle directly the IRQ number allocation and to
>>>> decorrelate the allocation from the interrupt source object :
>>>>
>>>> bool (*irq_test)(XICSFabric *xi, int irq);
>>>> int (*irq_alloc_block)(XICSFabric *xi, int count, int align);
>>>> void (*irq_free_block)(XICSFabric *xi, int irq, int num);
>>>>
>>>> In these prototypes, the 'irq' parameter refers to a number in the
>>>> global IRQ number space. Indexes for arrays storing different state
>>>> informations on the interrupts, like the ICSIRQState, are usually
>>>> named 'srcno'.
>>>>
>>>> Signed-off-by: Cédric Le Goater <c...@kaod.org>
>>>
>>> This doesn't seem sensible to me. When I said you should move irq
>>> allocation to the machine, I mean actually move the code. The only
>>> user of irq allocation should be in the machine, so we shouldn't need
>>> to indirect via the XICSFabric interface to do that.
>>
>> OK. so we can probably do the same with machine class handlers because
>> we do need an indirection to handle the way older pseries machines
>> allocate IRQs that will change with newer machines supporting XIVE.
>
> Right. You could do it either with a MachineClass callback (similar
> to the phb placement one), or with just a flag in the MachineClass
> that's checked explicitly. I'd be ok with either approach.
I have changed the approach in the latest XIVE patchset and I am not
using any handlers of any sort anymore. It does not seem necessary
but if it is, you will have a global view of what XIVE requires to
decide with direction to take.
I will send the patchset later in the day.
Thanks,
C.
>>> And, we shouldn't be using XICSFabric things for XIVE.
>>
>> ok. The spapr machine should be enough.
>>
>> Thanks,
>>
>> C.
>>
>>>> ---
>>>> hw/ppc/spapr.c | 19 +++++++++++++++++++
>>>> include/hw/ppc/xics.h | 4 ++++
>>>> 2 files changed, 23 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index a2dcbee07214..84d68f2fdbae 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -3536,6 +3536,21 @@ static ICPState *spapr_icp_get(XICSFabric *xi, int
>>>> vcpu_id)
>>>> return cpu ? ICP(cpu->intc) : NULL;
>>>> }
>>>>
>>>> +static bool spapr_irq_test(XICSFabric *xi, int irq)
>>>> +{
>>>> + return false;
>>>> +}
>>>> +
>>>> +static int spapr_irq_alloc_block(XICSFabric *xi, int count, int align)
>>>> +{
>>>> + return -1;
>>>> +}
>>>> +
>>>> +static void spapr_irq_free_block(XICSFabric *xi, int irq, int num)
>>>> +{
>>>> + ;
>>>> +}
>>>> +
>>>> static void spapr_pic_print_info(InterruptStatsProvider *obj,
>>>> Monitor *mon)
>>>> {
>>>> @@ -3630,6 +3645,10 @@ static void spapr_machine_class_init(ObjectClass
>>>> *oc, void *data)
>>>> xic->ics_get = spapr_ics_get;
>>>> xic->ics_resend = spapr_ics_resend;
>>>> xic->icp_get = spapr_icp_get;
>>>> + xic->irq_test = spapr_irq_test;
>>>> + xic->irq_alloc_block = spapr_irq_alloc_block;
>>>> + xic->irq_free_block = spapr_irq_free_block;
>>>> +
>>>> ispc->print_info = spapr_pic_print_info;
>>>> /* Force NUMA node memory size to be a multiple of
>>>> * SPAPR_MEMORY_BLOCK_SIZE (256M) since that's the granularity
>>>> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
>>>> index 28d248abad61..30e7f2e0a7dd 100644
>>>> --- a/include/hw/ppc/xics.h
>>>> +++ b/include/hw/ppc/xics.h
>>>> @@ -175,6 +175,10 @@ typedef struct XICSFabricClass {
>>>> ICSState *(*ics_get)(XICSFabric *xi, int irq);
>>>> void (*ics_resend)(XICSFabric *xi);
>>>> ICPState *(*icp_get)(XICSFabric *xi, int server);
>>>> + /* IRQ allocator helpers */
>>>> + bool (*irq_test)(XICSFabric *xi, int irq);
>>>> + int (*irq_alloc_block)(XICSFabric *xi, int count, int align);
>>>> + void (*irq_free_block)(XICSFabric *xi, int irq, int num);
>>>> } XICSFabricClass;
>>>>
>>>> #define XICS_IRQS_SPAPR 1024
>>>
>>
>
