Hello Stefano, > On 17 Nov 2020, at 1:20 am, Stefano Stabellini <sstabell...@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.si...@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 >>