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

Jason Wang
2023-04-12T04:07:41Z

On Wed, Apr 12, 2023 at 5:25 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> 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.
>
> > 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.
> The reason is that extended config can be emulated more efficiently
> (2x less exits).

Any reason for it to get less exits? At least it has not been done in
current Qemu's emulation. (And do we really care about the performance
of config space access?)

Thanks

> WDYT?
>
>
> --
> MST