[PATCH v6] vPCI: re-init extended-capabilities when MMCFG availability changed

[Jan Beulich <jbeulich@xxxxxxxx>]

When Dom0 informs us about MMCFG usability, this may change whether
extended capabilities are available (accessible) for devices. Zap what
might be on record, and re-initialize things.

No synchronization is added for the case where devices may already be in
use. That'll need sorting when (a) DomU support was added and (b) DomU-s
may run already while Dom0 / hwdom still boots (dom0less, Hyperlaunch).

vpci_cleanup_capabilities() also shouldn't have used
pci_find_ext_capability(), as already when the function was introduced
extended config space may not have been (properly) accessible anymore,
no matter whether it was during init. Extended capability cleanup hooks
need to cope with being called when the respective capability doesn't
exist (and hence the corresponding ->init() hook was never called).

Fixes: 70e6dace747e ("vpci: Use cleanup to free capability resource during 
deassign")
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v6: Add comment in physdev_check_pci_extcfg(), while dropping the logging
    of a(nother) message there. Drop hwdom restriction from
    vpci_cleanup_capabilities(). In the re-init case don't bail early from
    vpci_init_capabilities(). Simplify assertion and re-order actions in
    vpci_reinit_ext_capabilities().
v5: Don't use pci_find_ext_capability() in vpci_cleanup_capabilities().
    Add assertion in vpci_reinit_ext_capabilities().
v4: Make sure ->cleanup() and ->init() are invoked.
v3: New.

--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -8,6 +8,8 @@
 #include <xen/guest_access.h>
 #include <xen/iocap.h>
 #include <xen/serial.h>
+#include <xen/vpci.h>
+
 #include <asm/current.h>
 #include <asm/io_apic.h>
 #include <asm/msi.h>
@@ -169,8 +171,19 @@ int cf_check physdev_check_pci_extcfg(st
 
     ASSERT(pdev->seg == info->segment);
     if ( pdev->bus >= info->start_bus && pdev->bus <= info->end_bus )
+    {
         pci_check_extcfg(pdev);
 
+        /*
+         * The re-init failing doesn't mean the device becomes entirely non-
+         * functional.  In case of failure, a message was already logged.
+         * Hence don't otherwise act upon failure.
+         *
+         * FIXME: Re-visit when DomU support is added to vPCI.
+         */
+        vpci_reinit_ext_capabilities(pdev);
+    }
+
     return 0;
 }
 #endif /* COMPAT */
--- a/xen/drivers/vpci/cap.c
+++ b/xen/drivers/vpci/cap.c
@@ -285,13 +285,16 @@ static int vpci_init_ext_capability_list
     return 0;
 }
 
-int vpci_init_capabilities(struct pci_dev *pdev)
+int vpci_init_capabilities(struct pci_dev *pdev, bool ext_only)
 {
-    int rc;
+    int rc, accum_rc = 0;
 
-    rc = vpci_init_capability_list(pdev);
-    if ( rc )
-        return rc;
+    if ( !ext_only )
+    {
+        rc = vpci_init_capability_list(pdev);
+        if ( rc )
+            return rc;
+    }
 
     rc = vpci_init_ext_capability_list(pdev);
     if ( rc )
@@ -305,7 +308,7 @@ int vpci_init_capabilities(struct pci_de
         unsigned int pos = 0;
 
         if ( !is_ext )
-            pos = pci_find_cap_offset(pdev->sbdf, cap);
+            pos = !ext_only ? pci_find_cap_offset(pdev->sbdf, cap) : 0;
         else if ( is_hardware_domain(pdev->domain) )
             pos = pci_find_ext_capability(pdev, cap);
 
@@ -341,30 +344,40 @@ int vpci_init_capabilities(struct pci_de
             {
                 printk(XENLOG_ERR "%pd %pp: hide %s cap %u fail rc=%d\n",
                        pdev->domain, &pdev->sbdf, type, cap, rc);
-                return rc;
+
+                /* Best effort for the re-init case. */
+                if ( !ext_only )
+                    return rc;
+
+                if ( !accum_rc )
+                    accum_rc = rc;
             }
         }
     }
 
-    return 0;
+    return accum_rc;
 }
 
-void vpci_cleanup_capabilities(struct pci_dev *pdev)
+void vpci_cleanup_capabilities(struct pci_dev *pdev, bool ext_only)
 {
     for ( unsigned int i = 0; i < NUM_VPCI_INIT; i++ )
     {
         const vpci_capability_t *capability = &__start_vpci_array[i];
         const unsigned int cap = capability->id;
-        unsigned int pos = 0;
 
         if ( !capability->cleanup )
             continue;
 
-        if ( !capability->is_ext )
-            pos = pci_find_cap_offset(pdev->sbdf, cap);
-        else if ( is_hardware_domain(pdev->domain) )
-            pos = pci_find_ext_capability(pdev, cap);
-        if ( pos )
+        /*
+         * Cannot call pci_find_ext_capability() here, as extended config space
+         * may (no longer) be accessible.  As a result, extended capability
+         * ->cleanup() handlers need to cope with being called despite ->init()
+         * never having been called.  Which in turn allows calling them even
+         * for DomU-s, no matter that vpci_init_capabilities() excludes them
+         * there for now.
+         */
+        if ( capability->is_ext
+             || (!ext_only && pci_find_cap_offset(pdev->sbdf, cap)) )
         {
             int rc = capability->cleanup(pdev, false);
 
@@ -376,6 +389,27 @@ void vpci_cleanup_capabilities(struct pc
     }
 }
 
+int vpci_reinit_ext_capabilities(struct pci_dev *pdev)
+{
+    if ( !pdev->vpci )
+        return 0;
+
+    /*
+     * FIXME: DomU support is missing.  For already running domains we may
+     * need to pause them around the entire re-evaluation of extended config
+     * space accessibility.
+     */
+    ASSERT(pdev->domain == hardware_domain);
+
+    if ( vpci_remove_registers(pdev->vpci, PCI_CFG_SPACE_SIZE,
+                               PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) )
+        ASSERT_UNREACHABLE();
+
+    vpci_cleanup_capabilities(pdev, true);
+
+    return vpci_init_capabilities(pdev, true);
+}
+
 /*
  * Local variables:
  * mode: C
--- a/xen/drivers/vpci/private.h
+++ b/xen/drivers/vpci/private.h
@@ -46,8 +46,8 @@ typedef struct {
 
 int __must_check vpci_init_header(struct pci_dev *pdev);
 
-int vpci_init_capabilities(struct pci_dev *pdev);
-void vpci_cleanup_capabilities(struct pci_dev *pdev);
+int vpci_init_capabilities(struct pci_dev *pdev, bool ext_only);
+void vpci_cleanup_capabilities(struct pci_dev *pdev, bool ext_only);
 
 /* Add/remove a register handler. */
 int __must_check vpci_add_register_mask(struct vpci *vpci,
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -102,7 +102,7 @@ void vpci_deassign_device(struct pci_dev
                     &pdev->domain->vpci_dev_assigned_map);
 #endif
 
-    vpci_cleanup_capabilities(pdev);
+    vpci_cleanup_capabilities(pdev, false);
 
     spin_lock(&pdev->vpci->lock);
     while ( !list_empty(&pdev->vpci->handlers) )
@@ -159,7 +159,7 @@ int vpci_assign_device(struct pci_dev *p
     if ( rc )
         goto out;
 
-    rc = vpci_init_capabilities(pdev);
+    rc = vpci_init_capabilities(pdev, false);
 
  out:
     if ( rc )
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -25,6 +25,8 @@ int __must_check vpci_assign_device(stru
 /* Remove all handlers and free vpci related structures. */
 void vpci_deassign_device(struct pci_dev *pdev);
 
+int vpci_reinit_ext_capabilities(struct pci_dev *pdev);
+
 /* Generic read/write handlers for the PCI config space. */
 uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int reg, unsigned int size);
 void vpci_write(pci_sbdf_t sbdf, unsigned int reg, unsigned int size,
@@ -202,6 +204,11 @@ bool vpci_ecam_read(pci_sbdf_t sbdf, uns
 #else /* !CONFIG_HAS_VPCI */
 struct vpci_vcpu {};
 
+static inline int vpci_reinit_ext_capabilities(struct pci_dev *pdev)
+{
+    return 0;
+}
+
 static inline int vpci_assign_device(struct pci_dev *pdev)
 {
     return 0;