Skip site navigation (1)Skip section navigation (2) 
Date:      Mon, 14 Dec 2020 20:48:59 +0000 (UTC)
From:      John Baldwin <jhb@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org
Subject:   svn commit: r368642 - stable/12/sys/amd64/vmm/io
Message-ID:  <202012142048.0BEKmxpM003628@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: jhb
Date: Mon Dec 14 20:48:59 2020
New Revision: 368642
URL: https://svnweb.freebsd.org/changeset/base/368642

Log:
  MFC 368004: Pull the check for VM ownership into ppt_find().
  
  This reduces some code duplication.  One behavior change is that
  ppt_assign_device() will now only succeed if the device is unowned.
  Previously, a device could be assigned to the same VM multiple times,
  but each time it was assigned, the device's state was reset.

Modified:
  stable/12/sys/amd64/vmm/io/ppt.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/12/sys/amd64/vmm/io/ppt.c
==============================================================================
--- stable/12/sys/amd64/vmm/io/ppt.c	Mon Dec 14 20:40:21 2020	(r368641)
+++ stable/12/sys/amd64/vmm/io/ppt.c	Mon Dec 14 20:48:59 2020	(r368642)
@@ -198,8 +198,8 @@ static devclass_t ppt_devclass;
 DEFINE_CLASS_0(ppt, ppt_driver, ppt_methods, sizeof(struct pptdev));
 DRIVER_MODULE(ppt, pci, ppt_driver, ppt_devclass, NULL, NULL);
 
-static struct pptdev *
-ppt_find(int bus, int slot, int func)
+static int
+ppt_find(struct vm *vm, int bus, int slot, int func, struct pptdev **pptp)
 {
 	device_t dev;
 	struct pptdev *ppt;
@@ -211,9 +211,15 @@ ppt_find(int bus, int slot, int func)
 		s = pci_get_slot(dev);
 		f = pci_get_function(dev);
 		if (bus == b && slot == s && func == f)
-			return (ppt);
+			break;
 	}
-	return (NULL);
+
+	if (ppt == NULL)
+		return (ENOENT);
+	if (ppt->vm != vm)		/* Make sure we own this device */
+		return (EBUSY);
+	*pptp = ppt;
+	return (0);
 }
 
 static void
@@ -377,50 +383,40 @@ int
 ppt_assign_device(struct vm *vm, int bus, int slot, int func)
 {
 	struct pptdev *ppt;
+	int error;
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt != NULL) {
-		/*
-		 * If this device is owned by a different VM then we
-		 * cannot change its owner.
-		 */
-		if (ppt->vm != NULL && ppt->vm != vm)
-			return (EBUSY);
+	/* Passing NULL requires the device to be unowned. */
+	error = ppt_find(NULL, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
-		pci_save_state(ppt->dev);
-		ppt_pci_reset(ppt->dev);
-		pci_restore_state(ppt->dev);
-		ppt->vm = vm;
-		iommu_add_device(vm_iommu_domain(vm), pci_get_rid(ppt->dev));
-		return (0);
-	}
-	return (ENOENT);
+	pci_save_state(ppt->dev);
+	ppt_pci_reset(ppt->dev);
+	pci_restore_state(ppt->dev);
+	ppt->vm = vm;
+	iommu_add_device(vm_iommu_domain(vm), pci_get_rid(ppt->dev));
+	return (0);
 }
 
 int
 ppt_unassign_device(struct vm *vm, int bus, int slot, int func)
 {
 	struct pptdev *ppt;
+	int error;
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt != NULL) {
-		/*
-		 * If this device is not owned by this 'vm' then bail out.
-		 */
-		if (ppt->vm != vm)
-			return (EBUSY);
+	error = ppt_find(vm, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
-		pci_save_state(ppt->dev);
-		ppt_pci_reset(ppt->dev);
-		pci_restore_state(ppt->dev);
-		ppt_unmap_mmio(vm, ppt);
-		ppt_teardown_msi(ppt);
-		ppt_teardown_msix(ppt);
-		iommu_remove_device(vm_iommu_domain(vm), pci_get_rid(ppt->dev));
-		ppt->vm = NULL;
-		return (0);
-	}
-	return (ENOENT);
+	pci_save_state(ppt->dev);
+	ppt_pci_reset(ppt->dev);
+	pci_restore_state(ppt->dev);
+	ppt_unmap_mmio(vm, ppt);
+	ppt_teardown_msi(ppt);
+	ppt_teardown_msix(ppt);
+	iommu_remove_device(vm_iommu_domain(vm), pci_get_rid(ppt->dev));
+	ppt->vm = NULL;
+	return (0);
 }
 
 int
@@ -451,25 +447,22 @@ ppt_map_mmio(struct vm *vm, int bus, int slot, int fun
 	struct pptseg *seg;
 	struct pptdev *ppt;
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt != NULL) {
-		if (ppt->vm != vm)
-			return (EBUSY);
+	error = ppt_find(vm, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
-		for (i = 0; i < MAX_MMIOSEGS; i++) {
-			seg = &ppt->mmio[i];
-			if (seg->len == 0) {
-				error = vm_map_mmio(vm, gpa, len, hpa);
-				if (error == 0) {
-					seg->gpa = gpa;
-					seg->len = len;
-				}
-				return (error);
+	for (i = 0; i < MAX_MMIOSEGS; i++) {
+		seg = &ppt->mmio[i];
+		if (seg->len == 0) {
+			error = vm_map_mmio(vm, gpa, len, hpa);
+			if (error == 0) {
+				seg->gpa = gpa;
+				seg->len = len;
 			}
+			return (error);
 		}
-		return (ENOSPC);
 	}
-	return (ENOENT);
+	return (ENOSPC);
 }
 
 static int
@@ -511,11 +504,9 @@ ppt_setup_msi(struct vm *vm, int vcpu, int bus, int sl
 	if (numvec < 0 || numvec > MAX_MSIMSGS)
 		return (EINVAL);
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt == NULL)
-		return (ENOENT);
-	if (ppt->vm != vm)		/* Make sure we own this device */
-		return (EBUSY);
+	error = ppt_find(vm, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
 	/* Reject attempts to enable MSI while MSI-X is active. */
 	if (ppt->msix.num_msgs != 0 && numvec != 0)
@@ -604,11 +595,9 @@ ppt_setup_msix(struct vm *vm, int vcpu, int bus, int s
 	int numvec, alloced, rid, error;
 	size_t res_size, cookie_size, arg_size;
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt == NULL)
-		return (ENOENT);
-	if (ppt->vm != vm)		/* Make sure we own this device */
-		return (EBUSY);
+	error = ppt_find(vm, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
 	/* Reject attempts to enable MSI-X while MSI is active. */
 	if (ppt->msi.num_msgs != 0)
@@ -712,12 +701,11 @@ int
 ppt_disable_msix(struct vm *vm, int bus, int slot, int func)
 {
 	struct pptdev *ppt;
+	int error;
 
-	ppt = ppt_find(bus, slot, func);
-	if (ppt == NULL)
-		return (ENOENT);
-	if (ppt->vm != vm)		/* Make sure we own this device */
-		return (EBUSY);
+	error = ppt_find(vm, bus, slot, func, &ppt);
+	if (error)
+		return (error);
 
 	ppt_teardown_msix(ppt);
 	return (0);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202012142048.0BEKmxpM003628>