From 728c2edfcf14b3b61bd0ff82894f03455ca0e7d7 Mon Sep 17 00:00:00 2001 From: Jason Andryuk <jandryuk@gmail.com> Date: Mon, 29 Aug 2022 11:15:36 -0400 Subject: xen-pcifront: Handle missed Connected state An HVM guest with linux stubdomain and 2 PCI devices failed to start as libxl timed out waiting for the PCI devices to be added. It happens intermittently but with some regularity. libxl wrote the two xenstore entries for the devices, but then timed out waiting for backend state 4 (Connected) - the state stayed at 7 (Reconfiguring). (PCI passthrough to an HVM with stubdomain is PV passthrough to the stubdomain and then HVM passthrough with the QEMU inside the stubdomain.) The stubdomain kernel never printed "pcifront pci-0: Installing PCI frontend", so it seems to have missed state 4 which would have called pcifront_try_connect() -> pcifront_connect_and_init_dma() Have pcifront_detach_devices() special-case state Initialised and call pcifront_connect_and_init_dma(). Don't use pcifront_try_connect() because that sets the xenbus state which may throw off the backend. After connecting, skip the remainder of detach_devices since none have been initialized yet. When the backend switches to Reconfigured, pcifront_attach_devices() will pick them up again. Signed-off-by: Jason Andryuk <jandryuk@gmail.com> Reviewed-by: Juergen Gross <jgross@suse.com> Link: https://lore.kernel.org/r/20220829151536.8578-1-jandryuk@gmail.com Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/pci/xen-pcifront.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index 689271c4245c..77e61b470121 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -981,13 +981,26 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) { int err = 0; int i, num_devs; + enum xenbus_state state; unsigned int domain, bus, slot, func; struct pci_dev *pci_dev; char str[64]; - if (xenbus_read_driver_state(pdev->xdev->nodename) != - XenbusStateConnected) + state = xenbus_read_driver_state(pdev->xdev->nodename); + if (state == XenbusStateInitialised) { + dev_dbg(&pdev->xdev->dev, "Handle skipped connect.\n"); + /* We missed Connected and need to initialize. */ + err = pcifront_connect_and_init_dma(pdev); + if (err && err != -EEXIST) { + xenbus_dev_fatal(pdev->xdev, err, + "Error setting up PCI Frontend"); + goto out; + } + + goto out_switch_state; + } else if (state != XenbusStateConnected) { goto out; + } err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, "num_devs", "%d", &num_devs); @@ -1048,6 +1061,7 @@ static int pcifront_detach_devices(struct pcifront_device *pdev) domain, bus, slot, func); } + out_switch_state: err = xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring); out: -- cgit v1.2.3 From 2849752f36848359034616eb70dfc7fb14eb3cd4 Mon Sep 17 00:00:00 2001 From: Juergen Gross <jgross@suse.com> Date: Thu, 6 Oct 2022 10:50:28 +0200 Subject: xen/pcifront: move xenstore config scanning into sub-function pcifront_try_connect() and pcifront_attach_devices() share a large chunk of duplicated code for reading the config information from Xenstore, which only differs regarding calling pcifront_rescan_root() or pcifront_scan_root(). Put that code into a new sub-function. It is fine to always call pcifront_rescan_root() from that common function, as it will fallback to pcifront_scan_root() if the domain/bus combination isn't known yet (and pcifront_scan_root() should never be called for an already known domain/bus combination anyway). In order to avoid duplicate messages for the fallback case move the check for domain/bus not known to the beginning of pcifront_rescan_root(). While at it fix the error reporting in case the root-xx node had the wrong format. As the return value of pcifront_try_connect() and pcifront_attach_devices() are not used anywhere make those functions return void. As an additional bonus this removes the dubious return of -EFAULT in case of an unexpected driver state. Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jason Andryuk <jandryuk@gmail.com> Signed-off-by: Juergen Gross <jgross@suse.com> --- drivers/pci/xen-pcifront.c | 143 ++++++++++++--------------------------------- 1 file changed, 37 insertions(+), 106 deletions(-) (limited to 'drivers/pci') diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index 77e61b470121..7378e2f3e525 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -521,24 +521,14 @@ static int pcifront_rescan_root(struct pcifront_device *pdev, int err; struct pci_bus *b; -#ifndef CONFIG_PCI_DOMAINS - if (domain != 0) { - dev_err(&pdev->xdev->dev, - "PCI Root in non-zero PCI Domain! domain=%d\n", domain); - dev_err(&pdev->xdev->dev, - "Please compile with CONFIG_PCI_DOMAINS\n"); - return -EINVAL; - } -#endif - - dev_info(&pdev->xdev->dev, "Rescanning PCI Frontend Bus %04x:%02x\n", - domain, bus); - b = pci_find_bus(domain, bus); if (!b) /* If the bus is unknown, create it. */ return pcifront_scan_root(pdev, domain, bus); + dev_info(&pdev->xdev->dev, "Rescanning PCI Frontend Bus %04x:%02x\n", + domain, bus); + err = pcifront_scan_bus(pdev, domain, bus, b); /* Claim resources before going "live" with our devices */ @@ -819,76 +809,73 @@ out: return err; } -static int pcifront_try_connect(struct pcifront_device *pdev) +static void pcifront_connect(struct pcifront_device *pdev) { - int err = -EFAULT; + int err; int i, num_roots, len; char str[64]; unsigned int domain, bus; - - /* Only connect once */ - if (xenbus_read_driver_state(pdev->xdev->nodename) != - XenbusStateInitialised) - goto out; - - err = pcifront_connect_and_init_dma(pdev); - if (err && err != -EEXIST) { - xenbus_dev_fatal(pdev->xdev, err, - "Error setting up PCI Frontend"); - goto out; - } - err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, "root_num", "%d", &num_roots); if (err == -ENOENT) { xenbus_dev_error(pdev->xdev, err, "No PCI Roots found, trying 0000:00"); - err = pcifront_scan_root(pdev, 0, 0); + err = pcifront_rescan_root(pdev, 0, 0); if (err) { xenbus_dev_fatal(pdev->xdev, err, "Error scanning PCI root 0000:00"); - goto out; + return; } num_roots = 0; } else if (err != 1) { - if (err == 0) - err = -EINVAL; - xenbus_dev_fatal(pdev->xdev, err, + xenbus_dev_fatal(pdev->xdev, err >= 0 ? -EINVAL : err, "Error reading number of PCI roots"); - goto out; + return; } for (i = 0; i < num_roots; i++) { len = snprintf(str, sizeof(str), "root-%d", i); - if (unlikely(len >= (sizeof(str) - 1))) { - err = -ENOMEM; - goto out; - } + if (unlikely(len >= (sizeof(str) - 1))) + return; err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, str, "%x:%x", &domain, &bus); if (err != 2) { - if (err >= 0) - err = -EINVAL; - xenbus_dev_fatal(pdev->xdev, err, + xenbus_dev_fatal(pdev->xdev, err >= 0 ? -EINVAL : err, "Error reading PCI root %d", i); - goto out; + return; } - err = pcifront_scan_root(pdev, domain, bus); + err = pcifront_rescan_root(pdev, domain, bus); if (err) { xenbus_dev_fatal(pdev->xdev, err, "Error scanning PCI root %04x:%02x", domain, bus); - goto out; + return; } } - err = xenbus_switch_state(pdev->xdev, XenbusStateConnected); + xenbus_switch_state(pdev->xdev, XenbusStateConnected); +} -out: - return err; +static void pcifront_try_connect(struct pcifront_device *pdev) +{ + int err; + + /* Only connect once */ + if (xenbus_read_driver_state(pdev->xdev->nodename) != + XenbusStateInitialised) + return; + + err = pcifront_connect_and_init_dma(pdev); + if (err && err != -EEXIST) { + xenbus_dev_fatal(pdev->xdev, err, + "Error setting up PCI Frontend"); + return; + } + + pcifront_connect(pdev); } static int pcifront_try_disconnect(struct pcifront_device *pdev) @@ -914,67 +901,11 @@ out: return err; } -static int pcifront_attach_devices(struct pcifront_device *pdev) +static void pcifront_attach_devices(struct pcifront_device *pdev) { - int err = -EFAULT; - int i, num_roots, len; - unsigned int domain, bus; - char str[64]; - - if (xenbus_read_driver_state(pdev->xdev->nodename) != + if (xenbus_read_driver_state(pdev->xdev->nodename) == XenbusStateReconfiguring) - goto out; - - err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, - "root_num", "%d", &num_roots); - if (err == -ENOENT) { - xenbus_dev_error(pdev->xdev, err, - "No PCI Roots found, trying 0000:00"); - err = pcifront_rescan_root(pdev, 0, 0); - if (err) { - xenbus_dev_fatal(pdev->xdev, err, - "Error scanning PCI root 0000:00"); - goto out; - } - num_roots = 0; - } else if (err != 1) { - if (err == 0) - err = -EINVAL; - xenbus_dev_fatal(pdev->xdev, err, - "Error reading number of PCI roots"); - goto out; - } - - for (i = 0; i < num_roots; i++) { - len = snprintf(str, sizeof(str), "root-%d", i); - if (unlikely(len >= (sizeof(str) - 1))) { - err = -ENOMEM; - goto out; - } - - err = xenbus_scanf(XBT_NIL, pdev->xdev->otherend, str, - "%x:%x", &domain, &bus); - if (err != 2) { - if (err >= 0) - err = -EINVAL; - xenbus_dev_fatal(pdev->xdev, err, - "Error reading PCI root %d", i); - goto out; - } - - err = pcifront_rescan_root(pdev, domain, bus); - if (err) { - xenbus_dev_fatal(pdev->xdev, err, - "Error scanning PCI root %04x:%02x", - domain, bus); - goto out; - } - } - - xenbus_switch_state(pdev->xdev, XenbusStateConnected); - -out: - return err; + pcifront_connect(pdev); } static int pcifront_detach_devices(struct pcifront_device *pdev) -- cgit v1.2.3