hyperv/storvsc: Fix the blkvsc disk attachment issues.

- The original 'disengage' ATA controller model does not work properly
  for all possible disk configurations.  Use the newly added ATA disk
  veto eventhandler to fit into all possible disk configuration.
- If the 'invalid LUN' happens on blkvsc controllers, return
  CAM_DEV_NOT_THERE so that CAM will not destroy attached disks under
  the blkvsc controllers.

Submitted by:	Hongjiang Zhang <honzhan microsoft com>
Discussed with:	mav
MFC after:	1 week
Sponsored by:	Microsoft
Differential Revision:	https://reviews.freebsd.org/D7693

sys/conf/files.amd64

@@ -294,7 +294,6 @@ dev/hwpmc/hwpmc_x86.c		optional	hwpmc
 dev/hyperv/netvsc/hv_net_vsc.c				optional	hyperv
 dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c		optional	hyperv
 dev/hyperv/netvsc/hv_rndis_filter.c			optional	hyperv
-dev/hyperv/stordisengage/hv_ata_pci_disengage.c		optional	hyperv
 dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c		optional	hyperv
 dev/hyperv/utilities/hv_heartbeat.c			optional	hyperv
 dev/hyperv/utilities/hv_kvp.c				optional	hyperv

sys/conf/files.i386

@@ -251,7 +251,6 @@ dev/hwpmc/hwpmc_x86.c		optional hwpmc
 dev/hyperv/netvsc/hv_net_vsc.c				optional	hyperv
 dev/hyperv/netvsc/hv_netvsc_drv_freebsd.c		optional	hyperv
 dev/hyperv/netvsc/hv_rndis_filter.c			optional	hyperv
-dev/hyperv/stordisengage/hv_ata_pci_disengage.c		optional	hyperv
 dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c		optional	hyperv
 dev/hyperv/utilities/hv_heartbeat.c			optional	hyperv
 dev/hyperv/utilities/hv_kvp.c				optional	hyperv

sys/dev/hyperv/stordisengage/hv_ata_pci_disengage.c

@@ -1,157 +0,0 @@
-/*-
- * Copyright (c) 1998 - 2008 Søren Schmidt <sos@FreeBSD.org>
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer,
- *    without modification, immediately at the beginning of the file.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-/*-
- * Copyright (c) 2009-2013 Microsoft Corp.
- * Copyright (c) 2012 NetApp Inc.
- * Copyright (c) 2012 Citrix Inc.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice unmodified, this list of conditions, and the following
- *    disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
- * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
- * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <sys/cdefs.h>
-__FBSDID("$FreeBSD$");
-
-#include <sys/param.h>
-#include <sys/systm.h>
-#include <sys/kernel.h>
-#include <sys/module.h>
-#include <sys/ata.h>
-#include <sys/bus.h>
-#include <sys/conf.h>
-#include <sys/malloc.h>
-#include <sys/sema.h>
-#include <sys/taskqueue.h>
-#include <vm/uma.h>
-#include <machine/stdarg.h>
-#include <machine/resource.h>
-#include <machine/bus.h>
-#include <sys/rman.h>
-#include <dev/pci/pcivar.h>
-#include <dev/pci/pcireg.h>
-#include <dev/ata/ata-all.h>
-#include <dev/ata/ata-pci.h>
-#include <ata_if.h>
-
-/* prototypes */
-static int hv_ata_pci_probe(device_t dev);
-static int hv_ata_pci_attach(device_t dev);
-static int hv_ata_pci_detach(device_t dev);
-
-/*
- * generic PCI ATA device probe
- */
-static int
-hv_ata_pci_probe(device_t dev)
-{
-	device_t parent = device_get_parent(dev);
-	int ata_disk_enable;
-
-	ata_disk_enable = 0;
-
-	/*
-	 * Don't probe if not running in a Hyper-V environment
-	 */
-	if (vm_guest != VM_GUEST_HV)
-		return (ENXIO);
-
-	if (device_get_unit(parent) != 0 || device_get_ivars(dev) != 0)
-		return (ENXIO);
-
-	/*
-	 * On Hyper-V the default is to use the enlightened driver for
-	 * IDE disks. However, if the user wishes to use the native
-	 * ATA driver, the environment variable
-	 * hw_ata.disk_enable must be explicitly set to 1.
-	 */
-	if (getenv_int("hw.ata.disk_enable", &ata_disk_enable)) {
-		if (bootverbose)
-			device_printf(dev,
-			    "hw.ata.disk_enable flag is disabling Hyper-V"
-			    " ATA driver support\n");
-			return (ENXIO);
-	}
-
-	device_set_desc(dev, "Hyper-V ATA storage disengage driver");
-
-	return (BUS_PROBE_DEFAULT);
-}
-
-static int
-hv_ata_pci_attach(device_t dev)
-{
-
-	return (0);
-}
-
-static int
-hv_ata_pci_detach(device_t dev)
-{
-
-	return (0);
-}
-
-static device_method_t hv_ata_pci_methods[] = {
-	/* device interface */
-	DEVMETHOD(device_probe,	hv_ata_pci_probe),
-	DEVMETHOD(device_attach,	hv_ata_pci_attach),
-	DEVMETHOD(device_detach,	hv_ata_pci_detach),
-	DEVMETHOD(device_shutdown,	bus_generic_shutdown),
-
-	DEVMETHOD_END
-};
-
-devclass_t hv_ata_pci_devclass;
-
-static driver_t hv_ata_pci_disengage_driver = {
-	"ata",
-	hv_ata_pci_methods,
-	0,
-};
-
-DRIVER_MODULE(atapci_dis, atapci, hv_ata_pci_disengage_driver,
-    hv_ata_pci_devclass, NULL, NULL);
-MODULE_VERSION(atapci_dis, 1);
-MODULE_DEPEND(atapci_dis, ata, 1, 1, 1);

sys/dev/hyperv/storvsc/hv_storvsc_drv_freebsd.c

@@ -60,6 +60,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/lock.h>
 #include <sys/sema.h>
 #include <sys/sglist.h>
+#include <sys/eventhandler.h>
 #include <machine/bus.h>
 #include <sys/bus_dma.h>
 
@@ -202,6 +203,7 @@ struct storvsc_softc {
 	struct vmbus_channel		*hs_sel_chan[MAXCPU];
 };
 
+static eventhandler_tag storvsc_handler_tag;
 /*
  * The size of the vmscsi_request has changed in win8. The
  * additional size is for the newly added elements in the
@@ -898,21 +900,15 @@ hv_storvsc_on_channel_callback(struct vmbus_channel *channel, void *xsc)
 static int
 storvsc_probe(device_t dev)
 {
-	int ata_disk_enable = 0;
 	int ret	= ENXIO;
 	
 	switch (storvsc_get_storage_type(dev)) {
 	case DRIVER_BLKVSC:
 		if(bootverbose)
-			device_printf(dev, "DRIVER_BLKVSC-Emulated ATA/IDE probe\n");
+			device_printf(dev,
-		if (!getenv_int("hw.ata.disk_enable", &ata_disk_enable)) {
+			    "Enlightened ATA/IDE detected\n");
-			if(bootverbose)
+		device_set_desc(dev, g_drv_props_table[DRIVER_BLKVSC].drv_desc);
-				device_printf(dev,
+		ret = BUS_PROBE_DEFAULT;
-					"Enlightened ATA/IDE detected\n");
-			device_set_desc(dev, g_drv_props_table[DRIVER_BLKVSC].drv_desc);
-			ret = BUS_PROBE_DEFAULT;
-		} else if(bootverbose)
-			device_printf(dev, "Emulated ATA/IDE set (hw.ata.disk_enable set)\n");
 		break;
 	case DRIVER_STORVSC:
 		if(bootverbose)
@@ -2155,27 +2151,45 @@ storvsc_io_done(struct hv_storvsc_request *reqp)
 	ccb->ccb_h.status &= ~CAM_STATUS_MASK;
 	if (vm_srb->scsi_status == SCSI_STATUS_OK) {
 		const struct scsi_generic *cmd;
-
+		cmd = (const struct scsi_generic *)
+		    ((ccb->ccb_h.flags & CAM_CDB_POINTER) ?
+		     csio->cdb_io.cdb_ptr : csio->cdb_io.cdb_bytes);
 		if (vm_srb->srb_status != SRB_STATUS_SUCCESS) {
-			if (vm_srb->srb_status == SRB_STATUS_INVALID_LUN) {
-				xpt_print(ccb->ccb_h.path, "invalid LUN %d\n",
-				    vm_srb->lun);
-			} else {
-				xpt_print(ccb->ccb_h.path, "Unknown SRB flag: %d\n",
-				    vm_srb->srb_status);
-			}
 			/*
 			 * If there are errors, for example, invalid LUN,
 			 * host will inform VM through SRB status.
 			 */
-			ccb->ccb_h.status |= CAM_SEL_TIMEOUT;
+			if (bootverbose) {
+				if (vm_srb->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",
+					    vm_srb->srb_status,
+					    scsi_op_desc(cmd->opcode, NULL));
+				}
+			}
+
+			/*
+			 * XXX For a selection timeout, all of the LUNs
+			 * on the target will be gone.  It works for SCSI
+			 * disks, but does not work for IDE disks.
+			 *
+			 * For CAM_DEV_NOT_THERE, CAM will only get
+			 * rid of the device(s) specified by the path.
+			 */
+			if (storvsc_get_storage_type(sc->hs_dev) ==
+			    DRIVER_STORVSC)
+				ccb->ccb_h.status |= CAM_SEL_TIMEOUT;
+			else
+				ccb->ccb_h.status |= CAM_DEV_NOT_THERE;
 		} else {
 			ccb->ccb_h.status |= CAM_REQ_CMP;
 		}
 
-		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) {
 			struct scsi_inquiry_data *inq_data =
 			    (struct scsi_inquiry_data *)csio->data_ptr;
@@ -2287,3 +2301,58 @@ storvsc_get_storage_type(device_t dev)
 		return DRIVER_STORVSC;
 	return DRIVER_UNKNOWN;
 }
+
+#define	PCI_VENDOR_INTEL	0x8086
+#define	PCI_PRODUCT_PIIX4	0x7111
+
+static void
+storvsc_ada_probe_veto(void *arg __unused, struct cam_path *path,
+    struct ata_params *ident_buf __unused, int *veto)
+{
+
+	/*
+	 * The ATA disks are shared with the controllers managed
+	 * by this driver, so veto the ATA disks' attachment; the
+	 * ATA disks will be attached as SCSI disks once this driver
+	 * attached.
+	 */
+	if (path->device->protocol == PROTO_ATA) {
+		struct ccb_pathinq cpi;
+
+		bzero(&cpi, sizeof(cpi));
+		xpt_setup_ccb(&cpi.ccb_h, path, CAM_PRIORITY_NONE);
+		cpi.ccb_h.func_code = XPT_PATH_INQ;
+		xpt_action((union ccb *)&cpi);
+		if (cpi.ccb_h.status == CAM_REQ_CMP &&
+		    cpi.hba_vendor == PCI_VENDOR_INTEL &&
+		    cpi.hba_device == PCI_PRODUCT_PIIX4) {
+			(*veto)++;
+			if (bootverbose) {
+				xpt_print(path,
+				    "Disable ATA disks on "
+				    "simulated ATA controller (0x%04x%04x)\n",
+				    cpi.hba_device, cpi.hba_vendor);
+			}
+		}
+	}
+}
+
+static void
+storvsc_sysinit(void *arg __unused)
+{
+	if (vm_guest == VM_GUEST_HV) {
+		storvsc_handler_tag = EVENTHANDLER_REGISTER(ada_probe_veto,
+		    storvsc_ada_probe_veto, NULL, EVENTHANDLER_PRI_ANY);
+	}
+}
+SYSINIT(storvsc_sys_init, SI_SUB_DRIVERS, SI_ORDER_SECOND, storvsc_sysinit,
+    NULL);
+
+static void
+storvsc_sysuninit(void *arg __unused)
+{
+	if (storvsc_handler_tag != NULL)
+		EVENTHANDLER_DEREGISTER(ada_probe_veto, storvsc_handler_tag);
+}
+SYSUNINIT(storvsc_sys_uninit, SI_SUB_DRIVERS, SI_ORDER_SECOND,
+    storvsc_sysuninit, NULL);

sys/modules/hyperv/Makefile

@@ -1,5 +1,5 @@
 # $FreeBSD$
 
-SUBDIR = vmbus netvsc stordisengage storvsc utilities
+SUBDIR = vmbus netvsc storvsc utilities
 
 .include <bsd.subdir.mk>

sys/modules/hyperv/stordisengage/Makefile

@@ -1,9 +0,0 @@
-# $FreeBSD$
-
-.PATH:  ${.CURDIR}/../../../dev/hyperv/stordisengage
-
-KMOD=	hv_ata_pci_disengage
-SRCS=	hv_ata_pci_disengage.c
-SRCS+=	ata_if.h bus_if.h device_if.h pci_if.h
-
-.include <bsd.kmod.mk>