From owner-svn-src-all@freebsd.org Tue Mar 7 21:47:56 2017 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 80605D01AD8; Tue, 7 Mar 2017 21:47:56 +0000 (UTC) (envelope-from imp@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 587241A05; Tue, 7 Mar 2017 21:47:56 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v27LlthJ074557; Tue, 7 Mar 2017 21:47:55 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v27Lltmi074554; Tue, 7 Mar 2017 21:47:55 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201703072147.v27Lltmi074554@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Tue, 7 Mar 2017 21:47:55 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r314884 - head/sys/dev/nvme X-SVN-Group: head 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.23 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: Tue, 07 Mar 2017 21:47:56 -0000 Author: imp Date: Tue Mar 7 21:47:54 2017 New Revision: 314884 URL: https://svnweb.freebsd.org/changeset/base/314884 Log: Make multi-namespace nvme drives more robust. Fix assumptions about name spaces in NVME driver. First, it assumes cdata.nn is the number of configured devices. However, it is the number of supported name spaces. Second, it assumes that there will never be more than 16 name spaces supported, but a certain drive I'm testing reports 1024. It assumes that name spaces are a tightly packed namespace, but the standard seems to indicate otherwise. Finally, it assumes that an error would be generated when quearying an unconfigured namespace. Instead, it succeeds but the identify data is all zeros. Fix these by limiting the number of name spaces we probe to 16. Remove aborting when we find one in error. When the size of the name space is zero, ignore it. This is admittedly a bandaide. The long term fix will be to participate in the enumeration and name space change protocols definfed in the NVNe standard. Sponsored by: Netflix Modified: head/sys/dev/nvme/nvme.c head/sys/dev/nvme/nvme_ctrlr.c head/sys/dev/nvme/nvme_ns.c Modified: head/sys/dev/nvme/nvme.c ============================================================================== --- head/sys/dev/nvme/nvme.c Tue Mar 7 21:44:05 2017 (r314883) +++ head/sys/dev/nvme/nvme.c Tue Mar 7 21:47:54 2017 (r314884) @@ -327,8 +327,10 @@ nvme_notify(struct nvme_consumer *cons, */ return; } - for (ns_idx = 0; ns_idx < ctrlr->cdata.nn; ns_idx++) { + for (ns_idx = 0; ns_idx < min(ctrlr->cdata.nn, NVME_MAX_NAMESPACES); ns_idx++) { ns = &ctrlr->ns[ns_idx]; + if (ns->data.nsze == 0) + continue; if (cons->ns_fn != NULL) ns->cons_cookie[cons->id] = (*cons->ns_fn)(ns, ctrlr_cookie); Modified: head/sys/dev/nvme/nvme_ctrlr.c ============================================================================== --- head/sys/dev/nvme/nvme_ctrlr.c Tue Mar 7 21:44:05 2017 (r314883) +++ head/sys/dev/nvme/nvme_ctrlr.c Tue Mar 7 21:47:54 2017 (r314884) @@ -458,13 +458,11 @@ static int nvme_ctrlr_construct_namespaces(struct nvme_controller *ctrlr) { struct nvme_namespace *ns; - int i, status; + int i; - for (i = 0; i < ctrlr->cdata.nn; i++) { + for (i = 0; i < min(ctrlr->cdata.nn, NVME_MAX_NAMESPACES); i++) { ns = &ctrlr->ns[i]; - status = nvme_ns_construct(ns, i+1, ctrlr); - if (status != 0) - return (status); + nvme_ns_construct(ns, i+1, ctrlr); } return (0); Modified: head/sys/dev/nvme/nvme_ns.c ============================================================================== --- head/sys/dev/nvme/nvme_ns.c Tue Mar 7 21:44:05 2017 (r314883) +++ head/sys/dev/nvme/nvme_ns.c Tue Mar 7 21:47:54 2017 (r314884) @@ -512,13 +512,22 @@ nvme_ns_construct(struct nvme_namespace } /* + * If the size of is zero, chances are this isn't a valid + * namespace (eg one that's not been configured yet). The + * standard says the entire id will be zeros, so this is a + * cheap way to test for that. + */ + if (ns->data.nsze == 0) + return (ENXIO); + + /* * Note: format is a 0-based value, so > is appropriate here, * not >=. */ if (ns->data.flbas.format > ns->data.nlbaf) { printf("lba format %d exceeds number supported (%d)\n", ns->data.flbas.format, ns->data.nlbaf+1); - return (1); + return (ENXIO); } if (ctrlr->cdata.oncs.dsm)