Re: [Qemu-devel] [PATCH v2 4/8] spapr: Move interrupt allocator to xics

[Alexander Graf <agraf@suse.de>]

On 21.05.14 10:46, Alexey Kardashevskiy wrote:
> On 05/21/2014 06:34 PM, Alexander Graf wrote:
>> On 15.05.14 11:59, Alexey Kardashevskiy wrote:
>>> The current allocator returns IRQ numbers from a pool and does not
>>> support IRQs reuse in any form as it did not keep track of what it
>>> previously returned, it only keeps the last returned IRQ. Some use
>>> cases such as PCI hot(un)plug may require IRQ release and reallocation.
>>>
>>> This moves an allocator from SPAPR to XICS.
>>>
>>> This switches IRQ users to use new API.
>>>
>>> This uses LSI/MSI flags to know if interrupt is allocated.
>>>
>>> The interrupt release function will be posted as a separate patch.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>    hw/intc/xics.c         | 88
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>    hw/ppc/spapr.c         | 67 --------------------------------------
>>>    hw/ppc/spapr_events.c  |  2 +-
>>>    hw/ppc/spapr_pci.c     |  6 ++--
>>>    hw/ppc/spapr_vio.c     |  2 +-
>>>    include/hw/ppc/spapr.h | 10 ------
>>>    include/hw/ppc/xics.h  |  2 ++
>>>    trace-events           |  4 +++
>>>    8 files changed, 99 insertions(+), 82 deletions(-)
>>>
>>> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
>>> index 83a809e..fdcbb3a 100644
>>> --- a/hw/intc/xics.c
>>> +++ b/hw/intc/xics.c
>>> @@ -689,6 +689,94 @@ void xics_set_irq_type(XICSState *icp, int irq, bool
>>> lsi)
>>>        ics_set_irq_type(ics, irq - ics->offset, lsi);
>>>    }
>>>    +#define ICS_IRQ_FREE(ics, srcno)   \
>>> +    (!((ics)->irqs[(srcno)].flags & (XICS_FLAGS_IRQ_MASK)))
>>> +
>>> +static int ics_find_free_block(ICSState *ics, int num, int alignnum)
>>> +{
>>> +    int first, i;
>>> +
>>> +    for (first = 0; first < ics->nr_irqs; first += alignnum) {
>>> +        if (num > (ics->nr_irqs - first)) {
>>> +            return -1;
>>> +        }
>>> +        for (i = first; i < first + num; ++i) {
>>> +            if (!ICS_IRQ_FREE(ics, i)) {
>>> +                break;
>>> +            }
>>> +        }
>>> +        if (i == (first + num)) {
>>> +            return first;
>>> +        }
>>> +    }
>>> +
>>> +    return -1;
>>> +}
>>> +
>>> +int xics_alloc(XICSState *icp, int src, int irq_hint, bool lsi)
>>> +{
>>> +    ICSState *ics = &icp->ics[src];
>>> +    int irq;
>>> +
>>> +    if (irq_hint) {
>>> +        assert(src == xics_find_source(icp, irq_hint));
>> Could this ever get triggered by a guest? Why don't we just fail as if the
>> IRQ wasn't available?
> The @irq_hist is only used when the user specified IRQ in the command line,
> this is why it is assert. And the guest never gets to choose IRQ number, it
> receives numbers either via device tree (and QEMU puts those numbers there,
> not SLOF) or ibm,change-msi (QEMU returns numbers).

Ok, it's safe then :).


Alex