Re: [Qemu-devel] [PATCH v4 5/5] Add param Error ** for msi_init()

[Marcel Apfelbaum <marcel@redhat.com>]

On 04/10/2016 12:38 PM, Cao jin wrote:
>
>
> On 04/10/2016 04:20 PM, Marcel Apfelbaum wrote:
>
>>
>> Hi,
>> I'll let Markus to continue the review, it brings very valuable
>> information,
>> I will only try to answer the questions below.
>>
>>> Several questions on this topic:
>>> 1. How to confirm whether a device model has non-MSI variant? AFAICT,
>>> it is these who have msi property.
>>>
>>
>> MSI is required for PCI Express devices, optional for PCI devices.
>> Even if a PCI device supports MSI, it is strongly advised to support
>> legacy INTx for backward compatibility.
>> Bottom line, as far as I know, almost all PCI devices support legacy
>> interrupts.
>> (an exception is the ivshmem device that requires MSI)
>>
>>> 2. For those have non-MSI variant devices(have msi property), as I see
>>> in the code, they all have it on by default, So we won`t know it is
>>> user order, or user don`t set it at all.
>>>
>>
>> I didn't quite understand the sentence, but some devices have a
>> "use_msi" property that can be set by the user. If no such property exists,
>> we can assume the user "prefers" the msi version.
>>
> Hi,
> Sorry for my bad description. let me explain myself again.
> I think(guess), if a device model has msi property, it means this device model has non-msi variant. For those devices who has msi property, I see most of them will have it on by default. So when these
> devices initialize msi, qemu won`t know whether it is user order or not.
>
> If I understand Markus right:
> 1). If user orders msi on, when msi_init returns -ENOTSUP, It is error. Then I suggest to inform user "set msi=off and try again"

If the user *specifically* asked for msi=on (setting a property) is OK to fail the device init process.

> 2). If user doesn`t order msi on(so device have msi on by default), when msi_init returns -ENOTSUP, I am ok with Markus`s suggestion: *caller should silently switch to the non-MSI variant*
>
> But now the condition is, qemu can`t distinguish whether user ordered msi or not, so for the condition 2) above, my suggestion is the same as 1)
>

If we can't distinguish between the cases, 2 would not be user-friendly. Once it would be possible to differentiate we could go for 2.


Thanks,
Marcel

>>
>>> If user don`t know msi and don`t set it on, I think it is acceptable
>>> to create the non-msi variant for user silently. But if it is user
>>> order, like you said, it is an error.
>>>
>>
>> I am not sure about this. At least a warning should be given IMHO.
>>
>>> So, how about: inform user to swich msi off and try again when
>>> encounter -ENOTSUP, no matter it is user order, or user doesn`t set it
>>> at all?
>>>
>>
>> Not all devices have an "msi" switch. If the board has msi broken and
>> the devices supports legacy interrupts, its OK to continue without MSI.
>>
>>> Actually in this v4, I do checked whether device has a msi property,
>>> like cover-letter said:
>>>
>>>    3. most devices have msi/msix(except vmxnet3 & pvscsi) property as
>>> a   switch, if it has and is switched on, then msi_init() failure
>>> should   results in return directly. So in this version, mptsas
>>> is updated
>>>
>>
>> I don't see a "msi" properties on PCIDevice class or VirtioPCIClass, are
>> you sure we have an msi switch for most of the PCI devices?
>>
>
> My bad, I didn`t limit the range. I mean the devices who will call msi_init, they mostly have msi(or msix, or both) property
>
>
>