On 21.06.23 15:06, Jan Beulich wrote:
Hello all
On 13.06.2023 12:32, Volodymyr Babchuk wrote:
@@ -121,6 +124,62 @@ int vpci_add_handlers(struct pci_dev *pdev)
}
#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+static int add_virtual_device(struct pci_dev *pdev)
+{
+ struct domain *d = pdev->domain;
+ pci_sbdf_t sbdf = { 0 };
+ unsigned long new_dev_number;
+
+ if ( is_hardware_domain(d) )
+ return 0;
+
+ ASSERT(pcidevs_locked());
+
+ /*
+ * Each PCI bus supports 32 devices/slots at max or up to 256 when
+ * there are multi-function ones which are not yet supported.
+ */
+ if ( pdev->info.is_extfn )
+ {
+ gdprintk(XENLOG_ERR, "%pp: only function 0 passthrough supported\n",
+ &pdev->sbdf);
+ return -EOPNOTSUPP;
+ }
+
+ new_dev_number = find_first_zero_bit(d->vpci_dev_assigned_map,
+ VPCI_MAX_VIRT_DEV);
+ if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
+ return -ENOSPC;
+
+ __set_bit(new_dev_number, &d->vpci_dev_assigned_map);
Since the find-and-set can't easily be atomic, the lock used here (
asserted to be held above) needs to be the same as ...
+ /*
+ * Both segment and bus number are 0:
+ * - we emulate a single host bridge for the guest, e.g. segment 0
+ * - with bus 0 the virtual devices are seen as embedded
+ * endpoints behind the root complex
+ *
+ * TODO: add support for multi-function devices.
+ */
+ sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
+ pdev->vpci->guest_sbdf = sbdf;
+
+ return 0;
+
+}
+
+static void vpci_remove_virtual_device(const struct pci_dev *pdev)
+{
+ write_lock(&pdev->domain->vpci_rwlock);
+ if ( pdev->vpci )
+ {
+ __clear_bit(pdev->vpci->guest_sbdf.dev,
+ &pdev->domain->vpci_dev_assigned_map);
+ pdev->vpci->guest_sbdf.sbdf = ~0;
+ }
+ write_unlock(&pdev->domain->vpci_rwlock);
... the one used here.
I think, it makes sense, yes.
***
There is one more thing. As far as I remember, there were some requests
provided for the previous version (also v7) [1]. At least one of them, I
assume, is still applicable here. I am speaking about a request to
consider moving "cleaning up guest_sbdf / vpci_dev_assigned_map" into
vpci_remove_device() here and aliasing of vpci_deassign_device() to
vpci_remove_device() in commit #03/12.
The diff below (to be applied on top of current patch) is my
understanding (not even build tested):
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index a61282cc5b..c3e6c153bc 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -51,6 +51,15 @@ void vpci_remove_device(struct pci_dev *pdev)
return;
}
+#ifdef CONFIG_HAS_VPCI_GUEST_SUPPORT
+ if ( pdev->vpci->guest_sbdf.sbdf != ~0 )
+ {
+ __clear_bit(pdev->vpci->guest_sbdf.dev,
+ &pdev->domain->vpci_dev_assigned_map);
+ pdev->vpci->guest_sbdf.sbdf = ~0;
+ }
+#endif
+
vpci = pdev->vpci;
pdev->vpci = NULL;
write_unlock(&pdev->domain->vpci_rwlock);
@@ -152,10 +161,14 @@ static int add_virtual_device(struct pci_dev *pdev)
return -EOPNOTSUPP;
}
+ write_lock(&pdev->domain->vpci_rwlock);
new_dev_number = find_first_zero_bit(d->vpci_dev_assigned_map,
VPCI_MAX_VIRT_DEV);
if ( new_dev_number >= VPCI_MAX_VIRT_DEV )
+ {
+ write_unlock(&pdev->domain->vpci_rwlock);
return -ENOSPC;
+ }
__set_bit(new_dev_number, &d->vpci_dev_assigned_map);
@@ -169,23 +182,12 @@ static int add_virtual_device(struct pci_dev *pdev)
*/
sbdf.devfn = PCI_DEVFN(new_dev_number, 0);
pdev->vpci->guest_sbdf = sbdf;
+ write_unlock(&pdev->domain->vpci_rwlock);
return 0;
}
-static void vpci_remove_virtual_device(const struct pci_dev *pdev)
-{
- write_lock(&pdev->domain->vpci_rwlock);
- if ( pdev->vpci )
- {
- __clear_bit(pdev->vpci->guest_sbdf.dev,
- &pdev->domain->vpci_dev_assigned_map);
- pdev->vpci->guest_sbdf.sbdf = ~0;
- }
- write_unlock(&pdev->domain->vpci_rwlock);
-}
-
/* Notify vPCI that device is assigned to guest. */
int vpci_assign_device(struct pci_dev *pdev)
{
@@ -215,7 +217,6 @@ void vpci_deassign_device(struct pci_dev *pdev)
if ( !has_vpci(pdev->domain) )
return;
- vpci_remove_virtual_device(pdev);
vpci_remove_device(pdev);
}
#endif /* CONFIG_HAS_VPCI_GUEST_SUPPORT */
(END)
[1]
https://lore.kernel.org/xen-devel/[email protected]/
https://lore.kernel.org/xen-devel/[email protected]/
Jan
