Re: [PATCH v4 0/5] memory: Introduce memory controller mini-framework

2020-02-13 Thread Maxime Ripard
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

2020-02-13 Thread Stephen Boyd
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

2020-02-13 Thread Rob Herring
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

2020-02-13 Thread Rob Herring
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

2020-02-13 Thread Rob Herring
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

2020-02-13 Thread Robin Murphy

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

2020-02-13 Thread Rob Herring
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Robin Murphy

[+ 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

2020-02-13 Thread Robin Murphy

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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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()

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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()

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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()

2020-02-13 Thread Jean-Philippe Brucker
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()

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread Thierry Reding
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

2020-02-13 Thread John Garry


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

2020-02-13 Thread Robin Murphy
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

2020-02-13 Thread Greg Kroah-Hartman
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread Jean-Philippe Brucker
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

2020-02-13 Thread zhangfei



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

2020-02-13 Thread Will Deacon
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

2020-02-13 Thread Herbert Xu
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