[illumos-Developer] webrev: #725 apic_grp_set_cpu misorders checkfor FIXED interrupts

[Garrett D'Amore]

I think by definition PSM_INTR_OP_GRP_SET_CPU is only supposed to be called for real interrupt groups which exist only for MSI.  It is possible that we couls be more liberal here but I need to think through the ramifications of that.

Looking back at the original code, maybe that was the original intention.  Perhaps all that is needed here is a comment explaining the logic rather than a code change.

  -- Garrett D'Amore

On Jun 18, 2011, at 7:28 PM, Albert Lee <trisk at opensolaris.org> wrote:

> apic_set_cpu can still be entered directly through the apic_intr_ops
> dispatch for PSM_INTR_OP_SET_CPU. It already contains a check for
> irqp->airq_mps_intr_index == MSI_INDEX, but only to reject the request
> if the interrupt is MSI *and* part of an interrupt group (presumably
> because interrupt groups only exist in MSI).
> 
> I assume reassignment of a single non-MSI interrupt is allowed since
> programmable IO APICs predate MSI, so is it really erroneous to use
> PSM_INTR_OP_GRP_SET_CPU for a single non-MSI interrupt (when
> PSM_INTR_OP_SET_CPU would always succeed)?
> 
> -Albert
> 
> On Sat, Jun 18, 2011 at 3:47 PM, Dan Kruchinin <dkruchinin at acm.org> wrote:
>> On Sat, Jun 18, 2011 at 11:03 PM, Albert Lee <trisk at opensolaris.org> wrote:
>>> Dan, if you don't mind, I'd like to have a second person review this
>>> before it gets approved -- I can do the review myself later today.
>> 
>> Yes, sure, it'd be great.
>> 
>>> 
>>> -Albert
>>> 
>>> On Fri, May 13, 2011 at 1:09 AM, Garrett D'Amore <garrett at nexenta.com> wrote:
>>>> I think I already responded but this looks good.
>>>> 
>>>>  -- Garrett D'Amore
>>>> 
>>>> On May 10, 2011, at 5:02 AM, "Dan Kruchinin" <dkruchinin at acm.org> wrote:
>>>> 
>>>>> This webrev fixes a problem when apic_set_cpu() function is called for
>>>>> FIXED interrupt bypassing MSI checks
>>>>> in apic_grp_set_cpu(). CPU changing has sense for MIS/MSI-X interrupts
>>>>> only, so we should not allow it for
>>>>> FIXED interrupts. The patch checks at first if we're working with MSI
>>>>> interrupts. If it's not, exit from function
>>>>> setting *result to ENXIO.
>>>>> 
>>>>> http://cr.illumos.org/view/2jmd24zk/illumos-725-webrev/
>>>>> 
>>>>> 
>>>>> --
>>>>> W.B.R.
>>>>> Dan Kruchinin
>>>>> 
>>>>> _______________________________________________
>>>>> Developer mailing list
>>>>> Developer at lists.illumos.org
>>>>> http://lists.illumos.org/m/listinfo/developer
>>>> 
>>>> _______________________________________________
>>>> Developer mailing list
>>>> Developer at lists.illumos.org
>>>> http://lists.illumos.org/m/listinfo/developer
>>>> 
>>> 
>> 
>> 
>> 
>> --
>> W.B.R.
>> Dan Kruchinin
>> 
> 
> _______________________________________________
> Developer mailing list
> Developer at lists.illumos.org
> http://lists.illumos.org/m/listinfo/developer