Re: [PATCH v4 0/5] memory: Introduce memory controller mini-framework
On Thu, Feb 13, 2020 at 07:15:55PM +0100, Thierry Reding wrote: > On Thu, Feb 13, 2020 at 05:23:23PM +, Robin Murphy wrote: > > [+ Maxime] > > > > On 13/02/2020 4:39 pm, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > Hi, > > > > > > this set of patches adds a new binding that allows device tree nodes to > > > explicitly define the DMA parent for a given device. This supplements > > > the existing interconnect bindings and is useful to disambiguate in the > > > case where a device has multiple paths to system memory. Beyond that it > > > can also be useful when there aren't any actual interconnect paths that > > > can be controlled, so in simple cases this can serve as a simpler > > > variant of interconnect paths. > > > > Isn't that still squarely the intent of the "dma-mem" binding, though? i.e. > > it's not meant to be a 'real' interconnect provider, but a very simple way > > to encode DMA parentage piggybacked onto a more general binding (with the > > *option* of being a full-blown interconnect if it wants to, but certainly no > > expectation). > > The way that this works on Tegra is that we want to describe multiple > interconnect paths. A typical device will have a read and a write memory > client, which can be separately "tuned". Both of these paths will target > system memory, so they would both technically be "dma-mem" paths. But > that would make it impossible to treat them separately elsewhere. > > So we could choose any of them to be the "dma-mem" path, but then we > need to be very careful about defining which one that is, so that > drivers know how to look them up, which is also not really desirable. > > One other things we could do is to duplicate one of the entries, so that > we'd have "read", "write" and "dma-mem" interconnect paths, with > "dma-mem" referencing the same path as "read" or "write". That doesn't > sound *too* bad, but it's still a bit of a hack. Having an explicit > description for this sounds much clearer and less error prone to me. IIRC the dmaengine binding allows to do that, so it would make sense to me to have the same thing allowed for interconnects. Maxime signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu: arm-smmu-impl: Convert to a generic reset implementation
Quoting Sai Prakash Ranjan (2020-01-22 03:48:01) > Currently the QCOM specific smmu reset implementation is very > specific to SDM845 SoC and has a wait-for-safe logic which > may not be required for other SoCs. So move the SDM845 specific > logic to its specific reset function. Also add SC7180 SMMU > compatible for calling into QCOM specific implementation. > > Signed-off-by: Sai Prakash Ranjan > --- Reviewed-by: Stephen Boyd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm-smmu-v3: Add SMMUv3.2 range invalidation support
On Thu, Jan 30, 2020 at 11:34 AM Robin Murphy wrote: > > On 30/01/2020 3:06 pm, Auger Eric wrote: > > Hi Rob, > > On 1/17/20 10:16 PM, Rob Herring wrote: > >> Arm SMMUv3.2 adds support for TLB range invalidate operations. > >> Support for range invalidate is determined by the RIL bit in the IDR3 > >> register. > >> > >> The range invalidate is in units of the leaf page size and operates on > >> 1-32 chunks of a power of 2 multiple pages. First, we determine from the > >> size what power of 2 multiple we can use. Then we calculate how many > >> chunks (1-31) of the power of 2 size for the range on the iteration. On > >> each iteration, we move up in size by at least 5 bits. > >> > >> Cc: Eric Auger > >> Cc: Jean-Philippe Brucker > >> Cc: Will Deacon > >> Cc: Robin Murphy > >> Cc: Joerg Roedel > >> Signed-off-by: Rob Herring > >> @@ -2003,7 +2024,7 @@ static void arm_smmu_tlb_inv_range(unsigned long > >> iova, size_t size, > >> { > >> u64 cmds[CMDQ_BATCH_ENTRIES * CMDQ_ENT_DWORDS]; > >> struct arm_smmu_device *smmu = smmu_domain->smmu; > >> -unsigned long start = iova, end = iova + size; > >> +unsigned long start = iova, end = iova + size, num_pages = 0, tg = 0; > >> int i = 0; > >> struct arm_smmu_cmdq_ent cmd = { > >> .tlbi = { > >> @@ -2022,12 +2043,50 @@ static void arm_smmu_tlb_inv_range(unsigned long > >> iova, size_t size, > >> cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid; > >> } > >> > >> +if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { > >> +/* Get the leaf page size */ > >> +tg = __ffs(smmu_domain->domain.pgsize_bitmap); > >> + > >> +/* Convert page size of 12,14,16 (log2) to 1,2,3 */ > >> +cmd.tlbi.tg = ((tg - ilog2(SZ_4K)) / 2) + 1; > > Given the comment, I think "(tg - 10) / 2" would suffice ;) Well, duh... > > >> + > >> +/* Determine what level the granule is at */ > >> +cmd.tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); > > Is it possible to rephrase that with logs and shifts to avoid the division? Well, with a loop is the only other way I came up with: bpl = tg - 3; ttl = 3; mask = BIT(bpl) - 1; while ((granule & (mask << ((4 - ttl) * bpl + 3))) == 0) ttl--; Doesn't seem like much improvement to me given we have h/w divide... > > >> + > >> +num_pages = size / (1UL << tg); > > Similarly, in my experience GCC has always seemed too cautious to elide > non-constant division even in a seemingly-obvious case like this, so > explicit "size >> tg" might be preferable. My experience has been gcc is smart enough. I checked and there's only one divide and it is the prior one. But I'll change it anyways. > > >> +} > >> + > >> while (iova < end) { > >> if (i == CMDQ_BATCH_ENTRIES) { > >> arm_smmu_cmdq_issue_cmdlist(smmu, cmds, i, false); > >> i = 0; > >> } > >> > >> +if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { > >> +/* > >> + * On each iteration of the loop, the range is 5 bits > >> + * worth of the aligned size remaining. > >> + * The range in pages is: > >> + * > >> + * range = (num_pages & (0x1f << __ffs(num_pages))) > >> + */ > >> +unsigned long scale, num; > >> + > >> +/* Determine the power of 2 multiple number of pages > >> */ > >> +scale = __ffs(num_pages); > >> +cmd.tlbi.scale = scale; > >> + > >> +/* Determine how many chunks of 2^scale size we have > >> */ > >> +num = (num_pages >> scale) & CMDQ_TLBI_RANGE_NUM_MAX; > >> +cmd.tlbi.num = num - 1; > >> + > >> +/* range is num * 2^scale * pgsize */ > >> +granule = num << (scale + tg); > >> + > >> +/* Clear out the lower order bits for the next > >> iteration */ > >> +num_pages -= num << scale; > > Regarding the 2 options given in > > https://lore.kernel.org/linux-arm-kernel/CAL_JsqKABoE+0crGwyZdNogNgEoG=moopf6deqgh6s73c0u...@mail.gmail.com/raw, > > > > I understand you implemented 2) but I still do not understand why you > > preferred that one against 1). > > > > In your case of 1023*4k pages this will invalidate by 31 32*2^0*4K + > > 31*2^0*4K pages > > whereas you could achieve that with 10 invalidations with the 1st algo. > > I did not get the case where it is more efficient. Please can you detail. > > I guess essentially we want to solve for two variables to get a range as > close to size as possible. There might be a more elegant numerical > method, but for the numbers involved brute force is probably good enough > for the real world. 5 minutes alone with the architecture spec and a > blank editor begat this
[PATCH v2] iommu/arm-smmu-v3: Batch ATC invalidation commands
Similar to commit 2af2e72b18b4 ("iommu/arm-smmu-v3: Defer TLB invalidation until ->iotlb_sync()"), build up a list of ATC invalidation commands and submit them all at once to the command queue instead of one-by-one. As there is only one caller of arm_smmu_atc_inv_master() left, we can simplify it and avoid passing in struct arm_smmu_cmdq_ent. Cc: Jean-Philippe Brucker Cc: Will Deacon Cc: Robin Murphy Cc: Joerg Roedel Signed-off-by: Rob Herring --- v2: - Simplify arm_smmu_atc_inv_master() - Rebase on v5.6-rc1 drivers/iommu/arm-smmu-v3.c | 38 - 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index aa3ac2a03807..8161d9e6c068 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2132,17 +2132,16 @@ arm_smmu_atc_inv_to_cmd(int ssid, unsigned long iova, size_t size, cmd->atc.size = log2_span; } -static int arm_smmu_atc_inv_master(struct arm_smmu_master *master, - struct arm_smmu_cmdq_ent *cmd) +static int arm_smmu_atc_inv_master(struct arm_smmu_master *master) { int i; + struct arm_smmu_cmdq_ent cmd; - if (!master->ats_enabled) - return 0; + arm_smmu_atc_inv_to_cmd(0, 0, 0, ); for (i = 0; i < master->num_sids; i++) { - cmd->atc.sid = master->sids[i]; - arm_smmu_cmdq_issue_cmd(master->smmu, cmd); + cmd.atc.sid = master->sids[i]; + arm_smmu_cmdq_issue_cmd(master->smmu, ); } return arm_smmu_cmdq_issue_sync(master->smmu); @@ -2151,10 +2150,11 @@ static int arm_smmu_atc_inv_master(struct arm_smmu_master *master, static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, int ssid, unsigned long iova, size_t size) { - int ret = 0; + int i, cmdn = 0; unsigned long flags; struct arm_smmu_cmdq_ent cmd; struct arm_smmu_master *master; + u64 cmds[CMDQ_BATCH_ENTRIES * CMDQ_ENT_DWORDS]; if (!(smmu_domain->smmu->features & ARM_SMMU_FEAT_ATS)) return 0; @@ -2179,11 +2179,25 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain, arm_smmu_atc_inv_to_cmd(ssid, iova, size, ); spin_lock_irqsave(_domain->devices_lock, flags); - list_for_each_entry(master, _domain->devices, domain_head) - ret |= arm_smmu_atc_inv_master(master, ); + list_for_each_entry(master, _domain->devices, domain_head) { + if (!master->ats_enabled) + continue; + + for (i = 0; i < master->num_sids; i++) { + if (cmdn == CMDQ_BATCH_ENTRIES) { + arm_smmu_cmdq_issue_cmdlist(smmu_domain->smmu, + cmds, cmdn, false); + cmdn = 0; + } + + cmd.atc.sid = master->sids[i]; + arm_smmu_cmdq_build_cmd([cmdn * CMDQ_ENT_DWORDS], ); + cmdn++; + } + } spin_unlock_irqrestore(_domain->devices_lock, flags); - return ret ? -ETIMEDOUT : 0; + return arm_smmu_cmdq_issue_cmdlist(smmu_domain->smmu, cmds, cmdn, true); } /* IO_PGTABLE API */ @@ -2611,7 +2625,6 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master) static void arm_smmu_disable_ats(struct arm_smmu_master *master) { - struct arm_smmu_cmdq_ent cmd; struct arm_smmu_domain *smmu_domain = master->domain; if (!master->ats_enabled) @@ -2623,8 +2636,7 @@ static void arm_smmu_disable_ats(struct arm_smmu_master *master) * ATC invalidation via the SMMU. */ wmb(); - arm_smmu_atc_inv_to_cmd(0, 0, 0, ); - arm_smmu_atc_inv_master(master, ); + arm_smmu_atc_inv_master(master); atomic_dec(_domain->nr_ats_masters); } -- 2.20.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/arm-smmu-v3: Add SMMUv3.2 range invalidation support
On Thu, Jan 30, 2020 at 9:06 AM Auger Eric wrote: > > Hi Rob, > On 1/17/20 10:16 PM, Rob Herring wrote: > > Arm SMMUv3.2 adds support for TLB range invalidate operations. > > Support for range invalidate is determined by the RIL bit in the IDR3 > > register. > > > > The range invalidate is in units of the leaf page size and operates on > > 1-32 chunks of a power of 2 multiple pages. First, we determine from the > > size what power of 2 multiple we can use. Then we calculate how many > > chunks (1-31) of the power of 2 size for the range on the iteration. On > > each iteration, we move up in size by at least 5 bits. > > > > Cc: Eric Auger > > Cc: Jean-Philippe Brucker > > Cc: Will Deacon > > Cc: Robin Murphy > > Cc: Joerg Roedel > > Signed-off-by: Rob Herring > > --- > > drivers/iommu/arm-smmu-v3.c | 66 - > > 1 file changed, 65 insertions(+), 1 deletion(-) > > @@ -2003,7 +2024,7 @@ static void arm_smmu_tlb_inv_range(unsigned long > > iova, size_t size, > > { > > u64 cmds[CMDQ_BATCH_ENTRIES * CMDQ_ENT_DWORDS]; > > struct arm_smmu_device *smmu = smmu_domain->smmu; > > - unsigned long start = iova, end = iova + size; > > + unsigned long start = iova, end = iova + size, num_pages = 0, tg = 0; > > int i = 0; > > struct arm_smmu_cmdq_ent cmd = { > > .tlbi = { > > @@ -2022,12 +2043,50 @@ static void arm_smmu_tlb_inv_range(unsigned long > > iova, size_t size, > > cmd.tlbi.vmid = smmu_domain->s2_cfg.vmid; > > } > > > > + if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { > > + /* Get the leaf page size */ > > + tg = __ffs(smmu_domain->domain.pgsize_bitmap); > > + > > + /* Convert page size of 12,14,16 (log2) to 1,2,3 */ > > + cmd.tlbi.tg = ((tg - ilog2(SZ_4K)) / 2) + 1; > > + > > + /* Determine what level the granule is at */ > > + cmd.tlbi.ttl = 4 - ((ilog2(granule) - 3) / (tg - 3)); > > + > > + num_pages = size / (1UL << tg); > > + } > > + > > while (iova < end) { > > if (i == CMDQ_BATCH_ENTRIES) { > > arm_smmu_cmdq_issue_cmdlist(smmu, cmds, i, false); > > i = 0; > > } > > > > + if (smmu->features & ARM_SMMU_FEAT_RANGE_INV) { > > + /* > > + * On each iteration of the loop, the range is 5 bits > > + * worth of the aligned size remaining. > > + * The range in pages is: > > + * > > + * range = (num_pages & (0x1f << __ffs(num_pages))) > > + */ > > + unsigned long scale, num; > > + > > + /* Determine the power of 2 multiple number of pages > > */ > > + scale = __ffs(num_pages); > > + cmd.tlbi.scale = scale; > > + > > + /* Determine how many chunks of 2^scale size we have > > */ > > + num = (num_pages >> scale) & CMDQ_TLBI_RANGE_NUM_MAX; > > + cmd.tlbi.num = num - 1; > > + > > + /* range is num * 2^scale * pgsize */ > > + granule = num << (scale + tg); > > + > > + /* Clear out the lower order bits for the next > > iteration */ > > + num_pages -= num << scale; > Regarding the 2 options given in > https://lore.kernel.org/linux-arm-kernel/CAL_JsqKABoE+0crGwyZdNogNgEoG=moopf6deqgh6s73c0u...@mail.gmail.com/raw, > > I understand you implemented 2) but I still do not understand why you > preferred that one against 1). > > In your case of 1023*4k pages this will invalidate by 31 32*2^0*4K + > 31*2^0*4K pages > whereas you could achieve that with 10 invalidations with the 1st algo. > I did not get the case where it is more efficient. Please can you detail. No, it's only 2 commands. We do 31*4K and then 31*2^5*4K. Here's a the output of a test case: iova=10001000, num_pages=0x3e0, granule=1f000, num=31, scale=0, ttl=3 iova=1002, num_pages=0x0, granule=3e, num=31, scale=5, ttl=3 (num_pages being what's left at end of the loop) As I mentioned on v1, worst case is 4 commands for up to 4GB. It's 20-bits of size (32-12) and each loop processes a minimum of 5 bits. Each loop becomes a larger aligned size, so scale goes up each pass. This is what I tried to explain in the top comment. Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: arm64 iommu groups issue
On 13/02/2020 3:49 pm, John Garry wrote: The underlying issue is that, for historical reasons, OF/IORT-based IOMMU drivers have ended up with group allocation being tied to endpoint driver probing via the dma_configure() mechanism (long story short, driver probe is the only thing which can be delayed in order to wait for a specific IOMMU instance to be ready).However, in the meantime, the IOMMU API internals have evolved sufficiently that I think there's a way to really put things right - I have the spark of an idea which I'll try to sketch out ASAP... OK, great. Hi Robin, I was wondering if you have had a chance to consider this problem again? Yeah, sorry, more things came up such that it still hasn't been P yet ;) Lorenzo and I did get as far as discussing it a bit more and writing up a ticket, so it's formally on our internal radar now, at least. One simple idea could be to introduce a device link between the endpoint device and its parent bridge to ensure that they probe in order, as expected in pci_device_group(): Subject: [PATCH] PCI: Add device link to ensure endpoint device driver probes after bridge It is required to ensure that a device driver for an endpoint will probe after the parent port driver, so add a device link for this. --- diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 512cb4312ddd..4b832ad25b20 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2383,6 +2383,7 @@ static void pci_set_msi_domain(struct pci_dev *dev) void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) { int ret; + struct device *parent; pci_configure_device(dev); @@ -2420,6 +2421,10 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) /* Set up MSI IRQ domain */ pci_set_msi_domain(dev); + parent = dev->dev.parent; + if (parent && parent->bus == _bus_type) + device_link_add(>dev, parent, DL_FLAG_AUTOPROBE_CONSUMER); + /* Notifier could use PCI capabilities */ dev->match_driver = false; ret = device_add(>dev); -- This would work, but the problem is that if the port driver fails in probing - and not just for -EPROBE_DEFER - then the child device will never probe. This very thing happens on my dev board. However we could expand the device links API to cover this sort of scenario. Yes, that's an undesirable issue, but in fact I think it's mostly indicative that involving drivers in something which is designed to happen at a level below drivers is still fundamentally wrong and doomed to be fragile at best. Another thought that crosses my mind is that when pci_device_group() walks up to the point of ACS isolation and doesn't find an existing group, it can still infer that everything it walked past *should* be put in the same group it's then eventually going to return. Unfortunately I can't see an obvious way for it to act on that knowledge, though, since recursive iommu_probe_device() is unlikely to end well. As for alternatives, it looks pretty difficult to me to disassociate the group allocation from the dma_configure path. Indeed it's non-trivial, but it really does need cleaning up at some point. Having just had yet another spark, does something like the untested super-hack below work at all? I doubt it's a viable direction to take in itself, but it could be food for thought if it at least proves the concept. Robin. ->8- diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index aa3ac2a03807..554cde76c766 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -3841,20 +3841,20 @@ static int arm_smmu_set_bus_ops(struct iommu_ops *ops) int err; #ifdef CONFIG_PCI - if (pci_bus_type.iommu_ops != ops) { + if (1) { err = bus_set_iommu(_bus_type, ops); if (err) return err; } #endif #ifdef CONFIG_ARM_AMBA - if (amba_bustype.iommu_ops != ops) { + if (1) { err = bus_set_iommu(_bustype, ops); if (err) goto err_reset_pci_ops; } #endif - if (platform_bus_type.iommu_ops != ops) { + if (1) { err = bus_set_iommu(_bus_type, ops); if (err) goto err_reset_amba_ops; diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 660eea8d1d2f..b81ae2b4d4fb 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1542,6 +1542,14 @@ static int iommu_bus_init(struct bus_type *bus, const struct iommu_ops *ops) return err; } +static int iommu_bus_replay(struct device *dev, void *data) +{ + if (dev->iommu_group) + return 0; + + return add_iommu_group(dev, data); +} + /** * bus_set_iommu - set iommu-callbacks for the bus * @bus: bus. @@ -1564,6 +1572,9 @@ int bus_set_iommu(struct bus_type *bus, const struct iommu_ops *ops) return 0;
Re: [PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS
On Thu, Feb 13, 2020 at 10:52 AM Jean-Philippe Brucker wrote: > > Copy the ats-supported flag into the pci_host_bridge structure. > > Signed-off-by: Jean-Philippe Brucker > --- > drivers/pci/controller/pci-host-common.c | 1 + > drivers/pci/of.c | 9 + > include/linux/of_pci.h | 3 +++ > 3 files changed, 13 insertions(+) > > diff --git a/drivers/pci/controller/pci-host-common.c > b/drivers/pci/controller/pci-host-common.c > index 250a3fc80ec6..a6ac927be291 100644 > --- a/drivers/pci/controller/pci-host-common.c > +++ b/drivers/pci/controller/pci-host-common.c > @@ -92,6 +92,7 @@ int pci_host_common_probe(struct platform_device *pdev, > return ret; > } > > + of_pci_host_check_ats(bridge); > platform_set_drvdata(pdev, bridge->bus); > return 0; > } > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > index 81ceeaa6f1d5..4b8a877f1e9f 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -576,6 +576,15 @@ int pci_parse_request_of_pci_ranges(struct device *dev, > } > EXPORT_SYMBOL_GPL(pci_parse_request_of_pci_ranges); > > +void of_pci_host_check_ats(struct pci_host_bridge *bridge) > +{ > + struct device_node *np = bridge->bus->dev.of_node; > + > + if (!np) > + return; > + > + bridge->ats_supported = of_property_read_bool(np, "ats-supported"); > +} Not really any point in a common function if we expect this to be only for ECAM hosts which it seems to be based on the binding. Otherwise, needs an export if not. Rob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 0/5] memory: Introduce memory controller mini-framework
On Thu, Feb 13, 2020 at 05:23:23PM +, Robin Murphy wrote: > [+ Maxime] > > On 13/02/2020 4:39 pm, Thierry Reding wrote: > > From: Thierry Reding > > > > Hi, > > > > this set of patches adds a new binding that allows device tree nodes to > > explicitly define the DMA parent for a given device. This supplements > > the existing interconnect bindings and is useful to disambiguate in the > > case where a device has multiple paths to system memory. Beyond that it > > can also be useful when there aren't any actual interconnect paths that > > can be controlled, so in simple cases this can serve as a simpler > > variant of interconnect paths. > > Isn't that still squarely the intent of the "dma-mem" binding, though? i.e. > it's not meant to be a 'real' interconnect provider, but a very simple way > to encode DMA parentage piggybacked onto a more general binding (with the > *option* of being a full-blown interconnect if it wants to, but certainly no > expectation). The way that this works on Tegra is that we want to describe multiple interconnect paths. A typical device will have a read and a write memory client, which can be separately "tuned". Both of these paths will target system memory, so they would both technically be "dma-mem" paths. But that would make it impossible to treat them separately elsewhere. So we could choose any of them to be the "dma-mem" path, but then we need to be very careful about defining which one that is, so that drivers know how to look them up, which is also not really desirable. One other things we could do is to duplicate one of the entries, so that we'd have "read", "write" and "dma-mem" interconnect paths, with "dma-mem" referencing the same path as "read" or "write". That doesn't sound *too* bad, but it's still a bit of a hack. Having an explicit description for this sounds much clearer and less error prone to me. Thierry > > One other case where this is useful is to describe the relationship > > between devices such as the memory controller and an IOMMU, for example. > > On Tegra186 and later, the memory controller is programmed with a set of > > stream IDs that are to be associated with each memory client. This > > programming needs to happen before translations through the IOMMU start, > > otherwise the used stream IDs may deviate from the expected values. The > > memory-controllers property is used in this case to ensure that the > > memory controller driver has been probed (and hence has programmed the > > stream ID mappings) before the IOMMU becomes available. > > > > Patch 1 introduces the memory controller bindings, both from the > > perspective of the provider and the consumer. Patch 2 makes use of a > > memory-controllers property to determine the DMA parent for the purpose > > of setting up DMA masks (based on the dma-ranges property of the DMA > > parent). Patch 3 introduces a minimalistic framework that is used to > > register memory controllers with along with a set of helpers to look up > > the memory controller from device tree. > > > > An example of how to register a memory controller is shown in patch 4 > > for Tegra186 (and later) and finally the ARM SMMU driver is extended to > > become a consumer of an (optional) memory controller. As described > > above, the goal is to defer probe as long as the memory controller has > > not yet programmed the stream ID mappings. > > > > Thierry > > > > Thierry Reding (5): > >dt-bindings: Add memory controller bindings > >of: Use memory-controllers property for DMA parent > >memory: Introduce memory controller mini-framework > >memory: tegra186: Register as memory controller > >iommu: arm-smmu: Get reference to memory controller > > > > .../bindings/memory-controllers/consumer.yaml | 14 + > > .../memory-controllers/memory-controller.yaml | 32 +++ > > drivers/iommu/arm-smmu.c | 11 + > > drivers/iommu/arm-smmu.h | 2 + > > drivers/memory/Makefile | 1 + > > drivers/memory/core.c | 248 ++ > > drivers/memory/tegra/tegra186.c | 9 +- > > drivers/of/address.c | 25 +- > > include/linux/memory-controller.h | 34 +++ > > 9 files changed, 366 insertions(+), 10 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/memory-controllers/consumer.yaml > > create mode 100644 > > Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml > > create mode 100644 drivers/memory/core.c > > create mode 100644 include/linux/memory-controller.h > > signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/5] memory: Introduce memory controller mini-framework
On Thu, Feb 13, 2020 at 05:03:10PM +, Robin Murphy wrote: > On 13/02/2020 4:39 pm, Thierry Reding wrote: > > From: Thierry Reding > > > > This new framework is currently nothing more than a registry of memory > > controllers, with the goal being to order device probing. One use-case > > where this is useful, for example, is a memory controller device which > > needs to program some registers before the system MMU can be enabled. > > Associating the memory controller with the SMMU allows the SMMU driver > > to defer the probe until the memory controller has been registered. > > I'm doubtful of how generic an argument that really is - does anyone other > than Tegra actually do this? (Most things I know of with programmable Stream > IDs at least have the good grace to configure them in the bootloader or the > devices' own drivers) I'm not aware of any of the bootloaders doing anything with the SMMU, so adding only the stream ID programming seems a little useless. Since it's only at the kernel level that the SMMU will end up being used, it seems natural to define the stream ID mapping there as well. With regards to the devices' own drivers, they get probed way too late for this to take any effect. If the DMA API is backed by an IOMMU, the stream ID mappings will be required long before drivers actually take control of their devices. > If the underlying aim is just "make SMMUs on Tegras wait for an extra > thing", I'd suggest simply wiring up the existing tegra_mc APIs in your > arm-smmu-nvidia.c hooks. (hmm, what did happen to those patches?) Passing around global symbols seems like a bit of a hack, whereas encoding this in device tree actually gives us a way of properly describing this relationship. That said, I could look at moving the memory controller lookup code into the Tegra-specific ARM SMMU implementation if it's not something that's more broadly useful. The NVIDIA implementation is currently blocked on two things. On one hand we can't enable the SMMU before we have this series in place to make sure that it starts up with the correct stream ID mapping. The other blocker currently is that memory clients can access 40 bits of addresses, but bit 39 has special meaning, so there's a bit more glue that we need in device tree (via the DMA parent) to properly describe the DMA ranges for these devices. Otherwise the IOMMU will hand out IOVAs with bit 39 set (DMA API allocates from the top) and that causes memory accesses to be jumbled in undesirable ways. If I move the memory lookup code into the NVIDIA ARM SMMU implementation it would probably easiest to integrate all of it into a single series. Thierry > > One such example is Tegra186 where the memory controller contains some > > registers that are used to program stream IDs for the various memory > > clients (display, USB, PCI, ...) in the system. Programming these SIDs > > is required for the memory clients to emit the proper SIDs as part of > > their memory requests. The memory controller driver therefore needs to > > be programmed prior to the SMMU driver. To achieve that, the memory > > controller will be referenced via phandle from the SMMU device tree > > node, the SMMU driver can then use the memory controller framework to > > find it and defer probe until it has been registered. > > > > Signed-off-by: Thierry Reding > > --- > > Changes in v3: > > - add device-managed variants of the consumer APIs > > - add kerneldoc > > > > Changes in v2: > > - fix double unlock (Dan Carpenter, kbuild test robot) > > - add helper to get optional memory controllers > > - acquire and release module reference > > > > drivers/memory/Makefile | 1 + > > drivers/memory/core.c | 248 ++ > > include/linux/memory-controller.h | 34 > > 3 files changed, 283 insertions(+) > > create mode 100644 drivers/memory/core.c > > create mode 100644 include/linux/memory-controller.h > > > > diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile > > index 27b493435e61..d16e7dca8ef9 100644 > > --- a/drivers/memory/Makefile > > +++ b/drivers/memory/Makefile > > @@ -3,6 +3,7 @@ > > # Makefile for memory devices > > # > > +obj-y += core.o > > obj-$(CONFIG_DDR) += jedec_ddr_data.o > > ifeq ($(CONFIG_DDR),y) > > obj-$(CONFIG_OF) += of_memory.o > > diff --git a/drivers/memory/core.c b/drivers/memory/core.c > > new file mode 100644 > > index ..b2fbd2e808de > > --- /dev/null > > +++ b/drivers/memory/core.c > > @@ -0,0 +1,248 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2019-2020 NVIDIA Corporation. > > + */ > > + > > +#include > > +#include > > +#include > > + > > +static DEFINE_MUTEX(controllers_lock); > > +static LIST_HEAD(controllers); > > + > > +static void memory_controller_release(struct kref *ref) > > +{ > > + struct memory_controller *mc = container_of(ref, struct > > memory_controller, ref);
Re: [PATCH v4 0/5] memory: Introduce memory controller mini-framework
[+ Maxime] On 13/02/2020 4:39 pm, Thierry Reding wrote: From: Thierry Reding Hi, this set of patches adds a new binding that allows device tree nodes to explicitly define the DMA parent for a given device. This supplements the existing interconnect bindings and is useful to disambiguate in the case where a device has multiple paths to system memory. Beyond that it can also be useful when there aren't any actual interconnect paths that can be controlled, so in simple cases this can serve as a simpler variant of interconnect paths. Isn't that still squarely the intent of the "dma-mem" binding, though? i.e. it's not meant to be a 'real' interconnect provider, but a very simple way to encode DMA parentage piggybacked onto a more general binding (with the *option* of being a full-blown interconnect if it wants to, but certainly no expectation). Robin. One other case where this is useful is to describe the relationship between devices such as the memory controller and an IOMMU, for example. On Tegra186 and later, the memory controller is programmed with a set of stream IDs that are to be associated with each memory client. This programming needs to happen before translations through the IOMMU start, otherwise the used stream IDs may deviate from the expected values. The memory-controllers property is used in this case to ensure that the memory controller driver has been probed (and hence has programmed the stream ID mappings) before the IOMMU becomes available. Patch 1 introduces the memory controller bindings, both from the perspective of the provider and the consumer. Patch 2 makes use of a memory-controllers property to determine the DMA parent for the purpose of setting up DMA masks (based on the dma-ranges property of the DMA parent). Patch 3 introduces a minimalistic framework that is used to register memory controllers with along with a set of helpers to look up the memory controller from device tree. An example of how to register a memory controller is shown in patch 4 for Tegra186 (and later) and finally the ARM SMMU driver is extended to become a consumer of an (optional) memory controller. As described above, the goal is to defer probe as long as the memory controller has not yet programmed the stream ID mappings. Thierry Thierry Reding (5): dt-bindings: Add memory controller bindings of: Use memory-controllers property for DMA parent memory: Introduce memory controller mini-framework memory: tegra186: Register as memory controller iommu: arm-smmu: Get reference to memory controller .../bindings/memory-controllers/consumer.yaml | 14 + .../memory-controllers/memory-controller.yaml | 32 +++ drivers/iommu/arm-smmu.c | 11 + drivers/iommu/arm-smmu.h | 2 + drivers/memory/Makefile | 1 + drivers/memory/core.c | 248 ++ drivers/memory/tegra/tegra186.c | 9 +- drivers/of/address.c | 25 +- include/linux/memory-controller.h | 34 +++ 9 files changed, 366 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/memory-controllers/consumer.yaml create mode 100644 Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml create mode 100644 drivers/memory/core.c create mode 100644 include/linux/memory-controller.h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/5] memory: Introduce memory controller mini-framework
On 13/02/2020 4:39 pm, Thierry Reding wrote: From: Thierry Reding This new framework is currently nothing more than a registry of memory controllers, with the goal being to order device probing. One use-case where this is useful, for example, is a memory controller device which needs to program some registers before the system MMU can be enabled. Associating the memory controller with the SMMU allows the SMMU driver to defer the probe until the memory controller has been registered. I'm doubtful of how generic an argument that really is - does anyone other than Tegra actually do this? (Most things I know of with programmable Stream IDs at least have the good grace to configure them in the bootloader or the devices' own drivers) If the underlying aim is just "make SMMUs on Tegras wait for an extra thing", I'd suggest simply wiring up the existing tegra_mc APIs in your arm-smmu-nvidia.c hooks. (hmm, what did happen to those patches?) Robin. One such example is Tegra186 where the memory controller contains some registers that are used to program stream IDs for the various memory clients (display, USB, PCI, ...) in the system. Programming these SIDs is required for the memory clients to emit the proper SIDs as part of their memory requests. The memory controller driver therefore needs to be programmed prior to the SMMU driver. To achieve that, the memory controller will be referenced via phandle from the SMMU device tree node, the SMMU driver can then use the memory controller framework to find it and defer probe until it has been registered. Signed-off-by: Thierry Reding --- Changes in v3: - add device-managed variants of the consumer APIs - add kerneldoc Changes in v2: - fix double unlock (Dan Carpenter, kbuild test robot) - add helper to get optional memory controllers - acquire and release module reference drivers/memory/Makefile | 1 + drivers/memory/core.c | 248 ++ include/linux/memory-controller.h | 34 3 files changed, 283 insertions(+) create mode 100644 drivers/memory/core.c create mode 100644 include/linux/memory-controller.h diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index 27b493435e61..d16e7dca8ef9 100644 --- a/drivers/memory/Makefile +++ b/drivers/memory/Makefile @@ -3,6 +3,7 @@ # Makefile for memory devices # +obj-y+= core.o obj-$(CONFIG_DDR) += jedec_ddr_data.o ifeq ($(CONFIG_DDR),y) obj-$(CONFIG_OF) += of_memory.o diff --git a/drivers/memory/core.c b/drivers/memory/core.c new file mode 100644 index ..b2fbd2e808de --- /dev/null +++ b/drivers/memory/core.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019-2020 NVIDIA Corporation. + */ + +#include +#include +#include + +static DEFINE_MUTEX(controllers_lock); +static LIST_HEAD(controllers); + +static void memory_controller_release(struct kref *ref) +{ + struct memory_controller *mc = container_of(ref, struct memory_controller, ref); + + WARN_ON(!list_empty(>list)); +} + +/** + * memory_controller_register() - register a memory controller + * @mc: memory controller + */ +int memory_controller_register(struct memory_controller *mc) +{ + kref_init(>ref); + + mutex_lock(_lock); + list_add_tail(>list, ); + mutex_unlock(_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(memory_controller_register); + +/** + * memory_controller_unregister() - unregister a memory controller + * @mc: memory controller + */ +void memory_controller_unregister(struct memory_controller *mc) +{ + mutex_lock(_lock); + list_del_init(>list); + mutex_unlock(_lock); + + kref_put(>ref, memory_controller_release); +} +EXPORT_SYMBOL_GPL(memory_controller_unregister); + +static struct memory_controller * +of_memory_controller_get(struct device *dev, struct device_node *np, +const char *con_id) +{ + const char *cells = "#memory-controller-cells"; + const char *names = "memory-controller-names"; + const char *prop = "memory-controllers"; + struct memory_controller *mc; + struct of_phandle_args args; + int index = 0, err; + + if (con_id) { + index = of_property_match_string(np, names, con_id); + if (index < 0) + return ERR_PTR(index); + } + + err = of_parse_phandle_with_args(np, prop, cells, index, ); + if (err) { + if (err == -ENOENT) + err = -ENODEV; + + return ERR_PTR(err); + } + + mutex_lock(_lock); + + list_for_each_entry(mc, , list) { + if (mc->dev && mc->dev->of_node == args.np) { + __module_get(mc->dev->driver->owner); + kref_get(>ref); + goto unlock; + } + } + + mc = ERR_PTR(-EPROBE_DEFER); + +unlock: +
[PATCH 02/11] PCI: Add ats_supported host bridge flag
Each vendor has their own way of describing whether a host bridge supports ATS. The Intel and AMD ACPI tables selectively enable or disable ATS per device or sub-tree, while Arm has a single bit for each host bridge. For those that need it, add an ats_supported bit to the host bridge structure. Signed-off-by: Jean-Philippe Brucker --- drivers/pci/probe.c | 7 +++ include/linux/pci.h | 1 + 2 files changed, 8 insertions(+) diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 512cb4312ddd..75c0a25af44e 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -598,6 +598,13 @@ static void pci_init_host_bridge(struct pci_host_bridge *bridge) bridge->native_shpc_hotplug = 1; bridge->native_pme = 1; bridge->native_ltr = 1; + + /* +* Some systems may disable ATS at the host bridge (ACPI IORT, +* device-tree), other filter it with a smaller granularity (ACPI DMAR +* and IVRS). +*/ + bridge->ats_supported = 1; } struct pci_host_bridge *pci_alloc_host_bridge(size_t priv) diff --git a/include/linux/pci.h b/include/linux/pci.h index 3840a541a9de..9fe2e84d74d7 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -511,6 +511,7 @@ struct pci_host_bridge { unsigned intnative_pme:1; /* OS may use PCIe PME */ unsigned intnative_ltr:1; /* OS may use PCIe LTR */ unsigned intpreserve_config:1; /* Preserve FW resource setup */ + unsigned intats_supported:1; /* Resource alignment requirements */ resource_size_t (*align_resource)(struct pci_dev *dev, -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 04/11] ACPI/IORT: Check ATS capability in root complex node
When initializing a PCI root bridge, copy its "ATS supported" attribute into the root bridge. Signed-off-by: Jean-Philippe Brucker --- drivers/acpi/arm64/iort.c | 27 +++ drivers/acpi/pci_root.c | 3 +++ include/linux/acpi_iort.h | 8 3 files changed, 38 insertions(+) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index ed3d2d1a7ae9..d99d7f5b51e1 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -1633,6 +1633,33 @@ static void __init iort_enable_acs(struct acpi_iort_node *iort_node) } } } + +static acpi_status iort_match_host_bridge_callback(struct acpi_iort_node *node, + void *context) +{ + struct acpi_iort_root_complex *pci_rc; + struct pci_host_bridge *host_bridge = context; + + pci_rc = (struct acpi_iort_root_complex *)node->node_data; + + return pci_domain_nr(host_bridge->bus) == pci_rc->pci_segment_number ? + AE_OK : AE_NOT_FOUND; +} + +void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge) +{ + struct acpi_iort_node *node; + struct acpi_iort_root_complex *pci_rc; + + node = iort_scan_node(ACPI_IORT_NODE_PCI_ROOT_COMPLEX, + iort_match_host_bridge_callback, host_bridge); + if (!node) + return; + + pci_rc = (struct acpi_iort_root_complex *)node->node_data; + host_bridge->ats_supported = !!(pci_rc->ats_attribute & + ACPI_IORT_ATS_SUPPORTED); +} #else static inline void iort_enable_acs(struct acpi_iort_node *iort_node) { } #endif diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c index d1e666ef3fcc..eb2fb8f17c0b 100644 --- a/drivers/acpi/pci_root.c +++ b/drivers/acpi/pci_root.c @@ -6,6 +6,7 @@ * Copyright (C) 2001, 2002 Paul Diefenbaugh */ +#include #include #include #include @@ -917,6 +918,8 @@ struct pci_bus *acpi_pci_root_create(struct acpi_pci_root *root, if (!(root->osc_control_set & OSC_PCI_EXPRESS_LTR_CONTROL)) host_bridge->native_ltr = 0; + iort_pci_host_bridge_setup(host_bridge); + /* * Evaluate the "PCI Boot Configuration" _DSM Function. If it * exists and returns 0, we must preserve any PCI resource diff --git a/include/linux/acpi_iort.h b/include/linux/acpi_iort.h index 8e7e2ec37f1b..7b06871cc3aa 100644 --- a/include/linux/acpi_iort.h +++ b/include/linux/acpi_iort.h @@ -10,6 +10,7 @@ #include #include #include +#include #define IORT_IRQ_MASK(irq) (irq & 0xULL) #define IORT_IRQ_TRIGGER_MASK(irq) ((irq >> 32) & 0xULL) @@ -55,4 +56,11 @@ int iort_iommu_msi_get_resv_regions(struct device *dev, struct list_head *head) { return 0; } #endif +#if defined(CONFIG_ACPI_IORT) && defined(CONFIG_PCI) +void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge); +#else +static inline +void iort_pci_host_bridge_setup(struct pci_host_bridge *host_bridge) { } +#endif + #endif /* __ACPI_IORT_H__ */ -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 09/11] ACPI/IORT: Drop ATS fwspec flag
Now that the ats_supported flag is in the host bridge structure where it belongs, we can remove it from the per-device fwspec structure. Signed-off-by: Jean-Philippe Brucker --- drivers/acpi/arm64/iort.c | 11 --- include/linux/iommu.h | 4 2 files changed, 15 deletions(-) diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index d99d7f5b51e1..f634641b3699 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -924,14 +924,6 @@ static int arm_smmu_iort_xlate(struct device *dev, u32 streamid, return ret; } -static bool iort_pci_rc_supports_ats(struct acpi_iort_node *node) -{ - struct acpi_iort_root_complex *pci_rc; - - pci_rc = (struct acpi_iort_root_complex *)node->node_data; - return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED; -} - static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node, u32 streamid) { @@ -1026,9 +1018,6 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) info.node = node; err = pci_for_each_dma_alias(to_pci_dev(dev), iort_pci_iommu_init, ); - - if (!err && iort_pci_rc_supports_ats(node)) - dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS; } else { int i = 0; diff --git a/include/linux/iommu.h b/include/linux/iommu.h index d1b5f4d98569..1739f8a7a4b4 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -589,15 +589,11 @@ struct iommu_fwspec { const struct iommu_ops *ops; struct fwnode_handle*iommu_fwnode; void*iommu_priv; - u32 flags; u32 num_pasid_bits; unsigned intnum_ids; u32 ids[1]; }; -/* ATS is supported */ -#define IOMMU_FWSPEC_PCI_RC_ATS(1 << 0) - /** * struct iommu_sva - handle to a device-mm bond */ -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 08/11] iommu/vt-d: Use pci_ats_supported()
The pci_ats_supported() function checks if a device supports ATS and is allowed to use it. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/intel-iommu.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 9dc37672bf89..668f1b99111b 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -1449,8 +1449,7 @@ static void iommu_enable_dev_iotlb(struct device_domain_info *info) !pci_reset_pri(pdev) && !pci_enable_pri(pdev, 32)) info->pri_enabled = 1; #endif - if (!pdev->untrusted && info->ats_supported && - pci_ats_page_aligned(pdev) && + if (info->ats_supported && pci_ats_page_aligned(pdev) && !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) { info->ats_enabled = 1; domain_update_iotlb(info->domain); @@ -2611,10 +2610,8 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu, if (dev && dev_is_pci(dev)) { struct pci_dev *pdev = to_pci_dev(info->dev); - if (!pdev->untrusted && - !pci_ats_disabled() && - ecap_dev_iotlb_support(iommu->ecap) && - pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS) && + if (ecap_dev_iotlb_support(iommu->ecap) && + pci_ats_supported(pdev) && dmar_find_matched_atsr_unit(pdev)) info->ats_supported = 1; -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/11] dt-bindings: PCI: generic: Add ats-supported property
Add a way for firmware to tell the OS that ATS is supported by the PCI root complex. An endpoint with ATS enabled may send Translation Requests and Translated Memory Requests, which look just like Normal Memory Requests with a non-zero AT field. So a root controller that ignores the AT field may simply forward the request to the IOMMU as a Normal Memory Request, which could end badly. In any case, the endpoint will be unusable. The ats-supported property allows the OS to only enable ATS in endpoints if the root controller can handle ATS requests. Only add the property to pcie-host-ecam-generic for the moment. For non-generic root controllers, availability of ATS can be inferred from the compatible string. Signed-off-by: Jean-Philippe Brucker --- Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 6 ++ 1 file changed, 6 insertions(+) diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml index 47353d0cd394..7d40edd7f1ef 100644 --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml @@ -107,6 +107,12 @@ properties: dma-coherent: true + ats-supported: +description: + Indicates that a PCIe host controller supports ATS, and can handle Memory + Requests with Address Type (AT). +type: boolean + required: - compatible - reg -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 06/11] iommu/amd: Use pci_ats_supported()
The pci_ats_supported() function checks if a device supports ATS and is allowed to use it. In addition to checking that the device has an ATS capability and that the global pci=noats is not set (pci_ats_disabled()), it also checks if a device is untrusted (plugged into an external-facing port such as thunderbolt). Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/amd_iommu.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index aac132bd1ef0..084f0b2e132e 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -291,16 +291,15 @@ static struct iommu_group *acpihid_device_group(struct device *dev) static bool pci_iommuv2_capable(struct pci_dev *pdev) { static const int caps[] = { - PCI_EXT_CAP_ID_ATS, PCI_EXT_CAP_ID_PRI, PCI_EXT_CAP_ID_PASID, }; int i, pos; - if (pci_ats_disabled()) + if (!pci_ats_supported(pdev)) return false; - for (i = 0; i < 3; ++i) { + for (i = 0; i < 2; ++i) { pos = pci_find_ext_capability(pdev, caps[i]); if (pos == 0) return false; @@ -3040,11 +3039,8 @@ int amd_iommu_device_info(struct pci_dev *pdev, memset(info, 0, sizeof(*info)); - if (!pci_ats_disabled()) { - pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS); - if (pos) - info->flags |= AMD_IOMMU_DEVICE_FLAG_ATS_SUP; - } + if (pci_ats_supported(pdev)) + info->flags |= AMD_IOMMU_DEVICE_FLAG_ATS_SUP; pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_PRI); if (pos) -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 03/11] PCI: OF: Check whether the host bridge supports ATS
Copy the ats-supported flag into the pci_host_bridge structure. Signed-off-by: Jean-Philippe Brucker --- drivers/pci/controller/pci-host-common.c | 1 + drivers/pci/of.c | 9 + include/linux/of_pci.h | 3 +++ 3 files changed, 13 insertions(+) diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c index 250a3fc80ec6..a6ac927be291 100644 --- a/drivers/pci/controller/pci-host-common.c +++ b/drivers/pci/controller/pci-host-common.c @@ -92,6 +92,7 @@ int pci_host_common_probe(struct platform_device *pdev, return ret; } + of_pci_host_check_ats(bridge); platform_set_drvdata(pdev, bridge->bus); return 0; } diff --git a/drivers/pci/of.c b/drivers/pci/of.c index 81ceeaa6f1d5..4b8a877f1e9f 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -576,6 +576,15 @@ int pci_parse_request_of_pci_ranges(struct device *dev, } EXPORT_SYMBOL_GPL(pci_parse_request_of_pci_ranges); +void of_pci_host_check_ats(struct pci_host_bridge *bridge) +{ + struct device_node *np = bridge->bus->dev.of_node; + + if (!np) + return; + + bridge->ats_supported = of_property_read_bool(np, "ats-supported"); +} #endif /* CONFIG_PCI */ /** diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h index 29658c0ee71f..2d0af410438c 100644 --- a/include/linux/of_pci.h +++ b/include/linux/of_pci.h @@ -7,12 +7,14 @@ struct pci_dev; struct device_node; +struct pci_host_bridge; #if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_PCI) struct device_node *of_pci_find_child_device(struct device_node *parent, unsigned int devfn); int of_pci_get_devfn(struct device_node *np); void of_pci_check_probe_only(void); +void of_pci_host_check_ats(struct pci_host_bridge *bridge); #else static inline struct device_node *of_pci_find_child_device(struct device_node *parent, unsigned int devfn) @@ -26,6 +28,7 @@ static inline int of_pci_get_devfn(struct device_node *np) } static inline void of_pci_check_probe_only(void) { } +static inline void of_pci_host_check_ats(struct pci_host_bridge *bridge) { } #endif #if IS_ENABLED(CONFIG_OF_IRQ) -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 05/11] PCI/ATS: Gather checks into pci_ats_supported()
IOMMU drivers need to perform several tests when checking if a device supports ATS. Move them all into a new function that returns true when a device and its host bridge support ATS. Since pci_enable_ats() now calls pci_ats_supported(), the following new checks are now common: * whether a device is trusted. Devices plugged into external-facing ports such as thunderbolt are untrusted. * whether the host bridge supports ATS, which defaults to true unless the firmware description states that ATS isn't supported by the host bridge. Signed-off-by: Jean-Philippe Brucker --- drivers/pci/ats.c | 30 +- include/linux/pci-ats.h | 3 +++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 390e92f2d8d1..bbfd0d42b8b9 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -30,6 +30,34 @@ void pci_ats_init(struct pci_dev *dev) dev->ats_cap = pos; } +/** + * pci_ats_supported - check if the device can use ATS + * @dev: the PCI device + * + * Returns true if the device supports ATS and is allowed to use it, false + * otherwise. + */ +bool pci_ats_supported(struct pci_dev *dev) +{ + struct pci_host_bridge *bridge; + + if (!dev->ats_cap) + return false; + + if (dev->untrusted) + return false; + + bridge = pci_find_host_bridge(dev->bus); + if (!bridge) + return false; + + if (!bridge->ats_supported) + return false; + + return true; +} +EXPORT_SYMBOL_GPL(pci_ats_supported); + /** * pci_enable_ats - enable the ATS capability * @dev: the PCI device @@ -42,7 +70,7 @@ int pci_enable_ats(struct pci_dev *dev, int ps) u16 ctrl; struct pci_dev *pdev; - if (!dev->ats_cap) + if (!pci_ats_supported(dev)) return -EINVAL; if (WARN_ON(dev->ats_enabled)) diff --git a/include/linux/pci-ats.h b/include/linux/pci-ats.h index d08f0869f121..f75c307f346d 100644 --- a/include/linux/pci-ats.h +++ b/include/linux/pci-ats.h @@ -6,11 +6,14 @@ #ifdef CONFIG_PCI_ATS /* Address Translation Service */ +bool pci_ats_supported(struct pci_dev *dev); int pci_enable_ats(struct pci_dev *dev, int ps); void pci_disable_ats(struct pci_dev *dev); int pci_ats_queue_depth(struct pci_dev *dev); int pci_ats_page_aligned(struct pci_dev *dev); #else /* CONFIG_PCI_ATS */ +static inline bool pci_ats_supported(struct pci_dev *d) +{ return false; } static inline int pci_enable_ats(struct pci_dev *d, int ps) { return -ENODEV; } static inline void pci_disable_ats(struct pci_dev *d) { } -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 07/11] iommu/arm-smmu-v3: Use pci_ats_supported()
The new pci_ats_supported() function checks if a device supports ATS and is allowed to use it. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/arm-smmu-v3.c | 18 +++--- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 034ad9671b83..bd2cfd946a36 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2577,26 +2577,14 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master) } } -#ifdef CONFIG_PCI_ATS static bool arm_smmu_ats_supported(struct arm_smmu_master *master) { - struct pci_dev *pdev; + struct device *dev = master->dev; struct arm_smmu_device *smmu = master->smmu; - struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev); - - if (!(smmu->features & ARM_SMMU_FEAT_ATS) || !dev_is_pci(master->dev) || - !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS) || pci_ats_disabled()) - return false; - pdev = to_pci_dev(master->dev); - return !pdev->untrusted && pdev->ats_cap; + return (smmu->features & ARM_SMMU_FEAT_ATS) && dev_is_pci(dev) && + pci_ats_supported(to_pci_dev(dev)); } -#else -static bool arm_smmu_ats_supported(struct arm_smmu_master *master) -{ - return false; -} -#endif static void arm_smmu_enable_ats(struct arm_smmu_master *master) { -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 11/11] Documentation: Generalize the "pci=noats" boot parameter
The "pci=noats" kernel parameter disables PCIe ATS globally, and affects any ATS-capable IOMMU driver. So rather than adding Arm SMMUv3, which recently gained ATS support, to the list of relevant build options, simplify the noats description. Signed-off-by: Jean-Philippe Brucker --- Documentation/admin-guide/kernel-parameters.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index dbc22d684627..e5fa8d057a3c 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3606,8 +3606,8 @@ on: Turn realloc on realloc same as realloc=on noari do not use PCIe ARI. - noats [PCIE, Intel-IOMMU, AMD-IOMMU] - do not use PCIe ATS (and IOMMU device IOTLB). + noats [PCIE] Do not use PCIe ATS (and IOMMU device + IOTLB). pcie_scan_all Scan all possible PCIe devices. Otherwise we only look for one device below a PCIe downstream port. -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 10/11] arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP
Declare that the host controller supports ATS, so the OS can enable it for ATS-capable PCIe endpoints. Signed-off-by: Jean-Philippe Brucker --- All endpoints support ATS provided they have the ats_supported=1 model parameter. "lspci -vv" shows whether ATS is supported and enabled. --- arch/arm64/boot/dts/arm/fvp-base-revc.dts | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm64/boot/dts/arm/fvp-base-revc.dts b/arch/arm64/boot/dts/arm/fvp-base-revc.dts index 62ab0d54ff71..6e5bb7bcb4b3 100644 --- a/arch/arm64/boot/dts/arm/fvp-base-revc.dts +++ b/arch/arm64/boot/dts/arm/fvp-base-revc.dts @@ -170,6 +170,7 @@ pci: pci@4000 { iommu-map = <0x0 0x0 0x1>; dma-coherent; + ats-supported; }; smmu: smmu@2b40 { -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 00/10] PCI/ATS: Device-tree support and other improvements
Enable ATS on device-tree based systems, and factor the common ATS enablement checks into pci_enable_ats(). ATS support in PCIe endpoints is discovered through the ATS capability, but there is no common method for discovering whether the host bridge supports ATS. Each vendor provides their own ACPI method: * DMAR (Intel) reports ATS support per domain or per root port. * IVRS (AMD) reports negative ATS support for a range of devices. * IORT (ARM) reports ATS support for a root complex. In my opinion it's important that we only enable ATS if the host bridge supports it, but I don't think a finer granularity is useful. If the host bridge ignores the Address Translation field of TLP headers, it could in theory treat a Translation Request as a Memory Read Request, and a Translated Request as a normal memory access, which could result in silent memory corruption. Patches 1-3 add a device-tree property that declares ATS support in host controllers. We only add it to the generic host, but the property can easily be reused by other PCI hosts. Alternatively, the host drivers can directly set the new ats_supported property of the host bridge introduced in patch 1. Patch 4 uses the new ats_supported host bridge property for IORT. Patch 9 removes the old method that set a flag in each endpoint's fwspec. Patches 5-8 put all checks required for enabling ATS in common, along with the new host bridge check. Jean-Philippe Brucker (11): dt-bindings: PCI: generic: Add ats-supported property PCI: Add ats_supported host bridge flag PCI: OF: Check whether the host bridge supports ATS ACPI/IORT: Check ATS capability in root complex node PCI/ATS: Gather checks into pci_ats_supported() iommu/amd: Use pci_ats_supported() iommu/arm-smmu-v3: Use pci_ats_supported() iommu/vt-d: Use pci_ats_supported() ACPI/IORT: Drop ATS fwspec flag arm64: dts: fast models: Enable PCIe ATS for Base RevC FVP Documentation: Generalize the "pci=noats" boot parameter .../admin-guide/kernel-parameters.txt | 4 +- .../bindings/pci/host-generic-pci.yaml| 6 +++ arch/arm64/boot/dts/arm/fvp-base-revc.dts | 1 + drivers/acpi/arm64/iort.c | 38 +-- drivers/acpi/pci_root.c | 3 ++ drivers/iommu/amd_iommu.c | 12 ++ drivers/iommu/arm-smmu-v3.c | 18 ++--- drivers/iommu/intel-iommu.c | 9 ++--- drivers/pci/ats.c | 30 ++- drivers/pci/controller/pci-host-common.c | 1 + drivers/pci/of.c | 9 + drivers/pci/probe.c | 7 include/linux/acpi_iort.h | 8 include/linux/iommu.h | 4 -- include/linux/of_pci.h| 3 ++ include/linux/pci-ats.h | 3 ++ include/linux/pci.h | 1 + 17 files changed, 110 insertions(+), 47 deletions(-) -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 5/5] iommu: arm-smmu: Get reference to memory controller
From: Thierry Reding Use the memory controller framework to obtain a reference to the memory controller to which the SMMU will make memory requests. This allows the two drivers to properly order their probes so that the memory controller can be programmed first. An example where this is required is Tegra186 where the stream IDs need to be associated with memory clients before memory requests are emitted with the correct stream ID. Signed-off-by: Thierry Reding --- drivers/iommu/arm-smmu.c | 11 +++ drivers/iommu/arm-smmu.h | 2 ++ 2 files changed, 13 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 16c4b87af42b..862ea55018e8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -2109,6 +2109,17 @@ static int arm_smmu_device_probe(struct platform_device *pdev) } smmu->dev = dev; + smmu->mc = devm_memory_controller_get_optional(dev, NULL); + if (IS_ERR(smmu->mc)) { + err = PTR_ERR(smmu->mc); + + if (err != -EPROBE_DEFER) + dev_err(dev, "failed to get memory controller: %d\n", + err); + + return err; + } + if (dev->of_node) err = arm_smmu_device_dt_probe(pdev, smmu); else diff --git a/drivers/iommu/arm-smmu.h b/drivers/iommu/arm-smmu.h index 8d1cd54d82a6..d38bcd3ce447 100644 --- a/drivers/iommu/arm-smmu.h +++ b/drivers/iommu/arm-smmu.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -253,6 +254,7 @@ enum arm_smmu_implementation { struct arm_smmu_device { struct device *dev; + struct memory_controller*mc; void __iomem*base; unsigned intnumpage; -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 4/5] memory: tegra186: Register as memory controller
From: Thierry Reding Registering as memory controller allows other drivers to obtain a reference to it. This is mostly useful as a way of ordering probe between devices depending on one another. Signed-off-by: Thierry Reding --- drivers/memory/tegra/tegra186.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c index 5d53f11ca7b6..8c43702340e8 100644 --- a/drivers/memory/tegra/tegra186.c +++ b/drivers/memory/tegra/tegra186.c @@ -4,6 +4,7 @@ */ #include +#include #include #include #include @@ -32,6 +33,7 @@ struct tegra186_mc_soc { }; struct tegra186_mc { + struct memory_controller base; struct device *dev; void __iomem *regs; @@ -1532,13 +1534,18 @@ static int tegra186_mc_probe(struct platform_device *pdev) return -ENOMEM; mc->soc = of_device_get_match_data(>dev); + mc->dev = >dev; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); mc->regs = devm_ioremap_resource(>dev, res); if (IS_ERR(mc->regs)) return PTR_ERR(mc->regs); - mc->dev = >dev; + mc->base.dev = >dev; + + err = memory_controller_register(>base); + if (err < 0) + return err; err = of_platform_populate(pdev->dev.of_node, NULL, NULL, >dev); if (err < 0) -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 3/5] memory: Introduce memory controller mini-framework
From: Thierry Reding This new framework is currently nothing more than a registry of memory controllers, with the goal being to order device probing. One use-case where this is useful, for example, is a memory controller device which needs to program some registers before the system MMU can be enabled. Associating the memory controller with the SMMU allows the SMMU driver to defer the probe until the memory controller has been registered. One such example is Tegra186 where the memory controller contains some registers that are used to program stream IDs for the various memory clients (display, USB, PCI, ...) in the system. Programming these SIDs is required for the memory clients to emit the proper SIDs as part of their memory requests. The memory controller driver therefore needs to be programmed prior to the SMMU driver. To achieve that, the memory controller will be referenced via phandle from the SMMU device tree node, the SMMU driver can then use the memory controller framework to find it and defer probe until it has been registered. Signed-off-by: Thierry Reding --- Changes in v3: - add device-managed variants of the consumer APIs - add kerneldoc Changes in v2: - fix double unlock (Dan Carpenter, kbuild test robot) - add helper to get optional memory controllers - acquire and release module reference drivers/memory/Makefile | 1 + drivers/memory/core.c | 248 ++ include/linux/memory-controller.h | 34 3 files changed, 283 insertions(+) create mode 100644 drivers/memory/core.c create mode 100644 include/linux/memory-controller.h diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile index 27b493435e61..d16e7dca8ef9 100644 --- a/drivers/memory/Makefile +++ b/drivers/memory/Makefile @@ -3,6 +3,7 @@ # Makefile for memory devices # +obj-y += core.o obj-$(CONFIG_DDR) += jedec_ddr_data.o ifeq ($(CONFIG_DDR),y) obj-$(CONFIG_OF) += of_memory.o diff --git a/drivers/memory/core.c b/drivers/memory/core.c new file mode 100644 index ..b2fbd2e808de --- /dev/null +++ b/drivers/memory/core.c @@ -0,0 +1,248 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019-2020 NVIDIA Corporation. + */ + +#include +#include +#include + +static DEFINE_MUTEX(controllers_lock); +static LIST_HEAD(controllers); + +static void memory_controller_release(struct kref *ref) +{ + struct memory_controller *mc = container_of(ref, struct memory_controller, ref); + + WARN_ON(!list_empty(>list)); +} + +/** + * memory_controller_register() - register a memory controller + * @mc: memory controller + */ +int memory_controller_register(struct memory_controller *mc) +{ + kref_init(>ref); + + mutex_lock(_lock); + list_add_tail(>list, ); + mutex_unlock(_lock); + + return 0; +} +EXPORT_SYMBOL_GPL(memory_controller_register); + +/** + * memory_controller_unregister() - unregister a memory controller + * @mc: memory controller + */ +void memory_controller_unregister(struct memory_controller *mc) +{ + mutex_lock(_lock); + list_del_init(>list); + mutex_unlock(_lock); + + kref_put(>ref, memory_controller_release); +} +EXPORT_SYMBOL_GPL(memory_controller_unregister); + +static struct memory_controller * +of_memory_controller_get(struct device *dev, struct device_node *np, +const char *con_id) +{ + const char *cells = "#memory-controller-cells"; + const char *names = "memory-controller-names"; + const char *prop = "memory-controllers"; + struct memory_controller *mc; + struct of_phandle_args args; + int index = 0, err; + + if (con_id) { + index = of_property_match_string(np, names, con_id); + if (index < 0) + return ERR_PTR(index); + } + + err = of_parse_phandle_with_args(np, prop, cells, index, ); + if (err) { + if (err == -ENOENT) + err = -ENODEV; + + return ERR_PTR(err); + } + + mutex_lock(_lock); + + list_for_each_entry(mc, , list) { + if (mc->dev && mc->dev->of_node == args.np) { + __module_get(mc->dev->driver->owner); + kref_get(>ref); + goto unlock; + } + } + + mc = ERR_PTR(-EPROBE_DEFER); + +unlock: + mutex_unlock(_lock); + of_node_put(args.np); + return mc; +} + +/** + * memory_controller_get() - obtain a reference to a memory controller + * @dev: consumer device + * @con_id: consumer name + * + * Returns: A pointer to the requested memory controller or an ERR_PTR()- + * encoded error code on failure. + */ +struct memory_controller * +memory_controller_get(struct device *dev, const char *con_id) +{ + if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node) + return
[PATCH v4 2/5] of: Use memory-controllers property for DMA parent
From: Thierry Reding Prefer the memory-controllers property to determine the DMA parent of a device over the interconnects property, which can be ambiguous since it can be used to describe multiple paths to system memory. Signed-off-by: Thierry Reding --- drivers/of/address.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/drivers/of/address.c b/drivers/of/address.c index e8a39c3ec4d4..ae841bd36bb0 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -697,17 +697,24 @@ static struct device_node *__of_get_dma_parent(const struct device_node *np) struct of_phandle_args args; int ret, index; - index = of_property_match_string(np, "interconnect-names", "dma-mem"); - if (index < 0) - return of_get_parent(np); + ret = of_parse_phandle_with_args(np, "memory-controllers", +"#memory-controller-cells", +0, ); + if (!ret) { + return of_node_get(args.np); + } - ret = of_parse_phandle_with_args(np, "interconnects", -"#interconnect-cells", -index, ); - if (ret < 0) - return of_get_parent(np); + index = of_property_match_string(np, "interconnect-names", "dma-mem"); + if (index >= 0) { + ret = of_parse_phandle_with_args(np, "interconnects", +"#interconnect-cells", +index, ); + if (!ret) { + return of_node_get(args.np); + } + } - return of_node_get(args.np); + return of_get_parent(np); } static struct device_node *of_get_next_dma_parent(struct device_node *np) -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 0/5] memory: Introduce memory controller mini-framework
From: Thierry Reding Hi, this set of patches adds a new binding that allows device tree nodes to explicitly define the DMA parent for a given device. This supplements the existing interconnect bindings and is useful to disambiguate in the case where a device has multiple paths to system memory. Beyond that it can also be useful when there aren't any actual interconnect paths that can be controlled, so in simple cases this can serve as a simpler variant of interconnect paths. One other case where this is useful is to describe the relationship between devices such as the memory controller and an IOMMU, for example. On Tegra186 and later, the memory controller is programmed with a set of stream IDs that are to be associated with each memory client. This programming needs to happen before translations through the IOMMU start, otherwise the used stream IDs may deviate from the expected values. The memory-controllers property is used in this case to ensure that the memory controller driver has been probed (and hence has programmed the stream ID mappings) before the IOMMU becomes available. Patch 1 introduces the memory controller bindings, both from the perspective of the provider and the consumer. Patch 2 makes use of a memory-controllers property to determine the DMA parent for the purpose of setting up DMA masks (based on the dma-ranges property of the DMA parent). Patch 3 introduces a minimalistic framework that is used to register memory controllers with along with a set of helpers to look up the memory controller from device tree. An example of how to register a memory controller is shown in patch 4 for Tegra186 (and later) and finally the ARM SMMU driver is extended to become a consumer of an (optional) memory controller. As described above, the goal is to defer probe as long as the memory controller has not yet programmed the stream ID mappings. Thierry Thierry Reding (5): dt-bindings: Add memory controller bindings of: Use memory-controllers property for DMA parent memory: Introduce memory controller mini-framework memory: tegra186: Register as memory controller iommu: arm-smmu: Get reference to memory controller .../bindings/memory-controllers/consumer.yaml | 14 + .../memory-controllers/memory-controller.yaml | 32 +++ drivers/iommu/arm-smmu.c | 11 + drivers/iommu/arm-smmu.h | 2 + drivers/memory/Makefile | 1 + drivers/memory/core.c | 248 ++ drivers/memory/tegra/tegra186.c | 9 +- drivers/of/address.c | 25 +- include/linux/memory-controller.h | 34 +++ 9 files changed, 366 insertions(+), 10 deletions(-) create mode 100644 Documentation/devicetree/bindings/memory-controllers/consumer.yaml create mode 100644 Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml create mode 100644 drivers/memory/core.c create mode 100644 include/linux/memory-controller.h -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v4 1/5] dt-bindings: Add memory controller bindings
From: Thierry Reding Add the DT schema for memory controller and consumer bindings. Signed-off-by: Thierry Reding --- .../bindings/memory-controllers/consumer.yaml | 14 .../memory-controllers/memory-controller.yaml | 32 +++ 2 files changed, 46 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/consumer.yaml create mode 100644 Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml diff --git a/Documentation/devicetree/bindings/memory-controllers/consumer.yaml b/Documentation/devicetree/bindings/memory-controllers/consumer.yaml new file mode 100644 index ..7b71a6110c51 --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/consumer.yaml @@ -0,0 +1,14 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/memory-controllers/consumer.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common memory controller consumer binding + +maintainers: + - Thierry Reding + +properties: + memory-controller: +$ref: /schemas/types.yaml#/definitions/phandle-array diff --git a/Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml b/Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml new file mode 100644 index ..26257a666c3c --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/memory-controller.yaml @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/memory-controllers/memory-controller.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common memory controller binding + +maintainers: + - Thierry Reding + +description: | + The memory access hierarchy in a modern device can be fairly complicated. + Accesses to system memory typically end up going through a memory controller + that ensures that data is stored. Along the way, these accesses can undergo + classification and be prioritized and/or arbitrated. + + The interconnect bindings (see ../interconnect/interconnect.txt) provides a + way of describing the data paths between devices and system memory. However + these interconnect paths, in order to be most flexible, describe the paths + in a very fine-grained way, so situations can arise where it is no longer + possible to derive a unique memory parent for any given device. + + In order to remove such potential ambiguities, a memory controller can be + specified in device tree. A memory controller specified in this way will be + used as the DMA parent for a given device. The memory controller defines a + memory bus via the "dma-ranges" property, which will in turn be used to set + the range of memory accessible to DMA children of the memory controller. + +properties: + "#memory-controller-cells": true + dma-ranges: true -- 2.24.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: arm64 iommu groups issue
The underlying issue is that, for historical reasons, OF/IORT-based IOMMU drivers have ended up with group allocation being tied to endpoint driver probing via the dma_configure() mechanism (long story short, driver probe is the only thing which can be delayed in order to wait for a specific IOMMU instance to be ready).However, in the meantime, the IOMMU API internals have evolved sufficiently that I think there's a way to really put things right - I have the spark of an idea which I'll try to sketch out ASAP... OK, great. Hi Robin, I was wondering if you have had a chance to consider this problem again? One simple idea could be to introduce a device link between the endpoint device and its parent bridge to ensure that they probe in order, as expected in pci_device_group(): Subject: [PATCH] PCI: Add device link to ensure endpoint device driver probes after bridge It is required to ensure that a device driver for an endpoint will probe after the parent port driver, so add a device link for this. --- diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 512cb4312ddd..4b832ad25b20 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -2383,6 +2383,7 @@ static void pci_set_msi_domain(struct pci_dev *dev) void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) { int ret; + struct device *parent; pci_configure_device(dev); @@ -2420,6 +2421,10 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus) /* Set up MSI IRQ domain */ pci_set_msi_domain(dev); + parent = dev->dev.parent; + if (parent && parent->bus == _bus_type) + device_link_add(>dev, parent, DL_FLAG_AUTOPROBE_CONSUMER); + /* Notifier could use PCI capabilities */ dev->match_driver = false; ret = device_add(>dev); -- This would work, but the problem is that if the port driver fails in probing - and not just for -EPROBE_DEFER - then the child device will never probe. This very thing happens on my dev board. However we could expand the device links API to cover this sort of scenario. As for alternatives, it looks pretty difficult to me to disassociate the group allocation from the dma_configure path. Let me know. Thanks, John ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Use C99 flexible array in fwspec
Although the 1-element array was a typical pre-C99 way to implement variable-length structures, and indeed is a fundamental construct in the APIs of certain other popular platforms, there's no good reason for it here (and in particular the sizeof() trick is far too "clever" for its own good). We can just as easily implement iommu_fwspec's preallocation behaviour using a standard flexible array member, so let's make it look the way most readers would expect. Signed-off-by: Robin Murphy --- Before the Coccinelle police catch up with me... :) Deliberately no fixes tag, since the original code predates struct_size(), and it's really just a cosmetic cleanup that shouldn't be backported anyway. Robin. drivers/iommu/iommu.c | 15 --- include/linux/iommu.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 3e3528436e0b..660eea8d1d2f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -2405,7 +2405,8 @@ int iommu_fwspec_init(struct device *dev, struct fwnode_handle *iommu_fwnode, if (fwspec) return ops == fwspec->ops ? 0 : -EINVAL; - fwspec = kzalloc(sizeof(*fwspec), GFP_KERNEL); + /* Preallocate for the overwhelmingly common case of 1 ID */ + fwspec = kzalloc(struct_size(fwspec, ids, 1), GFP_KERNEL); if (!fwspec) return -ENOMEM; @@ -2432,15 +2433,15 @@ EXPORT_SYMBOL_GPL(iommu_fwspec_free); int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids) { struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); - size_t size; - int i; + int i, new_num; if (!fwspec) return -EINVAL; - size = offsetof(struct iommu_fwspec, ids[fwspec->num_ids + num_ids]); - if (size > sizeof(*fwspec)) { - fwspec = krealloc(fwspec, size, GFP_KERNEL); + new_num = fwspec->num_ids + num_ids; + if (new_num > 1) { + fwspec = krealloc(fwspec, struct_size(fwspec, ids, new_num), + GFP_KERNEL); if (!fwspec) return -ENOMEM; @@ -2450,7 +2451,7 @@ int iommu_fwspec_add_ids(struct device *dev, u32 *ids, int num_ids) for (i = 0; i < num_ids; i++) fwspec->ids[fwspec->num_ids + i] = ids[i]; - fwspec->num_ids += num_ids; + fwspec->num_ids = new_num; return 0; } EXPORT_SYMBOL_GPL(iommu_fwspec_add_ids); diff --git a/include/linux/iommu.h b/include/linux/iommu.h index d1b5f4d98569..4d1ba76c9a64 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -592,7 +592,7 @@ struct iommu_fwspec { u32 flags; u32 num_pasid_bits; unsigned intnum_ids; - u32 ids[1]; + u32 ids[]; }; /* ATS is supported */ -- 2.23.0.dirty ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] uacce: add uacce driver
On Thu, Feb 13, 2020 at 05:15:10PM +0800, Herbert Xu wrote: > On Mon, Feb 10, 2020 at 03:37:11PM -0800, Greg Kroah-Hartman wrote: > > > > Looks much saner now, thanks for all of the work on this: > > > > Reviewed-by: Greg Kroah-Hartman > > > > Or am I supposed to take this in my tree? If so, I can, but I need an > > ack for the crypto parts. > > I can take this series through the crypto tree if that's fine with > you. Please do, thanks! greg k-h ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/4] iommu/arm-smmu-v3: Write level-1 descriptors atomically
Use WRITE_ONCE() to make sure that the SMMU doesn't read incomplete stream table descriptors. Refer to the comment about 64-bit accesses, and add the comment to the equivalent context descriptor code. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/arm-smmu-v3.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 11123fbf22a5..034ad9671b83 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1539,6 +1539,7 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst, u64 val = (l1_desc->l2ptr_dma & CTXDESC_L1_DESC_L2PTR_MASK) | CTXDESC_L1_DESC_V; + /* See comment in arm_smmu_write_ctx_desc() */ WRITE_ONCE(*dst, cpu_to_le64(val)); } @@ -1734,7 +1735,8 @@ arm_smmu_write_strtab_l1_desc(__le64 *dst, struct arm_smmu_strtab_l1_desc *desc) val |= FIELD_PREP(STRTAB_L1_DESC_SPAN, desc->span); val |= desc->l2ptr_dma & STRTAB_L1_DESC_L2PTR_MASK; - *dst = cpu_to_le64(val); + /* See comment in arm_smmu_write_ctx_desc() */ + WRITE_ONCE(*dst, cpu_to_le64(val)); } static void arm_smmu_sync_ste_for_sid(struct arm_smmu_device *smmu, u32 sid) -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/4] iommu: Finish off PASID support for Arm SMMUv3
Support for context descriptor tables was added to the SMMUv3 driver by commit 87f42391f6a5 ("iommu/arm-smmu-v3: Add support for Substream IDs"). The last patch enabling PASID in PCI devices couldn't be included right away since it would have prevented from building SMMUv3 as a module, another feature introduced in Linux v5.6. Export the relevant symbols in patch 1 before using them in patch 2. Patches 3 and 4 address the other remaining comments for the PASID series [1]. [1] https://lore.kernel.org/linux-iommu/20200114154007.GC2579@willie-the-truck/ Jean-Philippe Brucker (4): PCI/ATS: Export symbols of PASID functions iommu/arm-smmu-v3: Add support for PCI PASID iommu/arm-smmu-v3: Batch context descriptor invalidation iommu/arm-smmu-v3: Write level-1 descriptors atomically drivers/iommu/arm-smmu-v3.c | 78 +++-- drivers/pci/ats.c | 4 ++ 2 files changed, 78 insertions(+), 4 deletions(-) -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/4] iommu/arm-smmu-v3: Add support for PCI PASID
Enable PASID for PCI devices that support it. Initialize PASID early in add_device() because it must be enabled before ATS. Tested-by: Zhangfei Gao Reviewed-by: Jonathan Cameron Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/arm-smmu-v3.c | 62 - 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index aa3ac2a03807..6b76df37025e 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2628,6 +2628,53 @@ static void arm_smmu_disable_ats(struct arm_smmu_master *master) atomic_dec(_domain->nr_ats_masters); } +static int arm_smmu_enable_pasid(struct arm_smmu_master *master) +{ + int ret; + int features; + int num_pasids; + struct pci_dev *pdev; + + if (!dev_is_pci(master->dev)) + return -ENODEV; + + pdev = to_pci_dev(master->dev); + + features = pci_pasid_features(pdev); + if (features < 0) + return features; + + num_pasids = pci_max_pasids(pdev); + if (num_pasids <= 0) + return num_pasids; + + ret = pci_enable_pasid(pdev, features); + if (ret) { + dev_err(>dev, "Failed to enable PASID\n"); + return ret; + } + + master->ssid_bits = min_t(u8, ilog2(num_pasids), + master->smmu->ssid_bits); + return 0; +} + +static void arm_smmu_disable_pasid(struct arm_smmu_master *master) +{ + struct pci_dev *pdev; + + if (!dev_is_pci(master->dev)) + return; + + pdev = to_pci_dev(master->dev); + + if (!pdev->pasid_enabled) + return; + + master->ssid_bits = 0; + pci_disable_pasid(pdev); +} + static void arm_smmu_detach_dev(struct arm_smmu_master *master) { unsigned long flags; @@ -2831,13 +2878,23 @@ static int arm_smmu_add_device(struct device *dev) master->ssid_bits = min(smmu->ssid_bits, fwspec->num_pasid_bits); + /* +* Note that PASID must be enabled before, and disabled after ATS: +* PCI Express Base 4.0r1.0 - 10.5.1.3 ATS Control Register +* +* Behavior is undefined if this bit is Set and the value of the PASID +* Enable, Execute Requested Enable, or Privileged Mode Requested bits +* are changed. +*/ + arm_smmu_enable_pasid(master); + if (!(smmu->features & ARM_SMMU_FEAT_2_LVL_CDTAB)) master->ssid_bits = min_t(u8, master->ssid_bits, CTXDESC_LINEAR_CDMAX); ret = iommu_device_link(>iommu, dev); if (ret) - goto err_free_master; + goto err_disable_pasid; group = iommu_group_get_for_dev(dev); if (IS_ERR(group)) { @@ -2850,6 +2907,8 @@ static int arm_smmu_add_device(struct device *dev) err_unlink: iommu_device_unlink(>iommu, dev); +err_disable_pasid: + arm_smmu_disable_pasid(master); err_free_master: kfree(master); fwspec->iommu_priv = NULL; @@ -2870,6 +2929,7 @@ static void arm_smmu_remove_device(struct device *dev) arm_smmu_detach_dev(master); iommu_group_remove_device(dev); iommu_device_unlink(>iommu, dev); + arm_smmu_disable_pasid(master); kfree(master); iommu_fwspec_free(dev); } -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/4] iommu/arm-smmu-v3: Batch context descriptor invalidation
Rather than publishing one command at a time when invalidating a context descriptor, batch the commands for all SIDs in the domain. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/arm-smmu-v3.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 6b76df37025e..11123fbf22a5 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -1487,8 +1487,10 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain, int ssid, bool leaf) { size_t i; + int cmdn = 0; unsigned long flags; struct arm_smmu_master *master; + u64 cmds[CMDQ_BATCH_ENTRIES * CMDQ_ENT_DWORDS]; struct arm_smmu_device *smmu = smmu_domain->smmu; struct arm_smmu_cmdq_ent cmd = { .opcode = CMDQ_OP_CFGI_CD, @@ -1501,13 +1503,19 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain, spin_lock_irqsave(_domain->devices_lock, flags); list_for_each_entry(master, _domain->devices, domain_head) { for (i = 0; i < master->num_sids; i++) { + if (cmdn == CMDQ_BATCH_ENTRIES) { + arm_smmu_cmdq_issue_cmdlist(smmu, cmds, cmdn, false); + cmdn = 0; + } + cmd.cfgi.sid = master->sids[i]; - arm_smmu_cmdq_issue_cmd(smmu, ); + arm_smmu_cmdq_build_cmd([cmdn * CMDQ_ENT_DWORDS], ); + cmdn++; } } spin_unlock_irqrestore(_domain->devices_lock, flags); - arm_smmu_cmdq_issue_sync(smmu); + arm_smmu_cmdq_issue_cmdlist(smmu, cmds, cmdn, true); } static int arm_smmu_alloc_cd_leaf_table(struct arm_smmu_device *smmu, -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/4] PCI/ATS: Export symbols of PASID functions
The Arm SMMUv3 driver uses pci_{enable,disable}_pasid() and related functions. Export them to allow the driver to be built as a module. Signed-off-by: Jean-Philippe Brucker --- drivers/pci/ats.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c index 3ef0bb281e7c..390e92f2d8d1 100644 --- a/drivers/pci/ats.c +++ b/drivers/pci/ats.c @@ -366,6 +366,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features) return 0; } +EXPORT_SYMBOL_GPL(pci_enable_pasid); /** * pci_disable_pasid - Disable the PASID capability @@ -390,6 +391,7 @@ void pci_disable_pasid(struct pci_dev *pdev) pdev->pasid_enabled = 0; } +EXPORT_SYMBOL_GPL(pci_disable_pasid); /** * pci_restore_pasid_state - Restore PASID capabilities @@ -441,6 +443,7 @@ int pci_pasid_features(struct pci_dev *pdev) return supported; } +EXPORT_SYMBOL_GPL(pci_pasid_features); #define PASID_NUMBER_SHIFT 8 #define PASID_NUMBER_MASK (0x1f << PASID_NUMBER_SHIFT) @@ -469,4 +472,5 @@ int pci_max_pasids(struct pci_dev *pdev) return (1 << supported); } +EXPORT_SYMBOL_GPL(pci_max_pasids); #endif /* CONFIG_PCI_PASID */ -- 2.25.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] uacce: add uacce driver
On 2020/2/13 下午5:15, Herbert Xu wrote: On Mon, Feb 10, 2020 at 03:37:11PM -0800, Greg Kroah-Hartman wrote: Looks much saner now, thanks for all of the work on this: Reviewed-by: Greg Kroah-Hartman Or am I supposed to take this in my tree? If so, I can, but I need an ack for the crypto parts. I can take this series through the crypto tree if that's fine with you. Thanks Herbert That's a good idea, otherwise there may be build issue if taken separately. By the way, the latest v13 is on v5.6-rc1 https://lkml.org/lkml/2020/2/11/54 Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/arm-smmu: fix the module name to be consistent with older kernel
On Wed, Feb 12, 2020 at 01:59:46PM -0600, Li Yang wrote: > On Wed, Feb 12, 2020 at 4:50 AM Will Deacon wrote: > > > > On Tue, Feb 11, 2020 at 06:37:20PM -0600, Li Yang wrote: > > > Commit cd221bd24ff5 ("iommu/arm-smmu: Allow building as a module") > > > introduced a side effect that changed the module name from arm-smmu to > > > arm-smmu-mod. This breaks the users of kernel parameters for the driver > > > (e.g. arm-smmu.disable_bypass). This patch changes the module name back > > > to be consistent. > > > > > > Signed-off-by: Li Yang > > > --- > > > drivers/iommu/Makefile | 4 ++-- > > > drivers/iommu/{arm-smmu.c => arm-smmu-common.c} | 0 > > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > rename drivers/iommu/{arm-smmu.c => arm-smmu-common.c} (100%) > > > > Can't we just override MODULE_PARAM_PREFIX instead of renaming the file? > > I can do that. But on the other hand, the "mod" in the module name > arm-smmu-mod.ko seems to be redundant and looks a little bit weird. > Wouldn't it be cleaner to make it just arm-smmu.ko? I just didn't fancy renaming the file because it's a pain for backports and why does the name of the module matter? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v12 2/4] uacce: add uacce driver
On Mon, Feb 10, 2020 at 03:37:11PM -0800, Greg Kroah-Hartman wrote: > > Looks much saner now, thanks for all of the work on this: > > Reviewed-by: Greg Kroah-Hartman > > Or am I supposed to take this in my tree? If so, I can, but I need an > ack for the crypto parts. I can take this series through the crypto tree if that's fine with you. Thank, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu