From owner-svn-src-all@freebsd.org Fri Jul 15 02:29:12 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 189CEB972A6; Fri, 15 Jul 2016 02:29:12 +0000 (UTC) (envelope-from sephe@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 mx1.freebsd.org (Postfix) with ESMTPS id CDAEA1D62; Fri, 15 Jul 2016 02:29:11 +0000 (UTC) (envelope-from sephe@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u6F2TB9i008992; Fri, 15 Jul 2016 02:29:11 GMT (envelope-from sephe@FreeBSD.org) Received: (from sephe@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u6F2TBXR008991; Fri, 15 Jul 2016 02:29:11 GMT (envelope-from sephe@FreeBSD.org) Message-Id: <201607150229.u6F2TBXR008991@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: sephe set sender to sephe@FreeBSD.org using -f From: Sepherosa Ziehau Date: Fri, 15 Jul 2016 02:29:11 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r302863 - stable/10/sys/dev/hyperv/storvsc X-SVN-Group: stable-10 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.22 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: Fri, 15 Jul 2016 02:29:12 -0000 Author: sephe Date: Fri Jul 15 02:29:10 2016 New Revision: 302863 URL: https://svnweb.freebsd.org/changeset/base/302863 Log: MFC 302541,302605 302541 hyperv/stor: Fix the INQUIRY checks Don't check the area that the host has not filled. PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209443 PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210425 Submitted by: Hongjiang Zhang Reviewed by: sephe, Dexuan Cui Sponsored by: Microsoft OSTC Differential Revision: https://reviews.freebsd.org/D6955 302605 hyperv/stor: Save the response status and xfer length properly. The current command response handling discards status and xfer length unconditionally, so that all of the commands would be considered successful, even if errors happened. When errors really happens, this causes all kinds of wiredness, since the buffer will not be filled on the host side and sense data will be ignored. Most of the time, errors do not happen, however, error does happen for the request sent immediately after the disk resizing. Discarding the SCSI status (SCSI_STATUS_CHECK_COND) and sense data (capacity changes) prevents the disk resizing from working properly. This commit saves the response status and xfer length properly for later use. Submitted by: Dexuan Cui Noticed by: sephe Sponsored by: Microsoft OSTC Differential Revision: https://reviews.freebsd.org/D7181 Modified: stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c ============================================================================== --- stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Fri Jul 15 02:24:39 2016 (r302862) +++ stable/10/sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c Fri Jul 15 02:29:10 2016 (r302863) @@ -805,6 +805,13 @@ hv_storvsc_on_iocompletion(struct storvs vm_srb = &vstor_packet->u.vm_srb; + /* + * Copy some fields of the host's response into the request structure, + * because the fields will be used later in storvsc_io_done(). + */ + request->vstor_packet.u.vm_srb.scsi_status = vm_srb->scsi_status; + request->vstor_packet.u.vm_srb.transfer_len = vm_srb->transfer_len; + if (((vm_srb->scsi_status & 0xFF) == SCSI_STATUS_CHECK_COND) && (vm_srb->srb_status & SRB_STATUS_AUTOSENSE_VALID)) { /* Autosense data available */ @@ -1939,62 +1946,24 @@ create_storvsc_request(union ccb *ccb, s } /* - * Modified based on scsi_print_inquiry which is responsible to - * print the detail information for scsi_inquiry_data. - * + * SCSI Inquiry checks qualifier and type. + * If qualifier is 011b, means the device server is not capable + * of supporting a peripheral device on this logical unit, and + * the type should be set to 1Fh. + * * Return 1 if it is valid, 0 otherwise. */ static inline int is_inquiry_valid(const struct scsi_inquiry_data *inq_data) { uint8_t type; - char vendor[16], product[48], revision[16]; - - /* - * Check device type and qualifier - */ - if (!(SID_QUAL_IS_VENDOR_UNIQUE(inq_data) || - SID_QUAL(inq_data) == SID_QUAL_LU_CONNECTED)) + if (SID_QUAL(inq_data) != SID_QUAL_LU_CONNECTED) { return (0); - + } type = SID_TYPE(inq_data); - switch (type) { - case T_DIRECT: - case T_SEQUENTIAL: - case T_PRINTER: - case T_PROCESSOR: - case T_WORM: - case T_CDROM: - case T_SCANNER: - case T_OPTICAL: - case T_CHANGER: - case T_COMM: - case T_STORARRAY: - case T_ENCLOSURE: - case T_RBC: - case T_OCRW: - case T_OSD: - case T_ADC: - break; - case T_NODEVICE: - default: + if (type == T_NODEVICE) { return (0); } - - /* - * Check vendor, product, and revision - */ - cam_strvis(vendor, inq_data->vendor, sizeof(inq_data->vendor), - sizeof(vendor)); - cam_strvis(product, inq_data->product, sizeof(inq_data->product), - sizeof(product)); - cam_strvis(revision, inq_data->revision, sizeof(inq_data->revision), - sizeof(revision)); - if (strlen(vendor) == 0 || - strlen(product) == 0 || - strlen(revision) == 0) - return (0); - return (1); } @@ -2071,7 +2040,6 @@ storvsc_io_done(struct hv_storvsc_reques ccb->ccb_h.status &= ~CAM_STATUS_MASK; if (vm_srb->scsi_status == SCSI_STATUS_OK) { const struct scsi_generic *cmd; - /* * Check whether the data for INQUIRY cmd is valid or * not. Windows 10 and Windows 2016 send all zero @@ -2080,23 +2048,59 @@ storvsc_io_done(struct hv_storvsc_reques cmd = (const struct scsi_generic *) ((ccb->ccb_h.flags & CAM_CDB_POINTER) ? csio->cdb_io.cdb_ptr : csio->cdb_io.cdb_bytes); - if (cmd->opcode == INQUIRY && - /* - * XXX: Temporary work around disk hot plugin on win2k12r2, - * only filtering the invalid disk on win10 or 2016 server. - * So, the hot plugin on win10 and 2016 server needs - * to be fixed. + if (cmd->opcode == INQUIRY) { + /* + * The host of Windows 10 or 2016 server will response + * the inquiry request with invalid data for unexisted device: + [0x7f 0x0 0x5 0x2 0x1f ... ] + * But on windows 2012 R2, the response is: + [0x7f 0x0 0x0 0x0 0x0 ] + * That is why here wants to validate the inquiry response. + * The validation will skip the INQUIRY whose response is short, + * which is less than SHORT_INQUIRY_LENGTH (36). + * + * For more information about INQUIRY, please refer to: + * ftp://ftp.avc-pioneer.com/Mtfuji_7/Proposal/Jun09/INQUIRY.pdf */ - vmstor_proto_version == VMSTOR_PROTOCOL_VERSION_WIN10 && - is_inquiry_valid( - (const struct scsi_inquiry_data *)csio->data_ptr) == 0) { + const struct scsi_inquiry_data *inq_data = + (const struct scsi_inquiry_data *)csio->data_ptr; + uint8_t* resp_buf = (uint8_t*)csio->data_ptr; + /* Get the buffer length reported by host */ + int resp_xfer_len = vm_srb->transfer_len; + /* Get the available buffer length */ + int resp_buf_len = resp_xfer_len >= 5 ? resp_buf[4] + 5 : 0; + int data_len = (resp_buf_len < resp_xfer_len) ? resp_buf_len : resp_xfer_len; + if (data_len < SHORT_INQUIRY_LENGTH) { + ccb->ccb_h.status |= CAM_REQ_CMP; + if (bootverbose && data_len >= 5) { + mtx_lock(&sc->hs_lock); + xpt_print(ccb->ccb_h.path, + "storvsc skips the validation for short inquiry (%d)" + " [%x %x %x %x %x]\n", + data_len,resp_buf[0],resp_buf[1],resp_buf[2], + resp_buf[3],resp_buf[4]); + mtx_unlock(&sc->hs_lock); + } + } else if (is_inquiry_valid(inq_data) == 0) { ccb->ccb_h.status |= CAM_DEV_NOT_THERE; + if (bootverbose && data_len >= 5) { + mtx_lock(&sc->hs_lock); + xpt_print(ccb->ccb_h.path, + "storvsc uninstalled invalid device" + " [%x %x %x %x %x]\n", + resp_buf[0],resp_buf[1],resp_buf[2],resp_buf[3],resp_buf[4]); + mtx_unlock(&sc->hs_lock); + } + } else { + ccb->ccb_h.status |= CAM_REQ_CMP; if (bootverbose) { mtx_lock(&sc->hs_lock); xpt_print(ccb->ccb_h.path, - "storvsc uninstalled device\n"); + "storvsc has passed inquiry response (%d) validation\n", + data_len); mtx_unlock(&sc->hs_lock); } + } } else { ccb->ccb_h.status |= CAM_REQ_CMP; }