Re: [virtio-comment] [PATCH RFC] virtio-pci: new configuration layout

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

On Wed, Sep 04, 2013 at 05:21:25PM +0930, Rusty Russell wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> > On Mon, Sep 02, 2013 at 10:45:11AM +0930, Rusty Russell wrote:
> >> "Michael S. Tsirkin" <mst@redhat.com> writes:
> >> > Hmm I am not sure I agree - what does it buy us that feature bits don't already supply?
> >> 
> >> AFAICT it's a PCI question more than a virtio question.  The use of
> >> revision IDs is left open in the PCI spec, but Microsoft's requirements
> >> here seem reasonable:
> >> 
> >> http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx
> >
> > They are only reasonable if you consider a requirement that users
> > load new drivers for new revision of devices reasonable.
> 
> Well, I think it's reasonable for a driver to contain revision-specific
> workarounds.  That's kind of what the field is for.
> 
> >> > More concerns:
> >> >
> >> > We are using revision ID now exactly as was intended to disable old
> >> > drivers - it served us well for 0.X-1.X and would be as useful if we
> >> > ever have 1.X->2.0 transition.
> >> 
> >> I think we'll use feature bit 64 for such a transition, now we don't
> >> have a feature bit limit.
> >
> > Windows drivers only match on device,vendor id, subsystem ids and revision.
> >
> > Once you match on that you can fail after checking feature bits
> > but it's too late to have another driver loaded.
> 
> So can Windows drivers not match multiple revids?

My understanding is that you can either match a specific list of rev
ids, or match all rev IDs (using *).

> If not, you're saying
> tha transitional drivers are impossible, and they will need separate
> legacy and v1.0 drivers?

A transitional driver for windows will just have to match
both rev IDs 0 and 1.

> >> >> > +bar
> >> >> > +
> >> >> > +	values 0x0 to 0x5 specify a Base Address register (BAR) belonging to
> >> >> > +	the function located beginning at 10h in Configuration Space
> >> >> > +	and used to map the structure into Memory or I/O Space.
> >> >> > +	The BAR is permitted to be either 32-bit or 64-bit, it can map Memory Space
> >> >> > +	or I/O Space.
> >> >> > +
> >> >> > +	The value 0xF specifies that the structure is in PCI configuration space
> >> >> > +	inline with this capability structure, following (not necessarily immediately)
> >> >> > +	the length field.
> >> >> 
> >> >> Why not immediately?
> >> >>  Or how would the driver know where it is?
> >> >
> >> > It's at the offset.
> >> >
> >> > E.g. for notification we stick multiplier after length.
> >> > Further, we might extend virtio_pci_cap in the future,
> >> > and we don't want to move stuff around like we
> >> > had to with MSI-X.
> >> 
> >> So, offset is described as "/* Offset within bar. */", but if bar is
> >> 0xF, it's offset within the config space?
> >
> > Yes. So I'll fix the wording?
> 
> Please.  And specify if the offset is from start of config space, or
> offset from start or end of the virtio_pci_cap.
> 
> >> This adds complexity: do we really want this?
> >
> > I think making stuff part of a vendor specific cap is important,
> > and we might add stuff to capabilities, so what are
> > some other options?
> 
> If we want to embed the notification within the PCI config space, this
> makes sense.  But I'd rather hear from someone who wants that.

I think PXE drivers will want to do this.
I'll check with Gleb.

> Otherwise we can reserve invalid BAR values and tell them to keep
> iterating until the find a cap they understand.  That would let us add
> this later.
> 
> Cheers,
> Rusty.