Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754397AbcDVOuD (ORCPT ); Fri, 22 Apr 2016 10:50:03 -0400 Received: from foss.arm.com ([217.140.101.70]:35014 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754339AbcDVOt5 (ORCPT ); Fri, 22 Apr 2016 10:49:57 -0400 Subject: Re: [PATCH v7 01/10] iommu: Add DOMAIN_ATTR_MSI_MAPPING attribute To: Eric Auger , eric.auger@st.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: <1461084994-2355-1-git-send-email-eric.auger@linaro.org> <1461084994-2355-2-git-send-email-eric.auger@linaro.org> <57177A4E.4090804@arm.com> <5717A738.9090904@linaro.org> <571A0B78.6070509@arm.com> <571A1273.9060808@linaro.org> Cc: patches@linaro.org, linux-kernel@vger.kernel.org, Bharat.Bhushan@freescale.com, pranav.sawargaonkar@gmail.com, p.fedin@samsung.com, iommu@lists.linux-foundation.org, Jean-Philippe.Brucker@arm.com, julien.grall@arm.com From: Robin Murphy Message-ID: <571A3A10.70601@arm.com> Date: Fri, 22 Apr 2016 15:49:52 +0100 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: <571A1273.9060808@linaro.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4752 Lines: 132 On 22/04/16 13:00, Eric Auger wrote: > Hi Robin, > On 04/22/2016 01:31 PM, Robin Murphy wrote: >> On 20/04/16 16:58, Eric Auger wrote: >>> Hi Robin, >>> On 04/20/2016 02:47 PM, Robin Murphy wrote: >>>> Hi Eric, >>>> >>>> On 19/04/16 17:56, Eric Auger wrote: >>>>> Introduce a new DOMAIN_ATTR_MSI_MAPPING domain attribute. If supported, >>>>> this means the MSI addresses need to be mapped in the IOMMU. >>>>> >>>>> x86 IOMMUs typically don't expose the attribute since on x86, MSI write >>>>> transaction addresses always are within the 1MB PA region [FEE0_0000h - >>>>> FEF0_000h] window which directly targets the APIC configuration >>>>> space and >>>>> hence bypass the sMMU. On ARM and PowerPC however MSI transactions are >>>>> conveyed through the IOMMU. >>>> >>>> What's stopping us from simply inferring this from the domain's IOMMU >>>> not advertising interrupt remapping capabilities? >>> My current understanding is it is not possible: >>> on x86 CAP_INTR_REMAP is not systematically exposed (the feature can be >>> disabled) and MSIs are never mapped in the IOMMU I think. >> >> Not sure I follow - if the feature is disabled such that the IOMMU >> doesn't isolate MSIs, then it's no different a situation from the SMMU, no? > > sorry I understood you wanted to use IOMMU_CAP_INTR_REMAP as the sole > criteria to detect whether MSI mapping was requested. >> >> My point was that this logic: >> >> if (IOMMU_CAP_INTR_REMAP) >> we're good >> else if (DOMAIN_ATTR_MSI_MAPPING) >> if (acquire_msi_remapping_resources(domain)) >> we're good >> else >> oh no! >> else >> oh no! >> >> should be easily reducible to this: >> >> if (IOMMU_CAP_INTR_REMAP) >> we're good >> else if (acquire_msi_remapping_resources(domain)) > > But Can't we imagine a mix of smmus on the same platform, some > requesting MSI mapping and some which don't. As soon as an smmu requires > MSI mapping, CONFIG_IOMMU_DMA_RESERVED is set and > acquire_msi_remapping_resources(domain) will be implemented & succeed. > Doesn't it lead to a wrong decision. Do I miss something, or do you > consider this situation as far-fetched? Sorry, what was implicit there was that the imaginary acquire_msi_remapping_resources(*IOMMU* domian) call still involves going all the way down to check for MSI_FLAG_IRQ_REMAPPING in the relevant place. Either way, now that I consider it further, a flag on the IOMMU domain is not just unnecessary, I think it's actually fundamentally incorrect: picture a system which for some crazy reason has both a GICv3 ITS plus some other dumb v2m-like MMIO-SPI bridge - whether a device's MSI domain targets the (safe) ITS or the (unsafe) bridge isn't a property of the IOMMU domain it's trying to attach to; if you can't rely on the IOMMU itself to isolate MSIs, then you can only know whether to allow or reject any given group by inspecting every device in that group to make sure they all have isolation provided by their MSI domains and that the IOMMU domain has all the relevant doorbell mappings ready. I guess the allow_unsafe_interrupts case might complicate matters beyond the logic I sketched out, because then we might actually care about the difference between "is isolation provided?" and "are sufficient IOVA/descriptor resources available?", but the main point still stands. Robin. > Thanks > > Eric > >> we're good >> else >> oh no! // Don't care whether the domain ran out of >> // resources or simply doesn't support it, >> // either way we can't proceed. >> >> Robin. >> >>> Best Regards >>> >>> Eric >>>> >>>> Robin. >>>> >>>>> Signed-off-by: Bharat Bhushan >>>>> Signed-off-by: Eric Auger >>>>> >>>>> --- >>>>> >>>>> v4 -> v5: >>>>> - introduce the user in the next patch >>>>> >>>>> RFC v1 -> v1: >>>>> - the data field is not used >>>>> - for this attribute domain_get_attr simply returns 0 if the >>>>> MSI_MAPPING >>>>> capability if needed or <0 if not. >>>>> - removed struct iommu_domain_msi_maps >>>>> --- >>>>> include/linux/iommu.h | 1 + >>>>> 1 file changed, 1 insertion(+) >>>>> >>>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h >>>>> index 62a5eae..b3e8c5b 100644 >>>>> --- a/include/linux/iommu.h >>>>> +++ b/include/linux/iommu.h >>>>> @@ -113,6 +113,7 @@ enum iommu_attr { >>>>> DOMAIN_ATTR_FSL_PAMU_ENABLE, >>>>> DOMAIN_ATTR_FSL_PAMUV1, >>>>> DOMAIN_ATTR_NESTING, /* two stages of translation */ >>>>> + DOMAIN_ATTR_MSI_MAPPING, /* Require MSIs mapping in iommu */ >>>>> DOMAIN_ATTR_MAX, >>>>> }; >>>>> >>>>> >>>> >>> >> >