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

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

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
> >       
>