BHYVE: OS-6740 bhyve vtd leaks mapping resources

Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
Approved by: Patrick Mooney <patrick.mooney@joyent.com>

usr/src/uts/i86pc/Makefile.files

@@ -262,6 +262,7 @@ VMM_OBJS += vmm.o \
 	vmx.o \
 	vmx_support.o \
 	vtd.o \
+	vtd_sol.o \
 	svm.o \
 	svm_msr.o \
 	npt.o \

usr/src/uts/i86pc/io/vmm/intel/vtd.c

@@ -122,6 +122,9 @@ static int		drhd_num;
 static struct vtdmap	*vtdmaps[DRHD_MAX_UNITS];
 static int		max_domains;
 typedef int		(*drhd_ident_func_t)(void);
+#ifndef __FreeBSD__
+static dev_info_t	*vtddips[DRHD_MAX_UNITS];
+#endif
 
 static uint64_t root_table[PAGE_SIZE / sizeof(uint64_t)] __aligned(4096);
 static uint64_t ctx_tables[256][PAGE_SIZE / sizeof(uint64_t)] __aligned(4096);
@@ -240,11 +243,8 @@ vtd_translation_disable(struct vtdmap *vtdmap)
 }
 
 static void *
-vtd_map(ACPI_DMAR_HARDWARE_UNIT *drhd, int unit)
+vtd_map(dev_info_t *dip)
 {
-	struct ddi_parent_private_data *pdptr;
-	struct regspec reg;
-	dev_info_t *dip;
 	caddr_t regs;
 	ddi_acc_handle_t hdl;
 	int error;
@@ -255,61 +255,33 @@ vtd_map(ACPI_DMAR_HARDWARE_UNIT *drhd, int unit)
 		DDI_STRICTORDER_ACC,
 	};
 
-	dip = ddi_add_child(ddi_root_node(), "vtd",
-	    DEVI_SID_NODEID, unit);
-
-#if 0
-	drhd->dr_dip = dip;
-#endif
-
-	reg.regspec_bustype = 0;
-	reg.regspec_addr = drhd->Address;
-	reg.regspec_size = PAGE_SIZE;
-
-	/*
-	 * update the reg properties
-	 *
-	 *   reg property will be used for register
-	 *   set access
-	 *
-	 * refer to the bus_map of root nexus driver
-	 * I/O or memory mapping:
-	 *
-	 * <bustype=0, addr=x, len=x>: memory
-	 * <bustype=1, addr=x, len=x>: i/o
-	 * <bustype>1, addr=0, len=x>: x86-compatibility i/o
-	 */
-	(void) ndi_prop_update_int_array(DDI_DEV_T_NONE,
-	    dip, "reg", (int *)&reg,
-	    sizeof (struct regspec) / sizeof (int));
-
-	/*
-	 * This is an artificially constructed dev_info, and we
-	 * need to set a few more things to be able to use it
-	 * for ddi_dma_alloc_handle/free_handle.
-	 */
-	ddi_set_driver(dip, ddi_get_driver(ddi_root_node()));
-	DEVI(dip)->devi_bus_dma_allochdl =
-	    DEVI(ddi_get_driver((ddi_root_node())));
-
-	pdptr = kmem_zalloc(sizeof (struct ddi_parent_private_data)
-	    + sizeof (struct regspec), KM_SLEEP);
-	pdptr->par_nreg = 1;
-	pdptr->par_reg = (struct regspec *)(pdptr + 1);
-	pdptr->par_reg->regspec_bustype = 0;
-	pdptr->par_reg->regspec_addr = drhd->Address;
-	pdptr->par_reg->regspec_size = PAGE_SIZE;
-	ddi_set_parent_data(dip, pdptr);
-
 	error = ddi_regs_map_setup(dip, 0, &regs, 0, PAGE_SIZE, &regs_attr,
 	    &hdl);
 
 	if (error != DDI_SUCCESS)
 		return (NULL);
 
+	ddi_set_driver_private(dip, hdl);
+
 	return (regs);
 }
 
+static void
+vtd_unmap(dev_info_t *dip)
+{
+	ddi_acc_handle_t hdl = ddi_get_driver_private(dip);
+
+	if (hdl != NULL)
+		ddi_regs_map_free(&hdl);
+}
+
+#ifndef __FreeBSD__
+/*
+ * This lives in vtd_sol.c for license reasons.
+ */
+extern dev_info_t *vtd_get_dip(ACPI_DMAR_HARDWARE_UNIT *, int);
+#endif
+
 static int
 vtd_init(void)
 {
@@ -373,7 +345,10 @@ vtd_init(void)
 #ifdef __FreeBSD__
 		vtdmaps[units++] = (struct vtdmap *)PHYS_TO_DMAP(drhd->Address);
 #else
-		vtdmaps[units] = (struct vtdmap *)vtd_map(drhd, units);
+		vtddips[units] = vtd_get_dip(drhd, units);
+		vtdmaps[units] = (struct vtdmap *)vtd_map(vtddips[units]);
+		if (vtdmaps[units] == NULL)
+			goto fail;
 		units++;
 #endif
 		if (units >= DRHD_MAX_UNITS)
@@ -407,11 +382,36 @@ vtd_init(void)
 	}
 
 	return (0);
+
+#ifndef __FreeBSD__
+fail:
+	for (i = 0; i <= units; i++)
+		vtd_unmap(vtddips[i]);
+	return (ENXIO);
+#endif
 }
 
 static void
 vtd_cleanup(void)
 {
+#ifndef __FreeBSD__
+	int i;
+
+	KASSERT(SLIST_EMPTY(&domhead), ("domain list not empty"));
+
+	bzero(root_table, sizeof (root_table));
+
+	for (i = 0; i <= drhd_num; i++) {
+		vtdmaps[i] = NULL;
+		/*
+		 * Unmap the vtd registers. Note that the devinfo nodes
+		 * themselves aren't removed, they are considered system state
+		 * and can be reused when the module is reloaded.
+		 */
+		if (vtddips[i] != NULL)
+			vtd_unmap(vtddips[i]);
+	}
+#endif
 }
 
 static void

usr/src/uts/i86pc/io/vmm/intel/vtd_sol.c

@@ -0,0 +1,83 @@
+/*
+ * This file and its contents are supplied under the terms of the
+ * Common Development and Distribution License ("CDDL"), version 1.0.
+ * You may only use this file in accordance with the terms of version
+ * 1.0 of the CDDL.
+ *
+ * A full copy of the text of the CDDL should have accompanied this
+ * source.  A copy of the CDDL is also available via the Internet at
+ * http://www.illumos.org/license/CDDL.
+ */
+
+/*
+ * Copyright 2018 Joyent, Inc.
+ */
+
+#include <sys/sunndi.h>
+#include <contrib/dev/acpica/include/acpi.h>
+
+dev_info_t *
+vtd_get_dip(ACPI_DMAR_HARDWARE_UNIT *drhd, int unit)
+{
+	dev_info_t *dip;
+	struct ddi_parent_private_data *pdptr;
+	struct regspec reg;
+	int circ;
+
+	/*
+	 * Try to find an existing devinfo node for this vtd unit.
+	 */
+	ndi_devi_enter(ddi_root_node(), &circ);
+	dip = ddi_find_devinfo("vtd", unit, 0);
+	ndi_devi_exit(ddi_root_node(), circ);
+
+	if (dip != NULL)
+		return (dip);
+
+	/*
+	 * None found, construct a devinfo node for this vtd unit.
+	 */
+	dip = ddi_add_child(ddi_root_node(), "vtd",
+	    DEVI_SID_NODEID, unit);
+
+	reg.regspec_bustype = 0;
+	reg.regspec_addr = drhd->Address;
+	reg.regspec_size = PAGE_SIZE;
+
+	/*
+	 * update the reg properties
+	 *
+	 *   reg property will be used for register
+	 *   set access
+	 *
+	 * refer to the bus_map of root nexus driver
+	 * I/O or memory mapping:
+	 *
+	 * <bustype=0, addr=x, len=x>: memory
+	 * <bustype=1, addr=x, len=x>: i/o
+	 * <bustype>1, addr=0, len=x>: x86-compatibility i/o
+	 */
+	(void) ndi_prop_update_int_array(DDI_DEV_T_NONE,
+	    dip, "reg", (int *)&reg,
+	    sizeof (struct regspec) / sizeof (int));
+
+	/*
+	 * This is an artificially constructed dev_info, and we
+	 * need to set a few more things to be able to use it
+	 * for ddi_dma_alloc_handle/free_handle.
+	 */
+	ddi_set_driver(dip, ddi_get_driver(ddi_root_node()));
+	DEVI(dip)->devi_bus_dma_allochdl =
+	    DEVI(ddi_get_driver((ddi_root_node())));
+
+	pdptr = kmem_zalloc(sizeof (struct ddi_parent_private_data)
+	    + sizeof (struct regspec), KM_SLEEP);
+	pdptr->par_nreg = 1;
+	pdptr->par_reg = (struct regspec *)(pdptr + 1);
+	pdptr->par_reg->regspec_bustype = 0;
+	pdptr->par_reg->regspec_addr = drhd->Address;
+	pdptr->par_reg->regspec_size = PAGE_SIZE;
+	ddi_set_parent_data(dip, pdptr);
+
+	return (dip);
+}

usr/src/uts/i86pc/io/vmm/io/iommu.c

@@ -68,6 +68,10 @@ static void *host_domain;
 static eventhandler_tag add_tag, delete_tag;
 #endif
 
+#ifndef __FreeBSD__
+static volatile u_int iommu_initted;
+#endif
+
 static __inline int
 IOMMU_INIT(void)
 {
@@ -179,14 +183,22 @@ iommu_pci_delete(void *arg, device_t dev)
 }
 #endif
 
+#ifndef __FreeBSD__
 static int
-iommu_find_device(dev_info_t *dip, void *unused)
+iommu_find_device(dev_info_t *dip, void *arg)
 {
-	if (pcie_is_pci_device(dip))
-		iommu_add_device(host_domain, pci_get_rid(dip));
+	boolean_t add = (boolean_t)arg;
+
+	if (pcie_is_pci_device(dip)) {
+		if (add)
+			iommu_add_device(host_domain, pci_get_rid(dip));
+		else
+			iommu_remove_device(host_domain, pci_get_rid(dip));
+	}
 
 	return (DDI_WALK_CONTINUE);
 }
+#endif
 
 static void
 iommu_init(void)
@@ -260,7 +272,7 @@ iommu_init(void)
 		}
 	}
 #else
-	ddi_walk_devs(ddi_root_node(), iommu_find_device, NULL);
+	ddi_walk_devs(ddi_root_node(), iommu_find_device, (void *)B_TRUE);
 #endif
 	IOMMU_ENABLE();
 
@@ -278,17 +290,23 @@ iommu_cleanup(void)
 		EVENTHANDLER_DEREGISTER(pci_delete_device, delete_tag);
 		delete_tag = NULL;
 	}
+#else
+	atomic_store_rel_int(&iommu_initted, 0);
 #endif
 	IOMMU_DISABLE();
+#ifndef __FreeBSD__
+	ddi_walk_devs(ddi_root_node(), iommu_find_device, (void *)B_FALSE);
+#endif
 	IOMMU_DESTROY_DOMAIN(host_domain);
 	IOMMU_CLEANUP();
+#ifndef __FreeBSD__
+	ops = NULL;
+#endif
 }
 
 void *
 iommu_create_domain(vm_paddr_t maxaddr)
 {
-	static volatile u_int iommu_initted;
-
 	if (iommu_initted < 2) {
 		if (atomic_cmpset_int(&iommu_initted, 0, 1)) {
 			iommu_init();

usr/src/uts/i86pc/io/vmm/vmm.c

@@ -497,6 +497,7 @@ vmm_mod_unload()
 {
 	int	error;
 
+	iommu_cleanup();
 	error = VMM_CLEANUP();
 	if (error)
 		return (error);