Re: [PATCH v2] scsi: virtio_scsi: implement eh_host_reset_handler

[Mike Christie <michael.christie@oracle.com>]

On 11/3/26 7:51 AM, Stefan Hajnoczi wrote:
> On Tue, Nov 03, 2026 at 08:27:52AM +0100, Hannes Reinecke wrote:
>> On 11/2/26 9:06 PM, Stefan Hajnoczi wrote:
>>> On Mon, Sep 14, 2026 at 08:31:44PM +0700, Nguyen Ngoc Thang wrote:
>>>> Once virtscsi_eh_timed_out() lets SCSI EH run on an unresponsive host,
>>>> abort and device reset both fail the same way (same dead ctrl vq), and
>>>> since virtio_scsi implements neither target nor bus reset, EH falls
>>>> through to scsi_eh_offline_sdevs(), which frees the stuck commands' DMA
>>>> buffers without any guarantee the device has actually stopped touching
>>>> them. If bus mastering (or whatever broke the transport) comes back
>>>> later, the device can still write into memory the kernel has already
>>>> reused.
>>>>
>>>> Add a real eh_host_reset_handler instead. virtio_reset_device() is the
>>>> guarantee scsi_eh_offline_sdevs() was missing: once it returns, the
>>>> device will not touch guest memory again, so it is safe to hand
>>>> outstanding commands' buffers back. Reuse virtscsi_remove_vqs() +
>>>> virtscsi_init() -- the exact sequence already used across
>>>> suspend/resume -- to tear the virtqueues down and rebuild them.
>>>>
>>>> Verified with a QEMU virtio-scsi repro (PCI_COMMAND_MASTER cleared
>>>> mid-write): EH now runs abort -> device reset -> host reset -> abort ->
>>>> device reset -> host reset -> offline, then dd's stuck fsync returns
>>>> EIO and D-state drains to 0, in ~244s total. No task remains
>>>> uninterruptibly blocked.
>>> By the way, there is a known limitation in the QEMU virtio-scsi
>>> implementation where some reset-related operations are implemented
>>> synchronously (can block vCPUs and QEMU activity while waiting for I/O
>>> to drain in some situations) instead of asynchronously. Recently there
>>> has been work in QEMU to make VIRTIO device reset asynchronous, for
>>> example, but it needs to be implemented on a per-device basis and
>>> virtio-scsi hasn't taken advantage of it yet.
>>>
>>> Please let me know if you'd like to discuss or contribute. It's
>>> something that is on my radar but not until 2027.
>>>
>>>> Suggested-by: Hannes Reinecke <hare@suse.de>
>>>> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
>>>> ---
>>>> v1 -> v2: implemented eh_host_reset_handler as suggested by Hannes
>>>> Reinecke in review of the original RFC, instead of relying solely on
>>>> the EH loop falling through to offline.
>>>> ---
>>>>   drivers/scsi/virtio_scsi.c | 24 ++++++++++++++++++++++++
>>>>   1 file changed, 24 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>>>> index b4f20c487718..9698f4f91cc6 100644
>>>> --- a/drivers/scsi/virtio_scsi.c
>>>> +++ b/drivers/scsi/virtio_scsi.c
>>>> @@ -833,6 +833,8 @@ static enum scsi_timeout_action virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
>>>>   	return SCSI_EH_NOT_HANDLED;
>>>>   }
>>>> +static int virtscsi_host_reset(struct scsi_cmnd *sc);
>>>> +
>>>>   static const struct scsi_host_template virtscsi_host_template = {
>>>>   	.module = THIS_MODULE,
>>>>   	.name = "Virtio SCSI HBA",
>>>> @@ -846,6 +848,7 @@ static const struct scsi_host_template virtscsi_host_template = {
>>>>   	.eh_abort_handler = virtscsi_abort,
>>>>   	.eh_device_reset_handler = virtscsi_device_reset,
>>>>   	.eh_timed_out = virtscsi_eh_timed_out,
>>>> +	.eh_host_reset_handler = virtscsi_host_reset,
>>>>   	.sdev_init = virtscsi_device_alloc,
>>>>   	.dma_boundary = UINT_MAX,
>>>> @@ -947,6 +950,27 @@ static int virtscsi_init(struct virtio_device *vdev,
>>>>   	return err;
>>>>   }
>>>> +/*
>>>> + * No bus/target reset in virtio-scsi, so EH lands here after device
>>> The choice to use the VIRTIO device reset in virtscsi_host_reset() looks
>>> good to me, but I would like clarification on this comment. virtio-scsi
>>> does have I_T_NEXUS_RESET and I believe this is the modern equivalent to
>>> the pre-SCSI-6 TARGET RESET. Is it really the case that virtio-scsi
>>> doesn't have target reset? And do you want to go ahead and implement
>>> .eh_target_reset_handler() using I_T_NEXUS_RESET?
>>>
>> Would probably not a bad idea to use I_T_NEXUS_RESET here.
>> Not sure what it does internally, though; _is_ there a nexus
>> to reset?
> Whether there is any benefit to I_T_NEXUS_RESET may depend on the device
> implementation. In QEMU:
> - LOGICAL_UNIT_RESET resets just 1 LUN
> - I_T_NEXUS_RESET resets all LUNs
> - VIRTIO device reset also resets the HBA itself
> 
> In practice QEMU's I_T_NEXUS_RESET is unlikely to fix things if
> LOGICAL_UNIT_RESET wasn't sufficient. Maybe it will help on other device
> implementations though.
> 
> I think the Linux driver mapping should be:
> - .eh_device_reset_handler() -> LOGICAL_UNIT_RESET
> - .eh_target_reset_handler() -> I_T_NEXUS_RESET
> - .eh_host_reset_handler() -> VIRTIO device reset

How are vhost drivers notified of the device reset? For qemu virtio-scsi
this would call virtio_scsi_reset right?

For vhost-scsi though, do we need to add a similar reset function?