Re: [virtio-dev] [PATCH v2 5/5] virtio-mmio: MSI vector and event mapping

["Liu, Jing2" <jing2.liu@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2678 bytes --]


On 1/29/2020 6:14 PM, Michael S. Tsirkin wrote:
> On Tue, Jan 21, 2020 at 09:54:33PM +0800, Jing Liu wrote:
>> Bit 1 msi_sharing reported in the MsiState register indicates the mapping mode
>> device uses.
>>
>> Bit 1 is 0 - device uses MSI non-sharing mode. This indicates vector per event and
>> fixed static vectors and events relationship. This fits for devices with a high interrupt
>> rate and best performance;
>> Bit 1 is 1 - device uses MSI sharing mode. This indicates vectors and events
>> dynamic mapping and fits for devices not requiring a high interrupt rate.

Thanks for reviewing! Sorry for the late reply.

For msi sharing/non-sharing mode, we are trying to define a rule that,

simply using 1 bit to indicate what device wants and what driver should 
do. Let me try to explain details.

If it's sharing mode (bit=1), it means device doesn't want vector per 
queue or a high interrupt rate.

Driver should meet such request.

So define that, device should support at least 2 msi vectors and driver 
should use

VIRTIO_MMIO_MSI_CMD_MAP_CONFIG/VIRTIO_MMIO_MSI_CMD_MAP_QUEUE commands

to map the configuration change/selected queue events.

In implementation, driver will detect the bit=1, turn to !per_vq_vector 
and map 1 vector for config

and 1 for all queues to meet the request. (Maybe we need restrict this 
in spec too.)

> It seems that sharing mode is a superset of non-sharing mode.

I think sharing mode is not a superset, because driver should not map 
1:1 which will go against

the request from device that it doesn't want 1:1 (non-sharing mode).

In other words, device chooses the way it wants, a high interrupt rate 
with 1:1 or not.

> Isn't that right? E.g. with sharing mode drivers
> can still avoid sharing if they like.
Drivers should not avoid msi sharing if that bit is 1.
> Maybe it should just be a hint to drivers whether to share
> interrupts,
The sharing mode bit is the hint to indicate whether to share interrupts. :)
> instead of a completely different layout?

Except the bit, no other different register layout is there for such 
feature.

Thanks!

Jing

>> diff --git a/msi-state.c b/msi-state.c
>> index b1fa0c1..d470be4 100644
>> --- a/msi-state.c
>> +++ b/msi-state.c
>> @@ -1,4 +1,5 @@
>>   le32 {
>>       msi_enabled : 1;
>> -    reserved : 31;
>> +    msi_sharing: 1;
>> +    reserved : 30;
>>   };
>> -- 
>> 2.7.4
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

[-- Attachment #2: Type: text/html, Size: 4920 bytes --]