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

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, May 16, 2023 12:33 AM

[..]
> > > > 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.
> > >
> > In response to your above suggestion of AQ command, I suggested the
> > eventq that contains the isr_status that reduces latency as you suggest.
> 
> I don't see why we need to keep adding queues though.
> Just use one of admin queues.
> 
Admin queue is cmd->rsp mode.
The purpose is different here. A device wants to async, notify the interrupt for each VF.
Like a nic rq.
Filling up the AQ full of these commands is sub-optimal for those VFs who may generate interrupt.

> > An eventq is a variation of it, where device can keep reporting the events
> without doing the extra mapping and without too many commands.
> 
> I don't get the difference then. The format you showed seems very close to
> admin command. What is the difference? How do you avoid the need to add a
> command per VF using INTx#?
> 
The main difference is it behaves like nic rq.
Driver posts empty descriptors, device reports the consumed entries.
Specific entry is not attached to any specific VF.
The eventq_entry structure is written by the device for a VF there fired the interrupt fired.

> How do you avoid the need to add a command per VF using INTx#?
The empty buffer is posted pointing to eventq_entry.
Total number of buffers posted = number_of_vfs in intx mode.
So entry is not attached to a specific VF, entries are in a pool (like nic rq).

> > Intel CPU has 256 per core (per vcpu). So they are really a lot.
> > One needs to connect lot more devices to the cpu to run out of it.
> > So yes, I would like to try the command to fail.
> 
> order of 128 functions then for a 1vcpu VM. You were previously talking about
> tends of 1000s of functions as justification for avoiding config space.
> 
Will try.

> 
> > > Do specific customers event use guests with msi-x disabled? Maybe no.
> > > Does anyone use virtio with msi-x disabled? Most likely yes.
> > I just feel that INTx emulation is extremely rare/narrow case of some
> applications that may never find its use on hw based devices.
> 
> If we use a dedicated command for this, I guess devices can just avoid
> implementing the command if they do not feel like it?
> 
I didn't follow. The guest device?
Do you mean guest driver?

> > A driver will be easily able to fail the call on INTx configuration failing the
> guest.
> 
> There's no configuration - INTx is the default - and no way to fail gracefully for
> legacy. That is one of the things we should fix, at least hypervisor should be able
> to detect failures.
For sure hypervisor can detect the failure on INTx configuration done by the guest and fail the call for this config.

What we are discussing is not anything legacy specific, it applies to 1.x as well sadly.
So, we don't need to mix INTx support with register emulation proposal, better to handle in the follow up proposal itself.
I also have some patches to improve driver resiliency when guest is not involved to work without INTx.

And INTx can still work from system and device point of view, because INTx generation does not have function id in it.
Existing PCIe INTx can work if PCI PF device can trigger INTx for the VFs.
So eventq scheme is still optional and orthogonal to legacy.

I would like to discuss it separately outside of legacy proposal.