Re: [Qemu-ppc] [Qemu-devel] [PATCH 1/6] xics: add flags for interrupts

[Alexey Kardashevskiy]

On 05/07/2014 11:09 PM, Mike Day wrote:
> 
>>  
>>      for (i = 0; i < ics->nr_irqs; i++) {
>>          /* FIXME: filter by server#? */
>> -        if (ics->islsi[i]) {
>> +        if (ics->irqs[i].flags & XICS_FLAGS_LSI) {
>>              resend_lsi(ics, i);
> 
> Not part of your patch, but I'm curious about this FIXME (there's an
> identical FIXME in resend_msi). Has this proved to be a problem?

ics_resend -> resend_lsi() -> icp_irq() and the last one delivers interrupt
to the correct server. I should probably remove that FIXME or I am missing
something here.

> With
> these patches you could have many unallocated interrupts in array AFTER
> the last allocated interrupt, correct?

I would have unallocated interrupts before this patch too.

> In that case, the loop would
> continue beyond the last allocated interrupt for no purpose. 

For LSI we check the type, if it is not, it is assumed MSI and then
resend_msi() would check for XICS_STATUS_REJECTED which is not set for
non-allocated interrupt so we are safe.

But since I can tell now if it is allocated or not, I will fix it to call
resend_msi() on only if irq is allocated as MSI.

> There are
> a couple ways to mitigate this type of situation by using alternative
> data structures to inform the loop traversal. I don't know if it is
> worth the effort, though.

Here I lost you :)

btw I just realized that in patch#2 it should be xics_find_source instead
of xics_find_server. There are many interrupt servers already and just one
interrupt source (we could have many like one per PHB or something like
that but we are not there yet), this is what I meant.


> 
> 
>> +/* @flags == 0 measn the interrupt is not allocated */
>> +#define XICS_FLAGS_LSI                 0x1
>> +#define XICS_FLAGS_MSI                 0x2
> 
> (nit) typo in the above comment
> 
> Mike
> 


-- 
Alexey