[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] xen/pci: Refactor PCI MSI interrupts related code



Hi Roger,

On 15/04/2021 14:26, Roger Pau Monné wrote:
On Wed, Apr 14, 2021 at 09:49:37AM +0100, Julien Grall wrote:
Hi Roger,

On 14/04/2021 09:05, Roger Pau Monné wrote:
On Tue, Apr 13, 2021 at 06:12:10PM +0100, Julien Grall wrote:
Hi Roger,

On 12/04/2021 11:49, Roger Pau Monné wrote:
On Fri, Apr 09, 2021 at 05:00:41PM +0100, Rahul Singh wrote:
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 705137f8be..1b473502a1 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1303,12 +1279,15 @@ static int __init setup_dump_pcidevs(void)
    }
    __initcall(setup_dump_pcidevs);
+
+#ifdef CONFIG_PCI_MSI_INTERCEPT
    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;
    }
+#endif

This is not exactly related to MSI intercepts, the IOMMU interrupt
remapping table will also be used for interrupt entries for devices
being used by Xen directly, where no intercept is required.

On Arm, this is not tie to the IOMMU. Instead, this handled is a separate
MSI controller (such as the ITS).


And then you also want to gate the hook from iommu_ops itself with
CONFIG_PCI_MSI_INTERCEPT, if we want to got this route.


All the callers are in the x86 code. So how about moving the function in an
x86 specific file?

Yes, that seems fine. Just place it in asm-x86/iommu.h. I wonder why
update_ire_from_msi wasn't moved when the rest of the x86 specific
functions where moved there.

I am guessing it is because the function was protected by CONFIG_HAS_PCI
rather than CONFIG_X86. So it was deferred until another arch enables
CONFIG_HAS_PCI (it is easier to know what code should be moved).

Was there an aim to use that in other
arches?

In the future we may need to use MSIs in Xen (IIRC some SMMUs only support
MSI interrupt).

Then I think some of the bits that are moved from
xen/drivers/passthrough/pci.c (alloc_pci_msi, free_pci_msi and
dump_pci_msi) need to be protected by a Kconfig option different than
CONFIG_PCI_MSI_INTERCEPT, as those are not related to MSI interception,
but to MSI handling in general (ie: Xen devices using MSI need
those).

Well... x86-specific devices yes. We don't know in what form MSI will be added on for other arch-specific devices.

 The same with the msi_list and msix fields of pci_dev, those
are not only used for MSI interception.

Or at least might be worth mentioning that the functions will be
needed for Xen to be able to use MSI interrupts itself, even if not
for interception purposes.

My preference would be to comment in the commit message (although, there is no promise they will ever get re-used). We can then modify the #ifdef once we introduce any use.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.