Re: [Xen-devel] [RFC] [PATCH 3/3] XEN ARM PCI support

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

On 20/02/15 5:23 pm, Jan Beulich wrote:
> > > > On 20.02.15 at 12:47, <mjaggi@xxxxxxxxxxxxxxxxxx> wrote:
> > @@ -287,6 +289,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg,
> > u8 bus, u8 devfn)
> >        if ( pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn),
> > PCI_FUNC(devfn),
> >                                 PCI_CAP_ID_MSIX) )
> >        {
> > +#ifndef CONFIG_ARM
> >            struct arch_msix *msix = xzalloc(struct arch_msix);
> >    
> >            if ( !msix )
> > @@ -296,6 +299,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg,
> > u8 bus, u8 devfn)
> >            }
> >            spin_lock_init(&msix->table_lock);
> >            pdev->msix = msix;
> > +#endif
> >        }
> >    
> >        list_add(&pdev->alldevs_list, &pseg->alldevs_list);
> > @@ -396,7 +400,9 @@ static void free_pdev(struct pci_seg *pseg, struct
> > pci_dev *pdev)
> >        }
> >    
> >        list_del(&pdev->alldevs_list);
> > +#ifndef CONFIG_ARM
> >        xfree(pdev->msix);
> > +#endif
> >        xfree(pdev);
> >    }
> >    
> > @@ -424,7 +430,7 @@ int __init pci_hide_device(int bus, int devfn)
> >    
> >        return rc;
> >    }
> > -
> > +#ifndef CONFIG_ARM
> >    int __init pci_ro_device(int seg, int bus, int devfn)
> >    {
> >        struct pci_seg *pseg = alloc_pseg(seg);
> > @@ -452,7 +458,7 @@ int __init pci_ro_device(int seg, int bus, int devfn)
> >    
> >        return 0;
> >    }
> > -
> > +#endif
> >    struct pci_dev *pci_get_pdev(int seg, int bus, int devfn)
> >    {
> >        struct pci_seg *pseg = get_pseg(seg);
> > @@ -740,7 +746,9 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
> >                ret = iommu_remove_device(pdev);
> >                if ( pdev->domain )
> >                    list_del(&pdev->domain_list);
> > +#ifndef CONFIG_ARM
> >                pci_cleanup_msi(pdev);
> > +#endif
> >                free_pdev(pseg, pdev);
> >                printk(XENLOG_DEBUG "PCI remove device %04x:%02x:%02x.%u\n",
> >                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
> > @@ -750,7 +758,7 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
> >        spin_unlock(&pcidevs_lock);
> >        return ret;
> >    }
> > -
> > +#ifndef CONFIG_ARM
> >    static int pci_clean_dpci_irq(struct domain *d,
> >                                  struct hvm_pirq_dpci *pirq_dpci, void *arg)
> >    {
> > @@ -798,11 +806,12 @@ static int pci_clean_dpci_irqs(struct domain *d)
> >        spin_unlock(&d->event_lock);
> >        return 0;
> >    }
> > -
> > +#endif
> >    int pci_release_devices(struct domain *d)
> >    {
> >        struct pci_dev *pdev;
> >        u8 bus, devfn;
> > +#ifndef CONFIG_ARM
> >        int ret;
> >    
> >        spin_lock(&pcidevs_lock);
> > @@ -812,6 +821,8 @@ int pci_release_devices(struct domain *d)
> >            spin_unlock(&pcidevs_lock);
> >            return ret;
> >        }
> > +#endif
> > +
> >        while ( (pdev = pci_get_pdev_by_domain(d, -1, -1, -1)) )
> >        {
> >            bus = pdev->bus;
> > @@ -1184,6 +1195,7 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev
> > *pdev)
> >    
> >    static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
> >    {
> > +#ifndef CONFIG_ARM
> >        struct pci_dev *pdev;
> >        struct msi_desc *msi;
> >    
> > @@ -1199,7 +1211,7 @@ static int _dump_pci_devices(struct pci_seg *pseg, void
> > *arg)
> >                   printk("%d ", msi->irq);
> >            printk(">\n");
> >        }
> > -
> > +#endif
> >        return 0;
> >    }
> >    
> > @@ -1341,7 +1353,7 @@ static int assign_device(struct domain *d, u16 seg, u8
> > bus, u8 devfn)
> >    
> >        if ( !iommu_enabled || !hd->platform_ops )
> >            return 0;
> > -
> > +#ifndef CONFIG_ARM
> >        /* Prevent device assign if mem paging or mem sharing have been
> >         * enabled for this domain */
> >        if ( unlikely(!need_iommu(d) &&
> > @@ -1349,7 +1361,7 @@ static int assign_device(struct domain *d, u16 seg, u8
> > bus, u8 devfn)
> >                 d->mem_event->paging.ring_page ||
> >                 p2m_get_hostp2m(d)->global_logdirty)) )
> >            return -EXDEV;
> > -
> > +#endif
> >        if ( !spin_trylock(&pcidevs_lock) )
> >            return -ERESTART;
> >    
> All of these #ifdef-s call for better abstraction. And in no case is it
> acceptable to replace blank lines with #ifdef-s like you do.
Good observation. I will take care for the blank lines.
A lot of code existed in generic files specific to x86, so arm specific 
defines.
> Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel