Re: proposal: use admin command (and aq) of the device to query config space

["Zhu, Lingshan" <lingshan.zhu@intel.com>]

On 8/2/2023 12:20 PM, Michael S. Tsirkin wrote:
> On Tue, Aug 01, 2023 at 07:09:14AM +0000, Parav Pandit wrote:
> > One line proposal:
> > Let's use new admin command and admin q for all device types to query device config space for new fields. (always).
> >
> > Details below.
> >
> > Query of device capabilities and configuration using DMA interface.
> > Need:
> > Currently device configuration space is exposed as read only registers. It is growing rapidly.
> > Some devices may be even multi-functionality device in coming future such as net + clock + rdma device.
> > For a PCI transport implementing such ever-growing capabilities, configuration is burdensome as plain registers.
> > Hence, it is required for the driver to query capabilities and configuration using a DMA interface.
> >
> > Interface requirements:
> > 1. Maintain backward compatibility for existing defined configuration fields to stay as registers.
> This is both too specific and too vague.  I think you mean "device
> should be able to optionally support both config access over DMA and support
> existing transitional and non-transitional virtio pci drivers".
>
> > 2. Any new field added must be accessed via DMA interface, regardless of device implementation (hw/sw etc).
> > Results in single driver code regardless of device implementation.
> > 3. A device must be able to choose, starting from which field driver must query
> > such configuration via DMA interface. This field offset must be greater than currently defined configuration field.
> 2 and 3 look like implementation more than like a requirement.
> Try to think what your actual requirement is.
>
> > 4. Any driver to device query operation must not be mandated to be
> > mediated by the owner device for PCI VFs or SIOV or SF devices. Driver must be able to
> > communicate query capabilities and configuration fields directly to
> > the device regardless of device type being PCI PF, VF, SF/SIOV device uniformly.
> I don't really know what does it mean to communicate directly with SIOV device.
> neither do I know query is and what capabilities are.
>
> > 5. When having multi-functionality device in future, it is desired to not always
> > query all the configuration but may be able to query per-functionality configurations.
> > For example, query only steering capabilities, query only rdma capabilities or query only clock capabilities.
> I don't know what query is and what capabilities are. Do you mean
> "access part of config space"?
>
>
> > 6. The driver should be able to query config/capabilities without
> > polling for the DMA completion, in other words, the driver should be able to get
> > notification from the device when DMA command completes.
> > 7. The driver should be able to utilize existing interrupt vector and/or
> > virtqueue for query and set operation without demanding
> > additional interrupt vector whenever possible.
> additional over what? existing where?
>
> > There are multiple options for DMA interface.
> > Some of these options are listed below that we would like to consider fulfilling above requirements.
>
> Cc: Zhu Lingshan <lingshan.zhu@intel.com>
>
> Zhu Lingshan are you still interested in a transport for SIOV (what was
> called transport vq)? Clearly SIOV needs this as part of the transport.
Hi Michael,

Rebasing "transport vq" on admin vq is still in my pipeline, I am 
currently working
on a patch series implementing virtio pci live migration in the 
spec(carry on Jason and Eugenio's work).
I will post it next week and back to transport vq task then.

For SIOV, I have implemented the interfaces for querying the 
configurations, for example:
1) Access config space:

247 +\begin{lstlisting}
248 +#define VIRTIO_TRANSPTQ_CTRL_CONFIGÂÂÂ 6
249 +Â #define VIRTIO_TRANSPTQ_CTRL_CONFIG_GETÂÂÂ 0
250 +Â #define VIRTIO_TRANSPTQ_CTRL_CONFIG_SETÂÂÂ 1
251 +
252 +struct virtio_transportq_ctrl_dev_config_get {
253 +ÂÂÂÂÂÂ u32 offset;
254 +ÂÂÂÂÂÂ u32 size;
255 +};
256 +
257 +struct virtio_transportq_ctrl_dev_config_set {
258 +ÂÂÂÂÂÂ u32 offset;
259 +ÂÂÂÂÂÂ u32 size;
260 +ÂÂÂÂÂÂ u8Â data[];
261 +};
262 +\end{lstlisting}

I think we can re-use the GET command for SR-IOV, I am afraid SET would 
be a side-channel attacking surface for SR-IOV.

2) a specific config, e.g., features:

142 +The features negotiation of managed devices is done by the
143 +following commands:
144 +
145 +\begin{lstlisting}
146 +#define VIRTIO_TRANSPTQ_CTRL_FEATÂÂ 3
147 + #define VIRTIO_TRANSPTQ_CTRL_FEAT_DEVICE_GETÂÂÂÂÂÂÂ 0
148 + #define VIRTIO_TRANSPTQ_CTRL_FEAT_DRIVER_SETÂÂÂÂÂÂÂ 1
149 + #define VIRTIO_TRANSPTQ_CTRL_FEAT_DRIVER_GETÂÂÂÂÂÂÂ 2
150 +
151 +struct virtio_transportq_ctrl_dev_features {
152 +ÂÂÂÂÂÂÂ u32 features_len;
153 +ÂÂÂÂÂÂÂ u32 features[features_len];
154 +};
155 +\end{lstlisting}

still can re-use the GET commands.

Thanks
Zhu Lingshan
>