[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging] x86/vPCI: tighten locking assertions



commit 13efde94f4af6ec0976b2065350c4cee511eff6a
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Thu Oct 29 09:09:31 2026 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Oct 29 09:09:31 2026 +0100

    x86/vPCI: tighten locking assertions
    
    Already when they were introduced, they seemed overly lax. In particular
    anything invoked solely from vpci_{read,write}() can check that the per-
    domain PCI r/w lock is held. There's no need to permit the alternative of
    holding the global PCI devices lock.
    
    vpci_msi_arch_update()'s sole call site is update_msi(), which in turn is
    solely called from write handling hooks.
    
    vpci_msi_update(), besides being called from vpci_msi_arch_update() (see
    above), has two further call sites:
    - vpci_msi_arch_enable(), called upon control register writes,
    - vpci_msix_arch_enable_entry(), called solely from update_entry(), which
      in turn is again called upon control register writes, plus from
      msix_write(), which read-locks the domain's PCI lock.
    Both arch_enable functions therefore can also have their assertions
    adjusted.
    
    vpci_msi_disable() is called from
    - vpci_msi_arch_disable(), called upon control register writes,
    - vpci_msix_arch_enable_entry(), covered above,
    - vpci_msix_arch_disable_entry(), called update_entry() (see above) and
      upon control register writes.
    
    Furthermore msixtbl_pt_{,un}register() are solely called from
    pt_irq_{create,destroy}_bind(), callers of which (create: when using
    PT_IRQ_TYPE_MSI) acquire the per-domain lock.
    
    As not using ASSERT_PDEV_LIST_IS_READ_LOCKED() isn't unusual anymore, drop
    the comment near the top of vpci_msix_arch_print().
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
---
 xen/arch/x86/hvm/vmsi.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/vmsi.c b/xen/arch/x86/hvm/vmsi.c
index 27b1f089e2..b272924a7e 100644
--- a/xen/arch/x86/hvm/vmsi.c
+++ b/xen/arch/x86/hvm/vmsi.c
@@ -621,7 +621,7 @@ int msixtbl_pt_register(struct domain *d, struct pirq 
*pirq, uint64_t gtable)
     struct msixtbl_entry *entry, *new_entry;
     int r = -EINVAL;
 
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(d);
+    ASSERT(rw_is_locked(&d->pci_lock));
     ASSERT(rw_is_write_locked(&d->event_lock));
 
     if ( !msixtbl_initialised(d) )
@@ -691,7 +691,7 @@ void msixtbl_pt_unregister(struct domain *d, struct pirq 
*pirq)
     struct pci_dev *pdev;
     struct msixtbl_entry *entry;
 
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(d);
+    ASSERT(rw_is_locked(&d->pci_lock));
     ASSERT(rw_is_write_locked(&d->event_lock));
 
     if ( !msixtbl_initialised(d) )
@@ -837,7 +837,7 @@ static int vpci_msi_update(const struct pci_dev *pdev, 
uint32_t data,
 {
     unsigned int i;
 
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(pdev->domain);
+    ASSERT(rw_is_locked(&pdev->domain->pci_lock));
 
     if ( (address & MSI_ADDR_BASE_MASK) != MSI_ADDR_HEADER )
     {
@@ -878,7 +878,7 @@ void vpci_msi_arch_update(struct vpci_msi *msi, const 
struct pci_dev *pdev)
     int rc;
 
     ASSERT(msi->arch.pirq != INVALID_PIRQ);
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(pdev->domain);
+    ASSERT(rw_is_locked(&pdev->domain->pci_lock));
 
     for ( i = 0; i < msi->vectors && msi->arch.bound; i++ )
     {
@@ -930,7 +930,8 @@ int vpci_msi_arch_enable(struct vpci_msi *msi, const struct 
pci_dev *pdev,
     int rc;
 
     ASSERT(msi->arch.pirq == INVALID_PIRQ);
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(pdev->domain);
+    ASSERT(rw_is_locked(&pdev->domain->pci_lock));
+
     rc = vpci_msi_enable(pdev, vectors, 0);
     if ( rc < 0 )
         return rc;
@@ -948,7 +949,7 @@ static void vpci_msi_disable(const struct pci_dev *pdev, 
int pirq,
     unsigned int i;
 
     ASSERT(pirq != INVALID_PIRQ);
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(pdev->domain);
+    ASSERT(rw_is_locked(&pdev->domain->pci_lock));
 
     for ( i = 0; i < nr && bound; i++ )
     {
@@ -1004,7 +1005,8 @@ int vpci_msix_arch_enable_entry(struct vpci_msix_entry 
*entry,
     int rc;
 
     ASSERT(entry->arch.pirq == INVALID_PIRQ);
-    ASSERT_PDEV_LIST_IS_READ_LOCKED(pdev->domain);
+    ASSERT(rw_is_locked(&pdev->domain->pci_lock));
+
     rc = vpci_msi_enable(pdev, vmsix_entry_nr(pdev->vpci->msix, entry),
                          table_base);
     if ( rc < 0 )
@@ -1044,12 +1046,6 @@ int vpci_msix_arch_print(const struct vpci_msix *msix)
 {
     unsigned int i;
 
-    /*
-     * Assert that pdev_list doesn't change. ASSERT_PDEV_LIST_IS_READ_LOCKED
-     * is not suitable here because it allows either pcidevs_lock() or
-     * pci_lock to be held, but here we rely on pci_lock being held, not
-     * pcidevs_lock() (see the transient lock dropping further down).
-     */
     ASSERT(rw_is_locked(&msix->pdev->domain->pci_lock));
     ASSERT(spin_is_locked(&msix->pdev->vpci->lock));
 
--
generated by git-patchbot for /home/xen/git/xen.git#staging



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.