Re: [PATCH v1 1/2] transport-pci: Introduce legacy registers access commands

[Parav Pandit <parav@nvidia.com>]

On 5/3/2023 12:48 PM, Michael S. Tsirkin wrote:

> > > As simple as it is, I feel this falls far short of describing how
> > > a device should operate.
> > > Some issues:
> > > 	- legacy device config offset changes as msi is enabled/disabled
> > > 	  suggest separate commands for device/common config
> > This is fine and covered here. The one who is making msix enable/disable
> > knows which registers to access before/after disable/enable and device also
> > knows it as it is supplying the register.
> > So they just follow the standard legacy register access behavior.
>
> But do VFs support INT#x? I will have to re-read the spec.
VFs do not support INTx.
When hypervisor knows that it cannot support msix for the guest, it can 
avoid using the VF for the guest.

> > > 	- legacy device endian-ness changes with guest
> > > 	  suggest commands to enable LE and BE mode
> > guest endianeness is not known to the device.
>
> But is known to hypervisor.
It can be an extension command in future as part of the VF 
administration command to set it by the hypervisor PF.

> > Currently it is only for the
> > LE guests who had some legacy requirement.
>
> I don't like tying this to LE implicitly some devices might be BE only.
> With my idea device can support command to set LE, command to set BE or
> both.
It can be an addition in future if needed.

> > and PCIe is LE anyway.
>
> PCIE config endian-ness does not matter heere.
>
> > > 	- legacy guests often assume INT#x support
> > > 	  suggest a way to tunnel that too;
> > INT#x is present on the PCI device itself. So no need to tunnel it.
> > Also INT#x is very narrow case. When MSI-X is not supported, a hypervisor
> > can choose not to even connect such device to the guest VM.
>
> devices will support MSI. But *guest* might not support MSIX you only
> find out late when it is driving the device.
It is a pathological case that may not exist because legacy drivers and 
linux kernels all the way upto 2.6.32 have msix support.

And in case if hypervisor sw wants to support for unknown scenario, it 
can use the one msix based interrupt to emulate intx.

> > > I expected to see more statements along the lines of
> > >           command ABC has the same effect as access
> > >           to register DEF of the member through the legacy pci interface
> > Yes, good point. I will add it in the theory of operation section for this
> > mapping detail.
>
> OK, and overall if you see an existing statement about legacy do not
> copy it, just explain how it is mapped.
Yes, will not copy, only the mapping part to add.