Re: [PATCH RFC] virtio-spec: flexible configuration layout

[Sasha Levin <levinsasha928@gmail.com>]

On Tue, 2011-11-08 at 23:40 +0200, Michael S. Tsirkin wrote:
> Here's a spec change documenting what my C patch does
> (almost - I tweaked the layout a bit, but the idea is the same).
> Some more cleanups are needed but I thought I'd send it
> for early flames/comments.
> 
> The idea is simple: we split functionally unrelated
> register groups to independent structures, and let
> the device place is anywhere using a capability
> in PCI configuration space.
> 
> It can then go into MMIO space which is cheaper than PIO.
> 
> A legacy portion of the configuration is mirrored
> in the first BAR, to keep legacy drivers working.
> Any new fields can be added in existing structures
> at the end, so they won't affect legacy.

If newer specs add more structures at the end of the config space, and
use the same config space for legacy, that space now becomes device
specific config space and not new-shiny-feature space, so we must
remember to handle those cases.

> Alternatively we can add new structures with new
> structure IDs, pointed to from PCI configuration space.
> 
> As we don't yet have devices or drivers with 64 bit features,
> I decided we don't need high feature bits in legacy space.
> This also frees up feature bit 31 as we don't need it
> to enable high feature bits anymore.

KVM tool actually has support for 64bit features, we can probably remove
that when Pekka isn't looking :)

> 
> As this solves the dynamic placement of MSIX vectors
> and high feature bits,
> I thought it's easier to just reserve space for that
> programming than give it a separate structure. This
> can be changed by a patch on top.
> 
> Note that data path is split from configuration.
> 
> PDF will follow.
> ----
> 

The device initialization sequence might use an update as well. Maybe
also a description of how device handles missing structure IDs.

[snip]

> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1320781133
> +These registers are specified using vendor-specific PCI capability located
> + on capability list in PCI configuration space of the device.
> + This virtio structure capability uses little-endian format; all bits are
> + read-only:
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1320772579
> +\begin_inset Tabular

Just a note, these tables are way too wide to work properly in PDFs :)

> +<lyxtabular version="3" rows="4" columns="34">
> +<features tabularvalignment="middle">
> +<column alignment="center" valignment="top" width="0pt">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0">
> +<column alignment="center" valignment="top" width="0pt">
> +<row>

[snip]

> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1320779667
> +Purpose:
> +\end_layout
> +
> +\begin_layout Standard
> +
> +\change_inserted 1986246365 1320780912
> +
> +\emph on
> +Capability ID
> +\emph default
> +, 
> +\emph on
> +Next Capability Pointer
> +\emph default
> +, 
> +\emph on
> +Capability Length
> +\emph default
> + - these fields are specified by PCI local bus specification, Rev 3.0

I'm not sure what capability length is, can't find it in the spec
either.

[snip]

> +\begin_layout Plain Layout
> +ie.
> + once you enable MSI-X on the device, the other fields move.
> + If you turn it off again, they move back!

Is it still true? We're talking about the new layout here (there are
several of this footnote, this one is located right *before* the section
which talks about legacy config space.

[snip]

> + 
> +\change_deleted 1986246365 1320784929
> +If more than 31 feature bits are supported, the device indicates so by setting
> + feature bit 31 (see 

The bit numbers below this text should be corrected as well.

-- 

Sasha.