RE: [PATCH v2] Add device reset timeout field

[Cornelia Huck <cohuck@redhat.com>]

On Fri, Oct 15 2021, Parav Pandit <parav@nvidia.com> wrote:

>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Friday, October 15, 2021 3:59 AM
>> 
>> On Thu, Oct 14, 2021 at 05:35:37PM +0000, Parav Pandit wrote:
>> > Below changes are good for v3?
>> > 1. driver should use device reset time during initialization stage
>> 
>> How does driver identify this though?
> Existence of device_reset_timeout field in struct virtio_pci_common_cfg indicates that this field exists.
> If device support it, it will place non zero value and driver knows that this field should be used.

But how does the driver know? The very first step in the initialization
sequence is "reset the device", before it may read the config space.

>
>> 
>> > 2. remove feature bit as feature bits are only readable after reset is
>> > completed 3. device reset timeout field of zero indicates that device doesn't
>> support it.
>> 
>> I'm not sure about 3. I think each transport will need its own way to do it.
>> 
> For pci a value of zero indicates it isn't supported.
> For mmio DeviceResetTimeout at offset 0x04c indicates same.
> Currently only these 2 transports have the use.

I don't think we want to force all transports, including not yet
existing ones, to use that mechanism. They might as well use a command
to retrieve the value and fail the command, for example.

>
>> So I propose: maybe a capability like this, with a timeout field?
> Do you mean a new capability like say VIRTIO_PCI_DEVICE_TIMEOUT like VIRTIO_PCI_CAP_COMMON_CFG?
> This will contain one or more timeout? For example with his proposal it contains only device reset timeout.
> Later same capability will be further extended to contains command timeout too? Yes?
>
>> And within VMs, we can just do without, since it got out of reset once it will
>> surely get out of reset again...
> Yes, VM might not need it. It is really the HV's choice to implement and not part of the virtio spec.
> Our internal cloud passthrough a PF to the VM.
> It is probably better to let HV to choose if they want to do ctrl+c or have timeout.

But you want it in the spec, don't you? Confused.