Isaku Yamahata
2008-Dec-12 02:36 UTC
[Xen-devel] [PATCH] remove ASSERT(spin_is_locked(&pcidevs_lock));
remove ASSERT(spin_is_locked(&pcidevs_lock));
spin_is_locked() is for spinlock, but pcidevs_lock is rwlock.
so the statement is bogus. remove them.
In fact they caused compilation error for ia64. This patch fixes it.
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -850,7 +850,6 @@ int map_domain_pirq(
struct msi_desc *msi_desc;
struct pci_dev *pdev = NULL;
- ASSERT(spin_is_locked(&pcidevs_lock));
ASSERT(spin_is_locked(&d->event_lock));
if ( !IS_PRIV(current->domain) )
@@ -930,7 +929,6 @@ int unmap_domain_pirq(struct domain *d,
if ( !IS_PRIV(current->domain) )
return -EINVAL;
- ASSERT(spin_is_locked(&pcidevs_lock));
ASSERT(spin_is_locked(&d->event_lock));
vector = d->arch.pirq_vector[pirq];
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -440,7 +440,6 @@ static int msi_capability_init(struct pc
u8 slot = PCI_SLOT(dev->devfn);
u8 func = PCI_FUNC(dev->devfn);
- ASSERT(spin_is_locked(&pcidevs_lock));
pos = pci_find_cap_offset(bus, slot, func, PCI_CAP_ID_MSI);
control = pci_conf_read16(bus, slot, func, msi_control_reg(pos));
/* MSI Entry Initialization */
@@ -509,7 +508,6 @@ static int msix_capability_init(struct p
u8 slot = PCI_SLOT(dev->devfn);
u8 func = PCI_FUNC(dev->devfn);
- ASSERT(spin_is_locked(&pcidevs_lock));
ASSERT(desc);
pos = pci_find_cap_offset(bus, slot, func, PCI_CAP_ID_MSIX);
@@ -574,7 +572,6 @@ static int __pci_enable_msi(struct msi_i
int status;
struct pci_dev *pdev;
- ASSERT(spin_is_locked(&pcidevs_lock));
pdev = pci_get_pdev(msi->bus, msi->devfn);
if ( !pdev )
return -ENODEV;
@@ -634,7 +631,6 @@ static int __pci_enable_msix(struct msi_
u8 slot = PCI_SLOT(msi->devfn);
u8 func = PCI_FUNC(msi->devfn);
- ASSERT(spin_is_locked(&pcidevs_lock));
pdev = pci_get_pdev(msi->bus, msi->devfn);
if ( !pdev )
return -ENODEV;
@@ -688,7 +684,6 @@ static void __pci_disable_msix(struct ms
*/
int pci_enable_msi(struct msi_info *msi, struct msi_desc **desc)
{
- ASSERT(spin_is_locked(&pcidevs_lock));
return msi->table_base ? __pci_enable_msix(msi, desc) :
__pci_enable_msi(msi, desc);
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -86,8 +86,6 @@ int iommu_add_device(struct pci_dev *pde
if ( !pdev->domain )
return -EINVAL;
-
- ASSERT(spin_is_locked(&pcidevs_lock));
hd = domain_hvm_iommu(pdev->domain);
if ( !iommu_enabled || !hd->platform_ops )
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -62,7 +62,6 @@ struct pci_dev *pci_get_pdev(int bus, in
{
struct pci_dev *pdev = NULL;
- ASSERT(spin_is_locked(&pcidevs_lock));
list_for_each_entry ( pdev, &alldevs_list, alldevs_list )
if ( (pdev->bus == bus || bus == -1) &&
@@ -78,7 +77,6 @@ struct pci_dev *pci_get_pdev_by_domain(s
{
struct pci_dev *pdev = NULL;
- ASSERT(spin_is_locked(&pcidevs_lock));
list_for_each_entry ( pdev, &alldevs_list, alldevs_list )
if ( (pdev->bus == bus || bus == -1) &&
diff --git a/xen/drivers/passthrough/vtd/iommu.c
b/xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1037,7 +1037,6 @@ static int domain_context_mapping_one(
struct pci_dev *pdev = NULL;
int agaw;
- ASSERT(spin_is_locked(&pcidevs_lock));
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
context_entries = (struct context_entry *)map_vtd_domain_page(maddr);
@@ -1214,7 +1213,6 @@ static int domain_context_mapping(struct
if ( !drhd )
return -ENODEV;
- ASSERT(spin_is_locked(&pcidevs_lock));
type = pdev_type(bus, devfn);
switch ( type )
@@ -1298,7 +1296,6 @@ static int domain_context_unmap_one(
struct context_entry *context, *context_entries;
u64 maddr;
- ASSERT(spin_is_locked(&pcidevs_lock));
spin_lock(&iommu->lock);
maddr = bus_to_context_maddr(iommu, bus);
@@ -1388,7 +1385,6 @@ static int reassign_device_ownership(
struct iommu *pdev_iommu;
int ret, found = 0;
- ASSERT(spin_is_locked(&pcidevs_lock));
pdev = pci_get_pdev_by_domain(source, bus, devfn);
if (!pdev)
@@ -1428,7 +1424,6 @@ void iommu_domain_teardown(struct domain
if ( list_empty(&acpi_drhd_units) )
return;
- ASSERT(spin_is_locked(&pcidevs_lock));
spin_lock(&hd->mapping_lock);
iommu_free_pagetable(hd->pgd_maddr, agaw_to_level(hd->agaw));
hd->pgd_maddr = 0;
@@ -1518,7 +1513,6 @@ static int iommu_prepare_rmrr_dev(struct
u64 base, end;
unsigned long base_pfn, end_pfn;
- ASSERT(spin_is_locked(&pcidevs_lock));
ASSERT(rmrr->base_address < rmrr->end_address);
base = rmrr->base_address & PAGE_MASK_4K;
@@ -1543,7 +1537,6 @@ static int intel_iommu_add_device(struct
u16 bdf;
int ret, i;
- ASSERT(spin_is_locked(&pcidevs_lock));
if ( !pdev->domain )
return -EINVAL;
@@ -1782,7 +1775,6 @@ int intel_iommu_assign_device(struct dom
if ( list_empty(&acpi_drhd_units) )
return -ENODEV;
- ASSERT(spin_is_locked(&pcidevs_lock));
pdev = pci_get_pdev(bus, devfn);
if (!pdev)
return -ENODEV;
--
yamahata
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Jiang, Yunhong
2008-Dec-12 04:12 UTC
RE: [Xen-devel] [PATCH] remove ASSERT(spin_is_locked(&pcidevs_lock));
Per discussion with Espen, the rw_lock could be changed to spin_lock. But I didn''t get more information from him, so I didn''t try it. Since it cause compilation error on IA64, I cook a patch for it and verified on x86 side. The attached patch change the pcidevs_lock from rw_lock to spin_lock, also two defect fixed: a) deassign_device will deadlock when try to get the pcidevs_lock if called by pci_release_devices, remove the lock to the caller. b) The iommu_domain_teardown should not ASSERT for the pcidevs_lock because it just update the domain''s vt-d mapping. Signed-off-by: Yunhong Jiang <yunhong.jiang@intel.com> Thanks Yunhong Jiang xen-devel-bounces@lists.xensource.com <> wrote:> remove ASSERT(spin_is_locked(&pcidevs_lock)); > > spin_is_locked() is for spinlock, but pcidevs_lock is rwlock. > so the statement is bogus. remove them. > In fact they caused compilation error for ia64. This patch fixes it. > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > > diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c --- > a/xen/arch/x86/irq.c +++ b/xen/arch/x86/irq.c > @@ -850,7 +850,6 @@ int map_domain_pirq( > struct msi_desc *msi_desc; > struct pci_dev *pdev = NULL; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > ASSERT(spin_is_locked(&d->event_lock)); > > if ( !IS_PRIV(current->domain) ) > @@ -930,7 +929,6 @@ int unmap_domain_pirq(struct domain *d, > if ( !IS_PRIV(current->domain) ) > return -EINVAL; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > ASSERT(spin_is_locked(&d->event_lock)); > > vector = d->arch.pirq_vector[pirq]; > diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c --- > a/xen/arch/x86/msi.c +++ b/xen/arch/x86/msi.c > @@ -440,7 +440,6 @@ static int msi_capability_init(struct pc > u8 slot = PCI_SLOT(dev->devfn); > u8 func = PCI_FUNC(dev->devfn); > > - ASSERT(spin_is_locked(&pcidevs_lock)); > pos = pci_find_cap_offset(bus, slot, func, PCI_CAP_ID_MSI); > control = pci_conf_read16(bus, slot, func, msi_control_reg(pos)); > /* MSI Entry Initialization */ > @@ -509,7 +508,6 @@ static int msix_capability_init(struct p > u8 slot = PCI_SLOT(dev->devfn); > u8 func = PCI_FUNC(dev->devfn); > > - ASSERT(spin_is_locked(&pcidevs_lock)); > ASSERT(desc); > > pos = pci_find_cap_offset(bus, slot, func, PCI_CAP_ID_MSIX); > @@ -574,7 +572,6 @@ static int __pci_enable_msi(struct msi_i int status; > struct pci_dev *pdev; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > pdev = pci_get_pdev(msi->bus, msi->devfn); > if ( !pdev ) > return -ENODEV; > @@ -634,7 +631,6 @@ static int __pci_enable_msix(struct msi_ > u8 slot = PCI_SLOT(msi->devfn); > u8 func = PCI_FUNC(msi->devfn); > > - ASSERT(spin_is_locked(&pcidevs_lock)); > pdev = pci_get_pdev(msi->bus, msi->devfn); > if ( !pdev ) > return -ENODEV; > @@ -688,7 +684,6 @@ static void __pci_disable_msix(struct ms */ > int pci_enable_msi(struct msi_info *msi, struct msi_desc **desc) { > - ASSERT(spin_is_locked(&pcidevs_lock)); > > return msi->table_base ? __pci_enable_msix(msi, desc) : > __pci_enable_msi(msi, desc); > diff --git a/xen/drivers/passthrough/iommu.c > b/xen/drivers/passthrough/iommu.c > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -86,8 +86,6 @@ int iommu_add_device(struct pci_dev *pde > > if ( !pdev->domain ) > return -EINVAL; > - > - ASSERT(spin_is_locked(&pcidevs_lock)); > > hd = domain_hvm_iommu(pdev->domain); > if ( !iommu_enabled || !hd->platform_ops ) > diff --git a/xen/drivers/passthrough/pci.c > b/xen/drivers/passthrough/pci.c > --- a/xen/drivers/passthrough/pci.c > +++ b/xen/drivers/passthrough/pci.c > @@ -62,7 +62,6 @@ struct pci_dev *pci_get_pdev(int bus, in { > struct pci_dev *pdev = NULL; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > > list_for_each_entry ( pdev, &alldevs_list, alldevs_list ) > if ( (pdev->bus == bus || bus == -1) && > @@ -78,7 +77,6 @@ struct pci_dev *pci_get_pdev_by_domain(s { > struct pci_dev *pdev = NULL; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > > list_for_each_entry ( pdev, &alldevs_list, alldevs_list ) > if ( (pdev->bus == bus || bus == -1) && > diff --git a/xen/drivers/passthrough/vtd/iommu.c > b/xen/drivers/passthrough/vtd/iommu.c > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -1037,7 +1037,6 @@ static int domain_context_mapping_one( > struct pci_dev *pdev = NULL; > int agaw; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > spin_lock(&iommu->lock); > maddr = bus_to_context_maddr(iommu, bus); > context_entries = (struct context_entry > *)map_vtd_domain_page(maddr); > @@ -1214,7 +1213,6 @@ static int domain_context_mapping(struct if ( > !drhd ) return -ENODEV; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > > type = pdev_type(bus, devfn); > switch ( type ) > @@ -1298,7 +1296,6 @@ static int domain_context_unmap_one( > struct context_entry *context, *context_entries; u64 maddr; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > spin_lock(&iommu->lock); > > maddr = bus_to_context_maddr(iommu, bus); > @@ -1388,7 +1385,6 @@ static int reassign_device_ownership( struct > iommu *pdev_iommu; int ret, found = 0; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > pdev = pci_get_pdev_by_domain(source, bus, devfn); > > if (!pdev) > @@ -1428,7 +1424,6 @@ void iommu_domain_teardown(struct domain > if ( list_empty(&acpi_drhd_units) ) > return; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > spin_lock(&hd->mapping_lock); > iommu_free_pagetable(hd->pgd_maddr, agaw_to_level(hd->agaw)); > hd->pgd_maddr = 0; @@ -1518,7 +1513,6 @@ static int > iommu_prepare_rmrr_dev(struct u64 base, end; unsigned long > base_pfn, end_pfn; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > ASSERT(rmrr->base_address < rmrr->end_address); > > base = rmrr->base_address & PAGE_MASK_4K; > @@ -1543,7 +1537,6 @@ static int intel_iommu_add_device(struct u16 bdf; > int ret, i; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > > if ( !pdev->domain ) > return -EINVAL; > @@ -1782,7 +1775,6 @@ int intel_iommu_assign_device(struct dom > if ( list_empty(&acpi_drhd_units) ) > return -ENODEV; > > - ASSERT(spin_is_locked(&pcidevs_lock)); > pdev = pci_get_pdev(bus, devfn); > if (!pdev) > return -ENODEV; > > > -- > yamahata > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel