diff mbox series

[v3,2/3] xen/pci: Move x86 specific code to x86 directory.

Message ID a84005e5aa6733043e043b015cde4983719c8535.1605527997.git.rahul.singh@arm.com (mailing list archive)
State Superseded
Headers show
Series xen/arm: Make PCI passthrough code non-x86 specific | expand

Commit Message

Rahul Singh Nov. 16, 2020, 12:25 p.m. UTC
passthrough/pci.c file is common for all architecture, but there is x86
specific code in this file.

Move x86 specific code to the drivers/passthrough/io.c file to avoid
compilation error for other architecture.

As drivers/passthrough/io.c is compiled only for x86 move it to
x86 directory and rename it to hvm.c.

No functional change.

Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---

Changes in v3:
- fixed typo
- As per suggestion move the code to the file io.c and move that file to
  x86 directory and rename it hvm.c

---
 xen/drivers/passthrough/Makefile            |  3 -
 xen/drivers/passthrough/pci.c               | 78 +--------------------
 xen/drivers/passthrough/x86/Makefile        |  1 +
 xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +++++++++++++++++
 xen/drivers/passthrough/x86/iommu.c         |  7 ++
 xen/include/xen/pci.h                       |  2 +
 6 files changed, 77 insertions(+), 80 deletions(-)
 rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)

Comments

Stefano Stabellini Nov. 17, 2020, 1:20 a.m. UTC | #1
On Mon, 16 Nov 2020, Rahul Singh wrote:
> passthrough/pci.c file is common for all architecture, but there is x86
> specific code in this file.
> 
> Move x86 specific code to the drivers/passthrough/io.c file to avoid
> compilation error for other architecture.
> 
> As drivers/passthrough/io.c is compiled only for x86 move it to
> x86 directory and rename it to hvm.c.
> 
> No functional change.
> 
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>

This patch breaks the x86 build if you disable CONFIG_HVM:

prelink-efi.o: In function `pci_release_devices':
/local/repos/xen-upstream/xen/drivers/passthrough/pci.c:900: undefined reference to `arch_pci_clean_pirqs'
Makefile:209: recipe for target '/local/repos/xen-upstream/xen/xen.efi' failed



> ---
> 
> Changes in v3:
> - fixed typo
> - As per suggestion move the code to the file io.c and move that file to
>   x86 directory and rename it hvm.c
> 
> ---
>  xen/drivers/passthrough/Makefile            |  3 -
>  xen/drivers/passthrough/pci.c               | 78 +--------------------
>  xen/drivers/passthrough/x86/Makefile        |  1 +
>  xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +++++++++++++++++
>  xen/drivers/passthrough/x86/iommu.c         |  7 ++
>  xen/include/xen/pci.h                       |  2 +
>  6 files changed, 77 insertions(+), 80 deletions(-)
>  rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)
> 
> diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
> index e973e16c74..cc646612c7 100644
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -6,6 +6,3 @@ obj-$(CONFIG_ARM) += arm/
>  obj-y += iommu.o
>  obj-$(CONFIG_HAS_PCI) += pci.o
>  obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
> -
> -x86-$(CONFIG_HVM) := io.o
> -obj-$(CONFIG_X86) += $(x86-y)
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index 51e584127e..e8a28df126 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -14,9 +14,6 @@
>   * this program; If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> -#include <xen/sched.h>
> -#include <xen/pci.h>
> -#include <xen/pci_regs.h>
>  #include <xen/pci_ids.h>
>  #include <xen/list.h>
>  #include <xen/prefetch.h>
> @@ -24,7 +21,6 @@
>  #include <xen/irq.h>
>  #include <xen/param.h>
>  #include <xen/vm_event.h>
> -#include <asm/hvm/irq.h>
>  #include <xen/delay.h>
>  #include <xen/keyhandler.h>
>  #include <xen/event.h>
> @@ -842,71 +838,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>      return ret;
>  }
>  
> -static int pci_clean_dpci_irq(struct domain *d,
> -                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
> -{
> -    struct dev_intx_gsi_link *digl, *tmp;
> -
> -    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
> -
> -    if ( pt_irq_need_timer(pirq_dpci->flags) )
> -        kill_timer(&pirq_dpci->timer);
> -
> -    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
> -    {
> -        list_del(&digl->list);
> -        xfree(digl);
> -    }
> -
> -    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
> -
> -    if ( !pt_pirq_softirq_active(pirq_dpci) )
> -        return 0;
> -
> -    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
> -
> -    return -ERESTART;
> -}
> -
> -static int pci_clean_dpci_irqs(struct domain *d)
> -{
> -    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
> -
> -    if ( !is_iommu_enabled(d) )
> -        return 0;
> -
> -    if ( !is_hvm_domain(d) )
> -        return 0;
> -
> -    spin_lock(&d->event_lock);
> -    hvm_irq_dpci = domain_get_irq_dpci(d);
> -    if ( hvm_irq_dpci != NULL )
> -    {
> -        int ret = 0;
> -
> -        if ( hvm_irq_dpci->pending_pirq_dpci )
> -        {
> -            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
> -                 ret = -ERESTART;
> -            else
> -                 hvm_irq_dpci->pending_pirq_dpci = NULL;
> -        }
> -
> -        if ( !ret )
> -            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
> -        if ( ret )
> -        {
> -            spin_unlock(&d->event_lock);
> -            return ret;
> -        }
> -
> -        hvm_domain_irq(d)->dpci = NULL;
> -        free_hvm_irq_dpci(hvm_irq_dpci);
> -    }
> -    spin_unlock(&d->event_lock);
> -    return 0;
> -}
> -
>  /* Caller should hold the pcidevs_lock */
>  static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
>                             uint8_t devfn)
> @@ -966,7 +897,7 @@ int pci_release_devices(struct domain *d)
>      int ret;
>  
>      pcidevs_lock();
> -    ret = pci_clean_dpci_irqs(d);
> +    ret = arch_pci_clean_pirqs(d);
>      if ( ret )
>      {
>          pcidevs_unlock();
> @@ -1370,13 +1301,6 @@ static int __init setup_dump_pcidevs(void)
>  }
>  __initcall(setup_dump_pcidevs);
>  
> -int iommu_update_ire_from_msi(
> -    struct msi_desc *msi_desc, struct msi_msg *msg)
> -{
> -    return iommu_intremap
> -           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
> -}
> -
>  static int iommu_add_device(struct pci_dev *pdev)
>  {
>      const struct domain_iommu *hd;
> diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
> index a70cf9460d..69284a5d19 100644
> --- a/xen/drivers/passthrough/x86/Makefile
> +++ b/xen/drivers/passthrough/x86/Makefile
> @@ -1,2 +1,3 @@
>  obj-y += ats.o
>  obj-y += iommu.o
> +obj-$(CONFIG_HVM) += hvm.o
> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/hvm.c
> similarity index 95%
> rename from xen/drivers/passthrough/io.c
> rename to xen/drivers/passthrough/x86/hvm.c
> index 6b1305a3e5..41cfa2e200 100644
> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/x86/hvm.c
> @@ -1036,6 +1036,72 @@ unlock:
>      spin_unlock(&d->event_lock);
>  }
>  
> +static int pci_clean_dpci_irq(struct domain *d,
> +                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
> +{
> +    struct dev_intx_gsi_link *digl, *tmp;
> +
> +    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
> +
> +    if ( pt_irq_need_timer(pirq_dpci->flags) )
> +        kill_timer(&pirq_dpci->timer);
> +
> +    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
> +    {
> +        list_del(&digl->list);
> +        xfree(digl);
> +    }
> +
> +    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
> +
> +    if ( !pt_pirq_softirq_active(pirq_dpci) )
> +        return 0;
> +
> +    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
> +
> +    return -ERESTART;
> +}
> +
> +int arch_pci_clean_pirqs(struct domain *d)
> +{
> +    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
> +
> +    if ( !is_iommu_enabled(d) )
> +        return 0;
> +
> +    if ( !is_hvm_domain(d) )
> +        return 0;
> +
> +    spin_lock(&d->event_lock);
> +    hvm_irq_dpci = domain_get_irq_dpci(d);
> +    if ( hvm_irq_dpci != NULL )
> +    {
> +        int ret = 0;
> +
> +        if ( hvm_irq_dpci->pending_pirq_dpci )
> +        {
> +            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
> +                 ret = -ERESTART;
> +            else
> +                 hvm_irq_dpci->pending_pirq_dpci = NULL;
> +        }
> +
> +        if ( !ret )
> +            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
> +        if ( ret )
> +        {
> +            spin_unlock(&d->event_lock);
> +            return ret;
> +        }
> +
> +        hvm_domain_irq(d)->dpci = NULL;
> +        free_hvm_irq_dpci(hvm_irq_dpci);
> +    }
> +    spin_unlock(&d->event_lock);
> +
> +    return 0;
> +}
> +
>  /*
>   * Note: 'pt_pirq_softirq_reset' can clear the STATE_SCHED before we get to
>   * doing it. If that is the case we let 'pt_pirq_softirq_reset' do ref-counting.
> diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
> index f17b1820f4..875e67b53b 100644
> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -308,6 +308,13 @@ struct page_info *iommu_alloc_pgtable(struct domain *d)
>      return pg;
>  }
>  
> +int iommu_update_ire_from_msi(
> +    struct msi_desc *msi_desc, struct msi_msg *msg)
> +{
> +    return iommu_intremap
> +           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
> index 20a54a5bb4..78d83afe64 100644
> --- a/xen/include/xen/pci.h
> +++ b/xen/include/xen/pci.h
> @@ -208,4 +208,6 @@ int msixtbl_pt_register(struct domain *, struct pirq *, uint64_t gtable);
>  void msixtbl_pt_unregister(struct domain *, struct pirq *);
>  void msixtbl_pt_cleanup(struct domain *d);
>  
> +int arch_pci_clean_pirqs(struct domain *d);
> +
>  #endif /* __XEN_PCI_H__ */
> -- 
> 2.17.1
>
Rahul Singh Nov. 17, 2020, 9:49 a.m. UTC | #2
Hello Stefano,

> On 17 Nov 2020, at 1:20 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 16 Nov 2020, Rahul Singh wrote:
>> passthrough/pci.c file is common for all architecture, but there is x86
>> specific code in this file.
>> 
>> Move x86 specific code to the drivers/passthrough/io.c file to avoid
>> compilation error for other architecture.
>> 
>> As drivers/passthrough/io.c is compiled only for x86 move it to
>> x86 directory and rename it to hvm.c.
>> 
>> No functional change.
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> 
> This patch breaks the x86 build if you disable CONFIG_HVM:
> 
> prelink-efi.o: In function `pci_release_devices':
> /local/repos/xen-upstream/xen/drivers/passthrough/pci.c:900: undefined reference to `arch_pci_clean_pirqs'
> Makefile:209: recipe for target '/local/repos/xen-upstream/xen/xen.efi' failed
> 
> 

Thanks for reviewing the code.
I will fix the build and will send the v3 patch.

Regards,
Rahul

> 
>> ---
>> 
>> Changes in v3:
>> - fixed typo
>> - As per suggestion move the code to the file io.c and move that file to
>>  x86 directory and rename it hvm.c
>> 
>> ---
>> xen/drivers/passthrough/Makefile            |  3 -
>> xen/drivers/passthrough/pci.c               | 78 +--------------------
>> xen/drivers/passthrough/x86/Makefile        |  1 +
>> xen/drivers/passthrough/{io.c => x86/hvm.c} | 66 +++++++++++++++++
>> xen/drivers/passthrough/x86/iommu.c         |  7 ++
>> xen/include/xen/pci.h                       |  2 +
>> 6 files changed, 77 insertions(+), 80 deletions(-)
>> rename xen/drivers/passthrough/{io.c => x86/hvm.c} (95%)
>> 
>> diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
>> index e973e16c74..cc646612c7 100644
>> --- a/xen/drivers/passthrough/Makefile
>> +++ b/xen/drivers/passthrough/Makefile
>> @@ -6,6 +6,3 @@ obj-$(CONFIG_ARM) += arm/
>> obj-y += iommu.o
>> obj-$(CONFIG_HAS_PCI) += pci.o
>> obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
>> -
>> -x86-$(CONFIG_HVM) := io.o
>> -obj-$(CONFIG_X86) += $(x86-y)
>> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
>> index 51e584127e..e8a28df126 100644
>> --- a/xen/drivers/passthrough/pci.c
>> +++ b/xen/drivers/passthrough/pci.c
>> @@ -14,9 +14,6 @@
>>  * this program; If not, see <http://www.gnu.org/licenses/>.
>>  */
>> 
>> -#include <xen/sched.h>
>> -#include <xen/pci.h>
>> -#include <xen/pci_regs.h>
>> #include <xen/pci_ids.h>
>> #include <xen/list.h>
>> #include <xen/prefetch.h>
>> @@ -24,7 +21,6 @@
>> #include <xen/irq.h>
>> #include <xen/param.h>
>> #include <xen/vm_event.h>
>> -#include <asm/hvm/irq.h>
>> #include <xen/delay.h>
>> #include <xen/keyhandler.h>
>> #include <xen/event.h>
>> @@ -842,71 +838,6 @@ int pci_remove_device(u16 seg, u8 bus, u8 devfn)
>>     return ret;
>> }
>> 
>> -static int pci_clean_dpci_irq(struct domain *d,
>> -                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
>> -{
>> -    struct dev_intx_gsi_link *digl, *tmp;
>> -
>> -    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
>> -
>> -    if ( pt_irq_need_timer(pirq_dpci->flags) )
>> -        kill_timer(&pirq_dpci->timer);
>> -
>> -    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
>> -    {
>> -        list_del(&digl->list);
>> -        xfree(digl);
>> -    }
>> -
>> -    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
>> -
>> -    if ( !pt_pirq_softirq_active(pirq_dpci) )
>> -        return 0;
>> -
>> -    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
>> -
>> -    return -ERESTART;
>> -}
>> -
>> -static int pci_clean_dpci_irqs(struct domain *d)
>> -{
>> -    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
>> -
>> -    if ( !is_iommu_enabled(d) )
>> -        return 0;
>> -
>> -    if ( !is_hvm_domain(d) )
>> -        return 0;
>> -
>> -    spin_lock(&d->event_lock);
>> -    hvm_irq_dpci = domain_get_irq_dpci(d);
>> -    if ( hvm_irq_dpci != NULL )
>> -    {
>> -        int ret = 0;
>> -
>> -        if ( hvm_irq_dpci->pending_pirq_dpci )
>> -        {
>> -            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
>> -                 ret = -ERESTART;
>> -            else
>> -                 hvm_irq_dpci->pending_pirq_dpci = NULL;
>> -        }
>> -
>> -        if ( !ret )
>> -            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
>> -        if ( ret )
>> -        {
>> -            spin_unlock(&d->event_lock);
>> -            return ret;
>> -        }
>> -
>> -        hvm_domain_irq(d)->dpci = NULL;
>> -        free_hvm_irq_dpci(hvm_irq_dpci);
>> -    }
>> -    spin_unlock(&d->event_lock);
>> -    return 0;
>> -}
>> -
>> /* Caller should hold the pcidevs_lock */
>> static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
>>                            uint8_t devfn)
>> @@ -966,7 +897,7 @@ int pci_release_devices(struct domain *d)
>>     int ret;
>> 
>>     pcidevs_lock();
>> -    ret = pci_clean_dpci_irqs(d);
>> +    ret = arch_pci_clean_pirqs(d);
>>     if ( ret )
>>     {
>>         pcidevs_unlock();
>> @@ -1370,13 +1301,6 @@ static int __init setup_dump_pcidevs(void)
>> }
>> __initcall(setup_dump_pcidevs);
>> 
>> -int iommu_update_ire_from_msi(
>> -    struct msi_desc *msi_desc, struct msi_msg *msg)
>> -{
>> -    return iommu_intremap
>> -           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
>> -}
>> -
>> static int iommu_add_device(struct pci_dev *pdev)
>> {
>>     const struct domain_iommu *hd;
>> diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
>> index a70cf9460d..69284a5d19 100644
>> --- a/xen/drivers/passthrough/x86/Makefile
>> +++ b/xen/drivers/passthrough/x86/Makefile
>> @@ -1,2 +1,3 @@
>> obj-y += ats.o
>> obj-y += iommu.o
>> +obj-$(CONFIG_HVM) += hvm.o
>> diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/hvm.c
>> similarity index 95%
>> rename from xen/drivers/passthrough/io.c
>> rename to xen/drivers/passthrough/x86/hvm.c
>> index 6b1305a3e5..41cfa2e200 100644
>> --- a/xen/drivers/passthrough/io.c
>> +++ b/xen/drivers/passthrough/x86/hvm.c
>> @@ -1036,6 +1036,72 @@ unlock:
>>     spin_unlock(&d->event_lock);
>> }
>> 
>> +static int pci_clean_dpci_irq(struct domain *d,
>> +                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
>> +{
>> +    struct dev_intx_gsi_link *digl, *tmp;
>> +
>> +    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
>> +
>> +    if ( pt_irq_need_timer(pirq_dpci->flags) )
>> +        kill_timer(&pirq_dpci->timer);
>> +
>> +    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
>> +    {
>> +        list_del(&digl->list);
>> +        xfree(digl);
>> +    }
>> +
>> +    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
>> +
>> +    if ( !pt_pirq_softirq_active(pirq_dpci) )
>> +        return 0;
>> +
>> +    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
>> +
>> +    return -ERESTART;
>> +}
>> +
>> +int arch_pci_clean_pirqs(struct domain *d)
>> +{
>> +    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
>> +
>> +    if ( !is_iommu_enabled(d) )
>> +        return 0;
>> +
>> +    if ( !is_hvm_domain(d) )
>> +        return 0;
>> +
>> +    spin_lock(&d->event_lock);
>> +    hvm_irq_dpci = domain_get_irq_dpci(d);
>> +    if ( hvm_irq_dpci != NULL )
>> +    {
>> +        int ret = 0;
>> +
>> +        if ( hvm_irq_dpci->pending_pirq_dpci )
>> +        {
>> +            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
>> +                 ret = -ERESTART;
>> +            else
>> +                 hvm_irq_dpci->pending_pirq_dpci = NULL;
>> +        }
>> +
>> +        if ( !ret )
>> +            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
>> +        if ( ret )
>> +        {
>> +            spin_unlock(&d->event_lock);
>> +            return ret;
>> +        }
>> +
>> +        hvm_domain_irq(d)->dpci = NULL;
>> +        free_hvm_irq_dpci(hvm_irq_dpci);
>> +    }
>> +    spin_unlock(&d->event_lock);
>> +
>> +    return 0;
>> +}
>> +
>> /*
>>  * Note: 'pt_pirq_softirq_reset' can clear the STATE_SCHED before we get to
>>  * doing it. If that is the case we let 'pt_pirq_softirq_reset' do ref-counting.
>> diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
>> index f17b1820f4..875e67b53b 100644
>> --- a/xen/drivers/passthrough/x86/iommu.c
>> +++ b/xen/drivers/passthrough/x86/iommu.c
>> @@ -308,6 +308,13 @@ struct page_info *iommu_alloc_pgtable(struct domain *d)
>>     return pg;
>> }
>> 
>> +int iommu_update_ire_from_msi(
>> +    struct msi_desc *msi_desc, struct msi_msg *msg)
>> +{
>> +    return iommu_intremap
>> +           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
>> +}
>> +
>> /*
>>  * Local variables:
>>  * mode: C
>> diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
>> index 20a54a5bb4..78d83afe64 100644
>> --- a/xen/include/xen/pci.h
>> +++ b/xen/include/xen/pci.h
>> @@ -208,4 +208,6 @@ int msixtbl_pt_register(struct domain *, struct pirq *, uint64_t gtable);
>> void msixtbl_pt_unregister(struct domain *, struct pirq *);
>> void msixtbl_pt_cleanup(struct domain *d);
>> 
>> +int arch_pci_clean_pirqs(struct domain *d);
>> +
>> #endif /* __XEN_PCI_H__ */
>> -- 
>> 2.17.1
>>
Jan Beulich Nov. 17, 2020, 11:03 a.m. UTC | #3
On 16.11.2020 13:25, Rahul Singh wrote:
> passthrough/pci.c file is common for all architecture, but there is x86
> specific code in this file.

In how far is ...

> @@ -1370,13 +1301,6 @@ static int __init setup_dump_pcidevs(void)
>  }
>  __initcall(setup_dump_pcidevs);
>  
> -int iommu_update_ire_from_msi(
> -    struct msi_desc *msi_desc, struct msi_msg *msg)
> -{
> -    return iommu_intremap
> -           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
> -}

... this code x86-specific? The hook being called lives in a
#ifdef CONFIG_HAS_PCI section, and MSI is a general PCI sub-
feature. IOW if this is another workaround, it should be
called so (if there's really no other way to address whatever
issue there is), which in turn likely means it wants to be in
a separate patch.

Jan
Rahul Singh Nov. 17, 2020, 4:52 p.m. UTC | #4
Hello Jan,

> On 17 Nov 2020, at 11:03 am, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 16.11.2020 13:25, Rahul Singh wrote:
>> passthrough/pci.c file is common for all architecture, but there is x86
>> specific code in this file.
> 
> In how far is ...
> 
>> @@ -1370,13 +1301,6 @@ static int __init setup_dump_pcidevs(void)
>> }
>> __initcall(setup_dump_pcidevs);
>> 
>> -int iommu_update_ire_from_msi(
>> -    struct msi_desc *msi_desc, struct msi_msg *msg)
>> -{
>> -    return iommu_intremap
>> -           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
>> -}
> 
> ... this code x86-specific? The hook being called lives in a
> #ifdef CONFIG_HAS_PCI section, and MSI is a general PCI sub-
> feature. IOW if this is another workaround, it should be
> called so (if there's really no other way to address whatever
> issue there is), which in turn likely means it wants to be in
> a separate patch.
> 

As of now there is no implementation for ARM to remap the interrupt and interrupt remapping is enabled for x86 only so I thought I will move this code to x86 file as this function is called from the x86 specific code only.

I will remove this code form this patch and will fix this once once will have proper MSI implementation for ARM.

> Jan
diff mbox series

Patch

diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index e973e16c74..cc646612c7 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -6,6 +6,3 @@  obj-$(CONFIG_ARM) += arm/
 obj-y += iommu.o
 obj-$(CONFIG_HAS_PCI) += pci.o
 obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
-
-x86-$(CONFIG_HVM) := io.o
-obj-$(CONFIG_X86) += $(x86-y)
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 51e584127e..e8a28df126 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -14,9 +14,6 @@ 
  * this program; If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <xen/sched.h>
-#include <xen/pci.h>
-#include <xen/pci_regs.h>
 #include <xen/pci_ids.h>
 #include <xen/list.h>
 #include <xen/prefetch.h>
@@ -24,7 +21,6 @@ 
 #include <xen/irq.h>
 #include <xen/param.h>
 #include <xen/vm_event.h>
-#include <asm/hvm/irq.h>
 #include <xen/delay.h>
 #include <xen/keyhandler.h>
 #include <xen/event.h>
@@ -842,71 +838,6 @@  int pci_remove_device(u16 seg, u8 bus, u8 devfn)
     return ret;
 }
 
-static int pci_clean_dpci_irq(struct domain *d,
-                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
-{
-    struct dev_intx_gsi_link *digl, *tmp;
-
-    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
-
-    if ( pt_irq_need_timer(pirq_dpci->flags) )
-        kill_timer(&pirq_dpci->timer);
-
-    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
-    {
-        list_del(&digl->list);
-        xfree(digl);
-    }
-
-    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
-
-    if ( !pt_pirq_softirq_active(pirq_dpci) )
-        return 0;
-
-    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
-
-    return -ERESTART;
-}
-
-static int pci_clean_dpci_irqs(struct domain *d)
-{
-    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
-
-    if ( !is_iommu_enabled(d) )
-        return 0;
-
-    if ( !is_hvm_domain(d) )
-        return 0;
-
-    spin_lock(&d->event_lock);
-    hvm_irq_dpci = domain_get_irq_dpci(d);
-    if ( hvm_irq_dpci != NULL )
-    {
-        int ret = 0;
-
-        if ( hvm_irq_dpci->pending_pirq_dpci )
-        {
-            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
-                 ret = -ERESTART;
-            else
-                 hvm_irq_dpci->pending_pirq_dpci = NULL;
-        }
-
-        if ( !ret )
-            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
-        if ( ret )
-        {
-            spin_unlock(&d->event_lock);
-            return ret;
-        }
-
-        hvm_domain_irq(d)->dpci = NULL;
-        free_hvm_irq_dpci(hvm_irq_dpci);
-    }
-    spin_unlock(&d->event_lock);
-    return 0;
-}
-
 /* Caller should hold the pcidevs_lock */
 static int deassign_device(struct domain *d, uint16_t seg, uint8_t bus,
                            uint8_t devfn)
@@ -966,7 +897,7 @@  int pci_release_devices(struct domain *d)
     int ret;
 
     pcidevs_lock();
-    ret = pci_clean_dpci_irqs(d);
+    ret = arch_pci_clean_pirqs(d);
     if ( ret )
     {
         pcidevs_unlock();
@@ -1370,13 +1301,6 @@  static int __init setup_dump_pcidevs(void)
 }
 __initcall(setup_dump_pcidevs);
 
-int iommu_update_ire_from_msi(
-    struct msi_desc *msi_desc, struct msi_msg *msg)
-{
-    return iommu_intremap
-           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
-}
-
 static int iommu_add_device(struct pci_dev *pdev)
 {
     const struct domain_iommu *hd;
diff --git a/xen/drivers/passthrough/x86/Makefile b/xen/drivers/passthrough/x86/Makefile
index a70cf9460d..69284a5d19 100644
--- a/xen/drivers/passthrough/x86/Makefile
+++ b/xen/drivers/passthrough/x86/Makefile
@@ -1,2 +1,3 @@ 
 obj-y += ats.o
 obj-y += iommu.o
+obj-$(CONFIG_HVM) += hvm.o
diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/x86/hvm.c
similarity index 95%
rename from xen/drivers/passthrough/io.c
rename to xen/drivers/passthrough/x86/hvm.c
index 6b1305a3e5..41cfa2e200 100644
--- a/xen/drivers/passthrough/io.c
+++ b/xen/drivers/passthrough/x86/hvm.c
@@ -1036,6 +1036,72 @@  unlock:
     spin_unlock(&d->event_lock);
 }
 
+static int pci_clean_dpci_irq(struct domain *d,
+                              struct hvm_pirq_dpci *pirq_dpci, void *arg)
+{
+    struct dev_intx_gsi_link *digl, *tmp;
+
+    pirq_guest_unbind(d, dpci_pirq(pirq_dpci));
+
+    if ( pt_irq_need_timer(pirq_dpci->flags) )
+        kill_timer(&pirq_dpci->timer);
+
+    list_for_each_entry_safe ( digl, tmp, &pirq_dpci->digl_list, list )
+    {
+        list_del(&digl->list);
+        xfree(digl);
+    }
+
+    radix_tree_delete(&d->pirq_tree, dpci_pirq(pirq_dpci)->pirq);
+
+    if ( !pt_pirq_softirq_active(pirq_dpci) )
+        return 0;
+
+    domain_get_irq_dpci(d)->pending_pirq_dpci = pirq_dpci;
+
+    return -ERESTART;
+}
+
+int arch_pci_clean_pirqs(struct domain *d)
+{
+    struct hvm_irq_dpci *hvm_irq_dpci = NULL;
+
+    if ( !is_iommu_enabled(d) )
+        return 0;
+
+    if ( !is_hvm_domain(d) )
+        return 0;
+
+    spin_lock(&d->event_lock);
+    hvm_irq_dpci = domain_get_irq_dpci(d);
+    if ( hvm_irq_dpci != NULL )
+    {
+        int ret = 0;
+
+        if ( hvm_irq_dpci->pending_pirq_dpci )
+        {
+            if ( pt_pirq_softirq_active(hvm_irq_dpci->pending_pirq_dpci) )
+                 ret = -ERESTART;
+            else
+                 hvm_irq_dpci->pending_pirq_dpci = NULL;
+        }
+
+        if ( !ret )
+            ret = pt_pirq_iterate(d, pci_clean_dpci_irq, NULL);
+        if ( ret )
+        {
+            spin_unlock(&d->event_lock);
+            return ret;
+        }
+
+        hvm_domain_irq(d)->dpci = NULL;
+        free_hvm_irq_dpci(hvm_irq_dpci);
+    }
+    spin_unlock(&d->event_lock);
+
+    return 0;
+}
+
 /*
  * Note: 'pt_pirq_softirq_reset' can clear the STATE_SCHED before we get to
  * doing it. If that is the case we let 'pt_pirq_softirq_reset' do ref-counting.
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c
index f17b1820f4..875e67b53b 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -308,6 +308,13 @@  struct page_info *iommu_alloc_pgtable(struct domain *d)
     return pg;
 }
 
+int iommu_update_ire_from_msi(
+    struct msi_desc *msi_desc, struct msi_msg *msg)
+{
+    return iommu_intremap
+           ? iommu_call(&iommu_ops, update_ire_from_msi, msi_desc, msg) : 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/pci.h b/xen/include/xen/pci.h
index 20a54a5bb4..78d83afe64 100644
--- a/xen/include/xen/pci.h
+++ b/xen/include/xen/pci.h
@@ -208,4 +208,6 @@  int msixtbl_pt_register(struct domain *, struct pirq *, uint64_t gtable);
 void msixtbl_pt_unregister(struct domain *, struct pirq *);
 void msixtbl_pt_cleanup(struct domain *d);
 
+int arch_pci_clean_pirqs(struct domain *d);
+
 #endif /* __XEN_PCI_H__ */