RE: [PATCH v2 1/2] transport-pci: Introduce legacy registers access commands

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, May 17, 2023 1:45 AM
> 
> On Sat, May 06, 2023 at 03:01:34AM +0300, Parav Pandit wrote:
> > +\begin{lstlisting}
> > +struct virtio_admin_cmd_lreg_wr_data {
> > +	u8 offset; /* Starting byte offset of the register(s) to write */
> > +	u8 register[];
> > +};
> > +\end{lstlisting}
> 
> So to summarize, I think the main comment here was to have separate
> commands for access to common versus device config. Just today I thought
> some more and came up with another reason why this is a good idea: if we ever
> want legacy MMIO emulation, we can reuse the device config command.
> 
MMIO is actually simpler than PCI because all the registers are contiguous regardless of msix.
So single command with offset can service just fine.
More below.

> Jason also feels the common config command can be shared with vq transport
> effort. We can discuss this, for sure, I guess its putting code in hardware versus
> in hypervisor, but if there are hardware vendors who strictly want this code in
> hardware I don't have a big problem with this even if I don't exactly get why.
> 
> With this idea all hypervisor has to do is subtract the offset from device config
> access, sounds like a small price to pay.  
It is more than the subtraction cost.

Hypervisor needs to inspect MSI-X enable/disable settings to decide when to issue command #1 vs #3 for the requested offset.

> Does this sound like a reasonable
> compromize to you?

Splitting proposed one command to two commands,
1. one for accessing legacy common config
2. second for accessing legacy device specific config 

seems fine to me as below.

So we will have total 5 commands (instead of 3).

1. legacy common config read
2. legacy common config write

3. legacy device config read
4. legacy device config write
5. query device notification area

#1 and #3 same cmd signature but different opcode.
#2 and #4 same cmd signature but different opcode.