RE: [PATCH v2] Add device reset timeout field

[Parav Pandit <parav@nvidia.com>]

> From: Cornelia Huck <cohuck@redhat.com>
> Sent: Monday, October 11, 2021 8:29 PM
> 
> On Mon, Oct 11 2021, Parav Pandit <parav@nvidia.com> wrote:
> 
> >> From: Michael S. Tsirkin <mst@redhat.com>
> >> Sent: Saturday, October 9, 2021 4:39 AM
> >>
> >> On Fri, Oct 08, 2021 at 01:23:52PM +0000, Parav Pandit wrote:
> >> >
> >> >
> >> > > From: Michael S. Tsirkin <mst@redhat.com>
> >> > > Sent: Friday, October 8, 2021 6:27 PM
> >> >
> >> > > > 2. A sriov VF virtio device for our case takes a lot lesser
> >> > > > than this, but may
> >> > > take anywhere between 10 msec to 250msec.
> >> > > > This can happen on a firmware where user enabled 500 SR-IOV VFs.
> >> > > > Pci spec indicates that all VFs to initialize within 100msec.
> >> > > > This translates to
> >> > > 0.2msec for one VF.
> >> > > > In some scenario this can be a hard to initialize a VF in 0.2
> >> > > > msec depending
> >> > > on what else a firmware is doing at that time.
> >> > >
> >> > > That's separate from virtio reset though. virtio reset is much
> >> > > lighter weight than a VF reset, all it needs to do is return
> >> > > config space to original values and stop DMA.
> >> > Again you took the valid example to stop the DMA of already
> >> > initialized device, while above case is for the first init. :-)
> >> > virtio device is going
> >> the first reset during initialization. It should be able to tell how long to wait.
> >> > A device firmware may take more than 0.2msec to finish needed
> >> > initialization
> >> to serve a virtio device.
> >> > Infinite wait of today works here.
> >>
> >> Looks like it's as Cornelia said - nothing to do with reset. E.g.
> >> it's likely device can not even serve pci config before the init is complete.
> >>
> > Not sure. I replied to Cornelia.
> > A pci device once placed on the pci bus has to respond to the config requests.
> > However virtio pci device virtio config registers can be accessed only after
> reset is completed.
> 
> What I don't understand: if a pci device is in such a state that it cannot
> complete a reset, how can it be able to serve the pci config? In the cases I saw
> mentioned (hotunplug, slow device startup), I'd expect the device simply being
> slow/unable to respond to anything.
> >
Not necessarily.
PCI device implementation in hw/fw needs to comply to the PCI spec that defines pci level semantics.
Device implements virtio specification on top of the PCI spec.
Serving virtio functionality is more than just PCI configs, so device implementation able to service pci requests but may not be ready to serve virtio block and networking IOs.
This provides best tradeoffs in hw implementation of what is done in which layer of hw/fw etc for PCI and virtio layer.

Such implementation is very common in multiple PCI device families such as mlx5, nvme and more.
For example nvme device has controller ready timeout with granularity of 500msec and has 16-bits of timeout.

> > More below.
> >
> > Feature negotiation is mainly for backward compatibility.
> 
> Related, I found I had opened this issue some time ago:
> https://github.com/oasis-tcs/virtio-spec/issues/98 Might be relevant when
> considering guarding this via a feature bit.
> 
If you think above can see day light, we may have feature bit for device reset too.
However above github note says "read optional field before FEATURES_OK", and not read optional field _before_ device reset.
Do you plan to modify issues/98 to read optional field before device reset?

> > This is unlikely to work the reset is completed. Because a real device
> implementing this would prefer to do this in fw for 1000 virtio devices sitting on
> the physical card.
> > And it is very much driven by such implementation at device devel.
> > So it cannot update the counter value if reset is not completed for the device.
> >
> > I think read only device reset timeout is most elegant option during device
> initialization phase that eliminates infinite loop of today.
> 
> Why can't a driver just go ahead and do a timeout regardless? 
o.k. lets consider this thought exercise. What is the timeout value that driver will choose if device doesn't specify one?
I explained in previous thread and you acked that actual fw based device may take longer to initialize than pure sw implementation backend.
In second example a pre-boot device can take even longer initialization time.
Sriov VF device may initialize lot faster.
Instead of driver having such transport, and device specific checks, (or some very short or very long timeout), we propose, that let device mention such timeout value.

> This seems
> pretty much to depend on the implementation. Nothing in the virtio spec forces
> the driver to reset all devices serially and wait potentially forever. (If you deal
> with a large amount of subdevices you cannot access concurrently, you likely
> already have problems.)
Sure, nothing in driver forces serial initialization.
However in reality, PCI device scanning on the bus in hotplug driver, preboot env, sriov vf initialization is serial.
In preboot case, even the order is defined, so it is sort of serial.

In other uses, concurrent access is fine and host does that today when it disables autoprobe on the PCI Bus and probes them in parallel after enabling sriov.
I hope you understand that there are both use-cases exists which scan parallel and some serial.