From mboxrd@z Thu Jan 1 00:00:00 1970 From: Varun Sethi Subject: RE: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block Date: Wed, 22 Jan 2014 13:54:11 +0000 Message-ID: References: <1389876263-25759-1-git-send-email-andreas.herrmann@calxeda.com> <1389876263-25759-3-git-send-email-andreas.herrmann@calxeda.com> <20140120222814.GI3471@alberich> <20140122122550.GA14108@mudshark.cambridge.arm.com> <20140122134028.GB14108@mudshark.cambridge.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20140122134028.GB14108-MRww78TxoiP5vMa5CHWGZ34zcgK1vI+I0E9HWUfgJXw@public.gmane.org> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Will Deacon Cc: Andreas Herrmann , "iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: iommu@lists.linux-foundation.org > -----Original Message----- > From: Will Deacon [mailto:will.deacon-5wv7dgnIgG8@public.gmane.org] > Sent: Wednesday, January 22, 2014 7:10 PM > To: Sethi Varun-B16395 > Cc: Andreas Herrmann; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; linux-arm- > kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; Andreas Herrmann > Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group > notifier block > > On Wed, Jan 22, 2014 at 01:14:13PM +0000, Varun Sethi wrote: > > > On Tue, Jan 21, 2014 at 05:48:02PM +0000, Varun Sethi wrote: > > > > > +static int arm_smmu_group_notifier(struct notifier_block *nb, > > > > > + unsigned long action, void *data) { > > > > > + struct device *dev = data; > > > > > + struct dma_iommu_mapping *mapping; > > > > > + struct arm_smmu_device *smmu; > > > > > + int ret; > > > > > + > > > > > + switch (action) { > > > > > + case IOMMU_GROUP_NOTIFY_BIND_DRIVER: > > > > > + > > > > > + smmu = dev->archdata.iommu; > > > > > + if (!smmu || !(smmu->options & > ARM_SMMU_OPT_ISOLATE_DEVICES)) > > > > > + break; > > > > [Sethi Varun-B16395] Should this check be really done here? The > > > > "Isolate devices" property would allow us to set up iommu groups. > > > > My understanding is that if we specify the isolate devices > > > > property, then each device would have a separate iommu group > > > > otherwise all devices connected to the SMMU would share the iommu > group. > > > > > > That's not what currently happens (at least, in the patch I have > > > queued for groups). The code queued adds each device to its own > > > group in arm_smmu_add_device, which I think is the right thing to do. > > > > > > > With that logic, we should link the mapping to the iommu group. > > > > > > Ok, so are you suggesting that we perform the isolation mapping in > > > arm_smmu_add_device and drop the notifier altogether? > > I think that should be fine, until we want to delay mapping creation > > till driver bind time. > > Is there a hard dependency on that? > Not sure, may be Andreas can answer that. > > But the "isolate device" property should dictate iommu group creation. > > The reason we added automatic group creation (per-device) is for VFIO, > which expects all devices to be in a group regardless of the device > isolation configuration. > IIUC, with the "isolate devices" property we ensure that there would be independent SMR and S2CR per device. Is that correct? -Varun From mboxrd@z Thu Jan 1 00:00:00 1970 From: Varun.Sethi@freescale.com (Varun Sethi) Date: Wed, 22 Jan 2014 13:54:11 +0000 Subject: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group notifier block In-Reply-To: <20140122134028.GB14108@mudshark.cambridge.arm.com> References: <1389876263-25759-1-git-send-email-andreas.herrmann@calxeda.com> <1389876263-25759-3-git-send-email-andreas.herrmann@calxeda.com> <20140120222814.GI3471@alberich> <20140122122550.GA14108@mudshark.cambridge.arm.com> <20140122134028.GB14108@mudshark.cambridge.arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org > -----Original Message----- > From: Will Deacon [mailto:will.deacon at arm.com] > Sent: Wednesday, January 22, 2014 7:10 PM > To: Sethi Varun-B16395 > Cc: Andreas Herrmann; iommu at lists.linux-foundation.org; linux-arm- > kernel at lists.infradead.org; Andreas Herrmann > Subject: Re: [PATCH v3 02/11] iommu/arm-smmu: Introduce iommu_group > notifier block > > On Wed, Jan 22, 2014 at 01:14:13PM +0000, Varun Sethi wrote: > > > On Tue, Jan 21, 2014 at 05:48:02PM +0000, Varun Sethi wrote: > > > > > +static int arm_smmu_group_notifier(struct notifier_block *nb, > > > > > + unsigned long action, void *data) { > > > > > + struct device *dev = data; > > > > > + struct dma_iommu_mapping *mapping; > > > > > + struct arm_smmu_device *smmu; > > > > > + int ret; > > > > > + > > > > > + switch (action) { > > > > > + case IOMMU_GROUP_NOTIFY_BIND_DRIVER: > > > > > + > > > > > + smmu = dev->archdata.iommu; > > > > > + if (!smmu || !(smmu->options & > ARM_SMMU_OPT_ISOLATE_DEVICES)) > > > > > + break; > > > > [Sethi Varun-B16395] Should this check be really done here? The > > > > "Isolate devices" property would allow us to set up iommu groups. > > > > My understanding is that if we specify the isolate devices > > > > property, then each device would have a separate iommu group > > > > otherwise all devices connected to the SMMU would share the iommu > group. > > > > > > That's not what currently happens (at least, in the patch I have > > > queued for groups). The code queued adds each device to its own > > > group in arm_smmu_add_device, which I think is the right thing to do. > > > > > > > With that logic, we should link the mapping to the iommu group. > > > > > > Ok, so are you suggesting that we perform the isolation mapping in > > > arm_smmu_add_device and drop the notifier altogether? > > I think that should be fine, until we want to delay mapping creation > > till driver bind time. > > Is there a hard dependency on that? > Not sure, may be Andreas can answer that. > > But the "isolate device" property should dictate iommu group creation. > > The reason we added automatic group creation (per-device) is for VFIO, > which expects all devices to be in a group regardless of the device > isolation configuration. > IIUC, with the "isolate devices" property we ensure that there would be independent SMR and S2CR per device. Is that correct? -Varun