From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751823AbcEIIGj (ORCPT ); Mon, 9 May 2016 04:06:39 -0400 Received: from mail-wm0-f42.google.com ([74.125.82.42]:38589 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751775AbcEIIGg (ORCPT ); Mon, 9 May 2016 04:06:36 -0400 Subject: Re: [PATCH v9 5/7] vfio/type1: also check IRQ remapping capability at msi domain To: "Chalamarla, Tirumalesh" , "eric.auger@st.com" , "robin.murphy@arm.com" , "alex.williamson@redhat.com" , "will.deacon@arm.com" , "joro@8bytes.org" , "tglx@linutronix.de" , "jason@lakedaemon.net" , "marc.zyngier@arm.com" , "christoffer.dall@linaro.org" , "linux-arm-kernel@lists.infradead.org" References: <1462362858-2925-1-git-send-email-eric.auger@linaro.org> <1462362858-2925-6-git-send-email-eric.auger@linaro.org> Cc: "julien.grall@arm.com" , "patches@linaro.org" , "Jean-Philippe.Brucker@arm.com" , "p.fedin@samsung.com" , "linux-kernel@vger.kernel.org" , "Bharat.Bhushan@freescale.com" , "iommu@lists.linux-foundation.org" , "pranav.sawargaonkar@gmail.com" , "yehuday@marvell.com" From: Eric Auger Message-ID: <573044B4.4000905@linaro.org> Date: Mon, 9 May 2016 10:05:08 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chalamarla, On 05/05/2016 09:23 PM, Chalamarla, Tirumalesh wrote: > > > > > > On 5/4/16, 4:54 AM, "linux-arm-kernel on behalf of Eric Auger" wrote: > >> On x86 IRQ remapping is abstracted by the IOMMU. On ARM this is abstracted >> by the msi controller. vfio_safe_irq_domain allows to check whether >> interrupts are "safe" for a given device. They are if the device does >> not use MSI or if the device uses MSI and the msi-parent controller >> supports IRQ remapping. >> >> Then we check at group level if all devices have safe interrupts: if not, >> we only allow the group to be attached if allow_unsafe_interrupts is set. >> >> At this point ARM sMMU still advertises IOMMU_CAP_INTR_REMAP. This is >> changed in next patch. > > Will this work in systems with multiple ITS? Yes it should support multiple ITS. Please note however that the series does not yet implement the msi_doorbell_info callback in GICv3 ITS PCIe/platform irqchip. Without that, the pci_enable is going to fail. I am going to submit a separate RFC for that but I can't test it at the moment. Best Regards Eric >> >> Signed-off-by: Eric Auger >> >> --- >> v3 -> v4: >> - rename vfio_msi_parent_irq_remapping_capable into vfio_safe_irq_domain >> and irq_remapping into safe_irq_domains >> >> v2 -> v3: >> - protect vfio_msi_parent_irq_remapping_capable with >> CONFIG_GENERIC_MSI_IRQ_DOMAIN >> --- >> drivers/vfio/vfio_iommu_type1.c | 44 +++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 42 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c >> index 4d3a6f1..2fc8197 100644 >> --- a/drivers/vfio/vfio_iommu_type1.c >> +++ b/drivers/vfio/vfio_iommu_type1.c >> @@ -37,6 +37,8 @@ >> #include >> #include >> #include >> +#include >> +#include >> >> #define DRIVER_VERSION "0.2" >> #define DRIVER_AUTHOR "Alex Williamson " >> @@ -777,6 +779,33 @@ static int vfio_bus_type(struct device *dev, void *data) >> return 0; >> } >> >> +/** >> + * vfio_safe_irq_domain: returns whether the irq domain >> + * the device is attached to is safe with respect to MSI isolation. >> + * If the irq domain is not an MSI domain, we return it is safe. >> + * >> + * @dev: device handle >> + * @data: unused >> + * returns 0 if the irq domain is safe, -1 if not. >> + */ >> +static int vfio_safe_irq_domain(struct device *dev, void *data) >> +{ >> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN >> + struct irq_domain *domain; >> + struct msi_domain_info *info; >> + >> + domain = dev_get_msi_domain(dev); >> + if (!domain) >> + return 0; >> + >> + info = msi_get_domain_info(domain); >> + >> + if (!(info->flags & MSI_FLAG_IRQ_REMAPPING)) >> + return -1; >> +#endif >> + return 0; >> +} >> + >> static int vfio_iommu_replay(struct vfio_iommu *iommu, >> struct vfio_domain *domain) >> { >> @@ -870,7 +899,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> struct vfio_group *group, *g; >> struct vfio_domain *domain, *d; >> struct bus_type *bus = NULL; >> - int ret; >> + int ret, safe_irq_domains; >> >> mutex_lock(&iommu->lock); >> >> @@ -893,6 +922,13 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> >> group->iommu_group = iommu_group; >> >> + /* >> + * Determine if all the devices of the group have a safe irq domain >> + * with respect to MSI isolation >> + */ >> + safe_irq_domains = !iommu_group_for_each_dev(iommu_group, &bus, >> + vfio_safe_irq_domain); >> + >> /* Determine bus_type in order to allocate a domain */ >> ret = iommu_group_for_each_dev(iommu_group, &bus, vfio_bus_type); >> if (ret) >> @@ -920,8 +956,12 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, >> INIT_LIST_HEAD(&domain->group_list); >> list_add(&group->next, &domain->group_list); >> >> + /* >> + * to advertise safe interrupts either the IOMMU or the MSI controllers >> + * must support IRQ remapping/interrupt translation >> + */ >> if (!allow_unsafe_interrupts && >> - !iommu_capable(bus, IOMMU_CAP_INTR_REMAP)) { >> + (!iommu_capable(bus, IOMMU_CAP_INTR_REMAP) && !safe_irq_domains)) { >> pr_warn("%s: No interrupt remapping support. Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n", >> __func__); >> ret = -EPERM; >> -- >> 1.9.1 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel