From mboxrd@z Thu Jan  1 00:00:00 1970
From: Manish Jaggi <mjaggi@caviumnetworks.com>
Subject: Re: [RFC] [PATCH 3/3] XEN ARM PCI support
Date: Fri, 20 Feb 2015 17:35:01 +0530
Message-ID: <54E722ED.50807@caviumnetworks.com>
References: <54E71EEF.30506@caviumnetworks.com>
	<54E72E620200007800061F4A@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <54E72E620200007800061F4A@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org


On 20/02/15 5:23 pm, Jan Beulich wrote:
>>>> On 20.02.15 at 12:47, <mjaggi@caviumnetworks.com> 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@lists.xen.org
> http://lists.xen.org/xen-devel