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

[Parav Pandit <parav@nvidia.com>]

> From: Jason Wang <jasowang@redhat.com>
> Sent: Monday, May 15, 2023 3:11 AM

> > It is just bunch of admin commands in the new SIOV or virtual virtio device
> chapter.
> > It is not a transport chapter.
> 
> 
> Provisioning could be a part of the transport. For example SR-IOV allows static
> and dynamic provisioning. If a virtio device needs to be implemented in VF, it
> needs to be provisioned first.
> 
Provisioning a device by admin != transport guest driver <-> device communication.

Two very different things.

> 
> >
> >>> And the motivation is also clear is to provide composing a virtio
> >>> device for the
> >> guest VM for the backward compatibility for 1.x part of the specification.
> >>> It seems fine and indeed orthogonal to me that: it is for backward
> >> compatibility for already defined config fields for existing guest VM driver.
> >>> It does not conflict with the cfgq/cmdq idea whose main purpose is
> >>> for the
> >> new config fields, new use cases that doesn't require any mediation.
> >>> Such cfgq works across PF, VF, SF/SIOV devices in uniform way
> >>> without
> >> mediation.
> >>
> >> My understanding is that the cfgq/cmdq could be done on top, since
> >> the commands are part of transport unless I miss something.
> >>
> > On top of what?
> > cfgq/cmdq is just another queue like any other VQ.
> > it is orthogonal to accessing 1.x registers using group owner's queue.
> 
> 
> I think there hasn't been any proposal that call any virtqueue like cfgq/cmdq. So
> using that may cause a lot of misunderstanding. I think the context here is to
> provide or extend transport facilities via a virtqueue not something like the
> control virtqueue.
The context is to compose a PCI device for a guest which for the currently defined features.
Once a new attribute is done using cfgq/cmdq there is no problem of transporting it via its group member.

And therefore, saying any new attribute also to ride on same tranportq/aq via is not appropriate.

> 
> Admin virtqueue doesn't preventing anything from letting IMS go directly to the
> device.
But there is no aq/cfgq/cmdq defined for the guest, so with current proposal it is prevented.

> 
> 
> >   Long discussion with Thomas on IMS topic.
> > I will avoid diverting to unrelated discussion for now.
> >
> >>> Or proposed command in [1] should be tagged as siov_r1, then things will
> be
> >> cleaner.
> >>
> >> Maybe we can say, one of the implementations is siov_r1, since it can be
> used
> >> to implement devices in other transport. One of the main motivations for
> >> transport virtqueue is to reduce the transport specific resources to ease the
> >> virtio device implementation.
> >>
> > Yes, the main motivation as for backward compatibility for the currently
> defined features.
> >
> >>> With that I don't see legacy 3 commands anyway conflict with [1].
> >> It doesn't, but it's not elegant since:
> >>
> > I donât see how making 3 commands to 9 commands makes it elegant by
> doing bisecting of registers offset in hypervisor.
> 
> 
> Or it needs to be done by the hardware and cross register write/read
> needs to be handled there as well.
> 
That is the limitation of legacy and device can always _decide_ when to apply the parameter later when enough bits are written.
Given its control path, it is not an overhead.

> 
> > Its same thing using more opcodes.
> 
> 
> Do we really care about the number of opcodes? I think we've reserved
> sufficient spaces.
> 
I donât see opcode as problem, we have plenty.
And hence, when there is a device that _does not implement config space for SIOV/SF, it should be introduced.
It is the introduction and bisection at the hypervisor level unnecessary.

And I donât have object to it either, the main reason is: I donât see it being useful for 1.x access going forward.

> 
> >
> >> 1) Modern transport, admin virtqueue with well defined transport
> commands
> >> 2) Legacy transport, works moe like a PCI transport on top of the admin
> >> virtqueue
> >>
> >> Transport virtqueue tend to be transport independent, but 2) tie it somehow
> to
> >> the PCI. That's why I suggest to
> >>
> > The 4 patches are not transporting all the PCI things over transport VQ. it is
> not a transport.
> 
> 
> It really depends on the different viewpoint. 

You wrote/asked for "PCI transport over admin q".
That means PCI capabilities, PCI config registers and all things defined in the PCI transport section to go over the AQ.
This is clearly not the objective of these 2 patches.
Nor it is your proposal of transport VQ.

You only want to exchange currently defined config registers, interrupts and notification configuration using AQ.

Transport VQ is NOT transporting actual data, it is not transporting notifications etc.
It is just a config channel.
Hence, they are just commands not a "full transport".

> To me, adminq is not
> specific to PCI, so this proposal looks more like a partial transport
> for legacy.
It is not a partial transport for legacy.
A future transport may be able to do for SIOV/MMIO by adding those group member identifier.

Hence, I agreed that commands in the tranportvq proposal is fine, as commands, not as new transport.

> 
> So we have
> 
> 1) all commands via PCI
> 2) all commands via adminq
> 3) part of the commands via PCI and the rest via adminq
> 
> This proposal goes to for 3 with transport specific commands while I
> think we need to try 1 and 2 then it is more general and it helps to
> avoid duplication with future features.
> 
All legacy interface via AQ.
All modern interface access via PCI or its own transport between driver and device.