MSI allocation regression, still to be corrected in HEAD and please MFC before release/12.0 gets branched

[Scott Long]

> On Nov 12, 2018, at 10:03 AM, Harry Schmalzbauer <freebsd at omnilan.de> wrote:
> 
> Am 11.06.2018 um 20:28 schrieb Harry Schmalzbauer:
>> Am 05.06.2018 um 19:54 schrieb Scott Long:
>> …
>>>>>> Late in the 11.2 phase, I identified this commit as a regression for MSI (non-x) alloctaion.
>>>>>> I have an idea what probably causes the problem here (INTx allocation, although MSI (and MSI-x) capability):
>>>>>> disable_msix is not 0 (I need to disable MSI-x because of ESXi-passthru…).
>>>>>> 
>>>>>> Corresponding lines:
>>>>>> {
>>>>>>          device_t dev;
>>>>>>          int error, msgs;
>>>>>> 
>>>>>>          dev = sc->mps_dev;
>>>>>>          error = 0;
>>>>>>          msgs = 0;
>>>>>> 
>>>>>>          if ((sc->disable_msix == 0) &&
>>>>>>              ((msgs = pci_msix_count(dev)) >= MPS_MSI_COUNT))
>>>>>>                  error = mps_alloc_msix(sc, MPS_MSI_COUNT);
>>>>>>          if ((error != 0) && (sc->disable_msi == 0) &&
>>>>>>              ((msgs = pci_msi_count(dev)) >= MPS_MSI_COUNT))
>>>>>>                  error = mps_alloc_msi(sc, MPS_MSI_COUNT);
>>>>>>          if (error != 0)
>>>>>>                  msgs = 0;
>>>>>> 
>>>>>>          sc->msi_msgs = msgs;
>>>>>>          return (error);
>>>>>> }
>>>>>> 
> …
>>>>> Hi Harry,
>>>>> You are correct about the bug.  Please change the line at the top of the function that reads
>>>>> error = 0;
>>>>> to
>>>>> error = ENXIO;
>>>>> Let me know if that fixes the MSI problem for you.
>> 
>> …
>> 
> …
>> Index: src/sys/dev/mps/mps_pci.c
>> ===================================================================
>> --- sys/dev/mps/mps_pci.c   (Revision 334948)
>> +++ sys/dev/mps/mps_pci.c   (Arbeitskopie)
>> @@ -244,7 +244,7 @@
>>         int error, msgs;
>> 
>>         dev = sc->mps_dev;
>> -       error = 0;
>> +       error = ENXIO;
>>         msgs = 0;
>> 
>>         if ((sc->disable_msix == 0) &&
>> 
> 
> To my understanding, it's obvious that the way mps_pci_alloc_interrupts() currently works is unintended.
> This might not affect too many people, but is there a reason not to fix it?
> 
> I already created a coresponding problem report: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=229267
> Anything else I should do?
> 

Hi Harry,

Sorry for ignoring this for so long.  I’m going to commit a fix today, but it won’t be the same one-line change.
Upon reviewing the code, I’d going to refactor it so it’s not so confusing and prone to these kinds of mistakes.
Thank you for the continued reminders to finish this.

Scott