Re: [virtio-dev] [PATCH 08/11] transport-pci: Introduce virtio extended capability

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Apr 12, 2023 at 12:40:53AM +0000, Parav Pandit wrote:
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, April 11, 2023 5:25 PM
> > 
> > On Tue, Apr 11, 2023 at 07:01:16PM +0000, Parav Pandit wrote:
> > >
> > > > From: virtio-dev@lists.oasis-open.org
> > > > <virtio-dev@lists.oasis-open.org> On Behalf Of Jason Wang
> > > > Sent: Monday, April 10, 2023 11:29 PM
> > >
> > > > > However, it is not backward compatible, if the device place them
> > > > > in extended capability, it will not work.
> > > > >
> > > >
> > > > It is kind of intended since it is only used for new PCI-E features:
> > > >
> > > New fields in new extended pci cap area is fine.
> > > Migrating old fields to be present in the new extended pci cap, is not your
> > intention. Right?
> > >
> > > > "
> > > > +The location of the virtio structures that depend on the PCI
> > > > +Express capability are specified using a vendor-specific extended
> > > > +capabilities on the extended capabilities list in PCI Express
> > > > +extended configuration space of the device.
> > > > "
> > > >
> > > > > To make it backward compatible, a device needs to expose existing
> > > > > structure in legacy area. And extended structure for same
> > > > > capability in extended pci capability region.
> > > > >
> > > > > In other words, it will have to be a both places.
> > > >
> > > > Then we will run out of config space again?
> > > No.
> > > Only currently defined caps to be placed in two places.
> > > New fields donât need to be placed in PCI cap, because no driver is looking
> > there.
> > >
> > > We probably already discussed this in previous email by now.
> > >
> > > > Otherwise we need to deal with the
> > > > case when existing structures were only placed at extended
> > > > capability. Michael suggest to add a new feature, but the driver may
> > > > not negotiate the feature which requires more thought.
> > > >
> > > Not sure I understand feature bit.
> > 
> > This is because we have a concept of dependency between features but not a
> > concept of dependency of feature on capability.
> > 
> A feature bit is accessible after virtio level initialization is complete.
> Virtio level initialization depends on the PCI layer transport attributes.
> So pci level attributes should exists regardless of upper layer initialization.

sure

> > > PCI transport fields existence is usually not dependent on upper layer
> > protocol.
> > >
> > > > > We may need it even sooner than this because the AQ patch is
> > > > > expanding the structure located in legacy area.
> > > >
> > > > Just to make sure I understand this, assuming we have adminq, any
> > > > reason a dedicated pcie ext cap is required?
> > > >
> > > No. it was my short sight. I responded right after above text that AQ doesnât
> > need cap extension.
> > 
> > 
> > 
> > You know, thinking about this, I begin to feel that we should require that if at
> > least one extended config exists then all caps present in the regular config are
> > *also* mirrored in the extended config. IOW extended >= regular.
> Fair but why is this a requirement to mirror?
> Is the below one to improve the perf?

yes

> If so, it is fine, but I guess it is optional.

It can not be optional otherwise guest has to scan legacy capability list too
losing perf advantage :).

> > The reason is that extended config can be emulated more efficiently (2x less
> > exits).
> > WDYT?
> > 
> > 
> > --
> > MST
>