Re: [virtio-comment] Re: [PATCH v2 0/2] transport-pci: Introduce legacy registers access using AQ

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, May 08, 2023 at 12:54:55PM -0400, Parav Pandit wrote:
> 
> On 5/7/2023 5:04 AM, Michael S. Tsirkin wrote:
> > 
> > one things I still don't see addressed here is support for
> > legacy interrupts. legacy driver can disable msix and
> > interrupts will be then sent.
> > how about a special command that is used when device would
> > normally send INT#x? it can also return ISR to reduce latency.
> 
> I am not sure if this is a real issue. Because even the legacy guests have
> msix enabled by default. In theory yes, it can fall back to intx.

Well. I feel we should be closer to being sure it's not
an issue if we are going to ignore it.
some actual data here:

Even linux only enabled MSI-X in 2009.
Of course, other guests took longer. E.g.
a quick google search gave me this for some bsd variant (2017):
https://twitter.com/dragonflybsd/status/834494984229421057

Many guests have tunables to disable msix. Why?
E.g. BSD keeps maintaining it at
     hw.virtio.pci.disable_msix
not a real use-case and you know 100% no guests have set this
to work around some bug e.g. in bsd MSI-X core?  How can you be sure?



intx is used when guests run out of legacy interrupts,
these setups are not hard to create at all: just constrain the
number of vCPUs while creating lots of devices.


I could go on.



> There are few options.
> 1. A hypervisor driver can be conservative and steal an msix of the VF for
> transporting intx.
> Pros: Does not need special things in device
> Cons:
> a. Fairly intrusive in hypervisor vf driver.
> b. May not be ever used as guest is unlikely to fail on msix

Yea I do not like this since we are burning up msix vectors.
More reasons: this "pass through" msix has no chance to
set ISR properly since msix does not set ISR.


> 2. Since multiple VFs intx to be serviced, one command per VF in AQ is too
> much overhead that device needs to map a request to,
> 
> A better way is to have an eventq of depth = num_vfs, like many other virtio
> devices have it.
> 
> An eventq can hold per VF interrupt entry including the isr value that you
> suggest above.
> 
> Something like,
> 
> union eventq_entry {
> 	u8 raw_data[16];
> 	struct intx_entry {
> 		u8 event_opcode;
> 		u8 group_type;
> 		u8 reserved[6];
> 		le64 group_identifier;
> 		u8 isr_status;
> 	};
> };
> 
> This eventq resides on the owner parent PF.
> isr_status is read on clear like today.

This is what I wrote no?
lore.kernel.org/all/20230507050146-mutt-send-email-mst%40kernel.org/t.mbox.gz

	how about a special command that is used when device would
	normally send INT#x? it can also return ISR to reduce latency.



> May be such eventq can be useful in future for wider case.

There's no maybe here is there? Things like live migration
need events for sure.

> We may have to find a different name for it as other devices has device
> specific eventq.

We don't need a special name for it. Just use an adminq
with a special command that is only consumed when there is an event.
Note you only need to queue a command if MSI is disabled.
Which is nice.




> I am inclined to differ this to a later point if one can identify the real
> failure with msix for the guest VM.
> So far we don't see this ever happening.

What is the question exactly?

Just have more devices than vectors,
an intel CPU only has ~200 of these, and current drivers
want to use 2 vectors and then fall back on INTx since that is shared.
Extremely easy to create - do you want a qemu command line to try?

Do specific customers event use guests with msi-x disabled? Maybe no.
Does anyone use virtio with msi-x disabled? Most likely yes.
So if we are going for legacy pci emulation let's have
a comprehensive legacy pci emulation please where
host can either enable it for a guest or deny completely,
not kind of start running then fail mysteriously.


-- 
MST