From owner-svn-src-all@freebsd.org Mon Aug 31 09:05:46 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 7DC143BAF5F; Mon, 31 Aug 2020 09:05:46 +0000 (UTC) (envelope-from whu@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Bg46y2np3z4H4x; Mon, 31 Aug 2020 09:05:46 +0000 (UTC) (envelope-from whu@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 434B925533; Mon, 31 Aug 2020 09:05:46 +0000 (UTC) (envelope-from whu@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 07V95kHj074263; Mon, 31 Aug 2020 09:05:46 GMT (envelope-from whu@FreeBSD.org) Received: (from whu@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 07V95jOW074261; Mon, 31 Aug 2020 09:05:45 GMT (envelope-from whu@FreeBSD.org) Message-Id: <202008310905.07V95jOW074261@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: whu set sender to whu@FreeBSD.org using -f From: Wei Hu Date: Mon, 31 Aug 2020 09:05:45 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r364984 - head/sys/dev/hyperv/storvsc X-SVN-Group: head X-SVN-Commit-Author: whu X-SVN-Commit-Paths: head/sys/dev/hyperv/storvsc X-SVN-Commit-Revision: 364984 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Aug 2020 09:05:46 -0000 Author: whu Date: Mon Aug 31 09:05:45 2020 New Revision: 364984 URL: https://svnweb.freebsd.org/changeset/base/364984 Log: Hyper-V: storvsc: Enhance srb_status code handling. In hv_storvsc_io_request() when coring, prevent changing of the send channel from the base channel to another one. storvsc_poll always probes on the base channel. Based upon conversations with Microsoft, changed the handling of srb_status codes. Most we should never get, others yes. All are treated as retry-able except for two. We should not get these statuses, but if we ever do, the I/O state is not known. Submitted by: Alexander Sideropoulos Reviewed by: trasz, allanjude, whu MFC after: 1 week Sponsored by: Netapp Inc Differential Revision: https://reviews.freebsd.org/D25756 Modified: head/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c head/sys/dev/hyperv/storvsc/hv_vstorage.h Modified: head/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c ============================================================================== --- head/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Mon Aug 31 05:25:13 2020 (r364983) +++ head/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Mon Aug 31 09:05:45 2020 (r364984) @@ -151,6 +151,12 @@ SYSCTL_UINT(_hw_storvsc, OID_AUTO, max_io, CTLFLAG_RDT static int hv_storvsc_chan_cnt = 0; SYSCTL_INT(_hw_storvsc, OID_AUTO, chan_cnt, CTLFLAG_RDTUN, &hv_storvsc_chan_cnt, 0, "# of channels to use"); +#ifdef DIAGNOSTIC +static int hv_storvsc_srb_status = -1; +SYSCTL_INT(_hw_storvsc, OID_AUTO, srb_status, CTLFLAG_RW, + &hv_storvsc_srb_status, 0, "srb_status to inject"); +TUNABLE_INT("hw_storvsc.srb_status", &hv_storvsc_srb_status); +#endif /* DIAGNOSTIC */ #define STORVSC_MAX_IO \ vmbus_chan_prplist_nelem(hv_storvsc_ringbuffer_size, \ @@ -704,7 +710,16 @@ hv_storvsc_io_request(struct storvsc_softc *sc, vstor_packet->operation = VSTOR_OPERATION_EXECUTESRB; ch_sel = (vstor_packet->u.vm_srb.lun + curcpu) % sc->hs_nchan; - outgoing_channel = sc->hs_sel_chan[ch_sel]; + /* + * If we are panic'ing, then we are dumping core. Since storvsc_polls + * always uses sc->hs_chan, then we must send to that channel or a poll + * timeout will occur. + */ + if (panicstr) { + outgoing_channel = sc->hs_chan; + } else { + outgoing_channel = sc->hs_sel_chan[ch_sel]; + } mtx_unlock(&request->softc->hs_lock); if (request->prp_list.gpa_range.gpa_len) { @@ -2155,26 +2170,133 @@ storvsc_io_done(struct hv_storvsc_request *reqp) ccb->ccb_h.status &= ~CAM_SIM_QUEUED; ccb->ccb_h.status &= ~CAM_STATUS_MASK; int srb_status = SRB_STATUS(vm_srb->srb_status); +#ifdef DIAGNOSTIC + if (hv_storvsc_srb_status != -1) { + srb_status = SRB_STATUS(hv_storvsc_srb_status & 0x3f); + hv_storvsc_srb_status = -1; + } +#endif /* DIAGNOSTIC */ if (vm_srb->scsi_status == SCSI_STATUS_OK) { if (srb_status != SRB_STATUS_SUCCESS) { - /* - * If there are errors, for example, invalid LUN, - * host will inform VM through SRB status. - */ - if (bootverbose) { - if (srb_status == SRB_STATUS_INVALID_LUN) { - xpt_print(ccb->ccb_h.path, - "invalid LUN %d for op: %s\n", - vm_srb->lun, - scsi_op_desc(cmd->opcode, NULL)); - } else { - xpt_print(ccb->ccb_h.path, - "Unknown SRB flag: %d for op: %s\n", - srb_status, - scsi_op_desc(cmd->opcode, NULL)); - } + bool log_error = true; + switch (srb_status) { + case SRB_STATUS_PENDING: + /* We should never get this */ + panic("storvsc_io_done: SRB_STATUS_PENDING"); + break; + case SRB_STATUS_ABORTED: + /* + * storvsc doesn't support aborts yet + * but if we ever get this status + * the I/O is complete - treat it as a + * timeout + */ + ccb->ccb_h.status |= CAM_CMD_TIMEOUT; + break; + case SRB_STATUS_ABORT_FAILED: + /* We should never get this */ + panic("storvsc_io_done: SRB_STATUS_ABORT_FAILED"); + break; + case SRB_STATUS_ERROR: + /* + * We should never get this. + * Treat it as a CAM_UNREC_HBA_ERROR. + * It will be retried + */ + ccb->ccb_h.status |= CAM_UNREC_HBA_ERROR; + break; + case SRB_STATUS_BUSY: + /* Host is busy. Delay and retry */ + ccb->ccb_h.status |= CAM_BUSY; + break; + case SRB_STATUS_INVALID_REQUEST: + case SRB_STATUS_INVALID_PATH_ID: + case SRB_STATUS_NO_DEVICE: + case SRB_STATUS_INVALID_TARGET_ID: + /* + * These indicate an invalid address + * and really should never be seen. + * A CAM_PATH_INVALID could be + * used here but I want to run + * down retries. Do a CAM_BUSY + * since the host might be having issues. + */ + ccb->ccb_h.status |= CAM_BUSY; + break; + case SRB_STATUS_TIMEOUT: + case SRB_STATUS_COMMAND_TIMEOUT: + /* The backend has timed this out */ + ccb->ccb_h.status |= CAM_BUSY; + break; + /* Some old pSCSI errors below */ + case SRB_STATUS_SELECTION_TIMEOUT: + case SRB_STATUS_MESSAGE_REJECTED: + case SRB_STATUS_PARITY_ERROR: + case SRB_STATUS_NO_HBA: + case SRB_STATUS_DATA_OVERRUN: + case SRB_STATUS_UNEXPECTED_BUS_FREE: + case SRB_STATUS_PHASE_SEQUENCE_FAILURE: + /* + * Old pSCSI responses, should never get. + * If we do treat as a CAM_UNREC_HBA_ERROR + * which will be retried + */ + ccb->ccb_h.status |= CAM_UNREC_HBA_ERROR; + break; + case SRB_STATUS_BUS_RESET: + ccb->ccb_h.status |= CAM_SCSI_BUS_RESET; + break; + case SRB_STATUS_BAD_SRB_BLOCK_LENGTH: + /* + * The request block is malformed and + * I doubt it is from the guest. Just retry. + */ + ccb->ccb_h.status |= CAM_UNREC_HBA_ERROR; + break; + /* Not used statuses just retry */ + case SRB_STATUS_REQUEST_FLUSHED: + case SRB_STATUS_BAD_FUNCTION: + case SRB_STATUS_NOT_POWERED: + ccb->ccb_h.status |= CAM_UNREC_HBA_ERROR; + break; + case SRB_STATUS_INVALID_LUN: + /* + * Don't log an EMS for this response since + * there is no device at this LUN. This is a + * normal and expected response when a device + * is detached. + */ + ccb->ccb_h.status |= CAM_DEV_NOT_THERE; + log_error = false; + break; + case SRB_STATUS_ERROR_RECOVERY: + case SRB_STATUS_LINK_DOWN: + /* + * I don't ever expect these from + * the host but if we ever get + * retry after a delay + */ + ccb->ccb_h.status |= CAM_BUSY; + break; + default: + /* + * An undefined response assert on + * on debug builds else retry + */ + ccb->ccb_h.status |= CAM_UNREC_HBA_ERROR; + KASSERT(srb_status <= SRB_STATUS_LINK_DOWN, + ("storvsc: %s, unexpected srb_status of 0x%x", + __func__, srb_status)); + break; } - ccb->ccb_h.status |= CAM_DEV_NOT_THERE; + if (log_error) { + xpt_print(ccb->ccb_h.path, "The hypervisor's I/O adapter " + "driver received an unexpected response code 0x%x " + "for operation: %s. If this continues to occur, " + "report the condition to your hypervisor vendor so " + "they can rectify the issue.\n", srb_status, + scsi_op_desc(cmd->opcode, NULL)); + } } else { ccb->ccb_h.status |= CAM_REQ_CMP; } Modified: head/sys/dev/hyperv/storvsc/hv_vstorage.h ============================================================================== --- head/sys/dev/hyperv/storvsc/hv_vstorage.h Mon Aug 31 05:25:13 2020 (r364983) +++ head/sys/dev/hyperv/storvsc/hv_vstorage.h Mon Aug 31 09:05:45 2020 (r364984) @@ -241,12 +241,34 @@ struct vstor_packet { /** * SRB (SCSI Request Block) Status Codes */ -#define SRB_STATUS_PENDING 0x00 -#define SRB_STATUS_SUCCESS 0x01 -#define SRB_STATUS_ABORTED 0x02 -#define SRB_STATUS_ERROR 0x04 -#define SRB_STATUS_DATA_OVERRUN 0x12 -#define SRB_STATUS_INVALID_LUN 0x20 +#define SRB_STATUS_PENDING 0x00 +#define SRB_STATUS_SUCCESS 0x01 +#define SRB_STATUS_ABORTED 0x02 +#define SRB_STATUS_ABORT_FAILED 0x03 +#define SRB_STATUS_ERROR 0x04 +#define SRB_STATUS_BUSY 0x05 +#define SRB_STATUS_INVALID_REQUEST 0x06 +#define SRB_STATUS_INVALID_PATH_ID 0x07 +#define SRB_STATUS_NO_DEVICE 0x08 +#define SRB_STATUS_TIMEOUT 0x09 +#define SRB_STATUS_SELECTION_TIMEOUT 0x0A +#define SRB_STATUS_COMMAND_TIMEOUT 0x0B +#define SRB_STATUS_MESSAGE_REJECTED 0x0D +#define SRB_STATUS_BUS_RESET 0x0E +#define SRB_STATUS_PARITY_ERROR 0x0F +#define SRB_STATUS_REQUEST_SENSE_FAILED 0x10 +#define SRB_STATUS_NO_HBA 0x11 +#define SRB_STATUS_DATA_OVERRUN 0x12 +#define SRB_STATUS_UNEXPECTED_BUS_FREE 0x13 +#define SRB_STATUS_PHASE_SEQUENCE_FAILURE 0x14 +#define SRB_STATUS_BAD_SRB_BLOCK_LENGTH 0x15 +#define SRB_STATUS_REQUEST_FLUSHED 0x16 +#define SRB_STATUS_INVALID_LUN 0x20 +#define SRB_STATUS_INVALID_TARGET_ID 0x21 +#define SRB_STATUS_BAD_FUNCTION 0x22 +#define SRB_STATUS_ERROR_RECOVERY 0x23 +#define SRB_STATUS_NOT_POWERED 0x24 +#define SRB_STATUS_LINK_DOWN 0x25 /** * SRB Status Masks (can be combined with above status codes) */