Re: [RFC 2/5] iommu/vt-d: Unlink device if failed to add to group

2020-01-13 Thread Lu Baolu

On 1/13/20 8:20 PM, Joerg Roedel wrote:

On Sun, Jan 12, 2020 at 09:36:56AM +0800, Lu Baolu wrote:

On 1/1/20 4:24 AM, Jon Derrick wrote:

If the device fails to be added to the group, make sure to unlink the
reference before returning.

Signed-off-by: Jon Derrick


Queued for v5.6.


No need to do so, I sent it upstream with the last pile of iommu fixes.


Got it. Thank you!

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 0/2] iommu: arm-smmu: Add support for early direct mappings

2020-01-13 Thread Bjorn Andersson
On Mon 13 Jan 14:01 PST 2020, Saravana Kannan wrote:

> I added everyone from the other thread, but somehow managed to miss
> the Bjorn who sent the emails! Fixing that now.
> 

Thanks for looping me in Saravana.

> On Mon, Jan 13, 2020 at 6:07 AM Thierry Reding  
> wrote:
> >
> > On Fri, Jan 10, 2020 at 08:56:39PM -0800, Saravana Kannan wrote:
[..]
> > In the case where you're trying to inherit the bootloader configuration
> > of the SMMU, how do you solve the problem of passing the page tables to
> > the kernel? You must have some way of reserving that memory in order to
> > prevent the kernel from reusing it.
> 
> Maybe "inherit" makes it sound a lot more complicated than it is?
> Bjorn will know the details of what the bootloader sets up. But
> AFAICT, it looks like a simple "bypass"/identity mapping too. I don't
> think it actually sets up page tables.
> 

In the Qualcomm case we have a number of stream mappings installed when
the bootloader jumps to the OS, each one with SMR/S2CR referring to a CB
with SMMU_CBn_SCTLR.M not set.

As such the relevant hardware is able to access (without translation)
DDR even with SMMU_CR0.USFCFG being set.

The one case where this causes issues is that upon attaching a device to
a context we'll set SMMU_CBn_SCTLR.M, so until we actually have a
translation installed we do get faults - the difference is that these
are not picked up as fatal faults by the secure firmware, so they are
simply reported in Linux.

[..]
> > > > One option that I can think of would be to create an early identity
> > > > domain for each master and inherit it when that master is attached to
> > > > the domain later on, but that seems rather complicated from an book-
> > > > keeping point of view and tricky because we need to be careful not to
> > > > map regions twice, etc.
> > > >
> > > > Any good ideas on how to solve this? It'd also be interesting to see if
> > > > there's a more generic way of doing this. I know that something like
> > > > this isn't necessary on earlier Tegra SoCs with the custom Tegra SMMU
> > > > because translations are only enabled when the devices are attached to a
> > > > domain.
> > >
> > > Good foresight. As [1] shows, identity mapping doesn't solve it in a
> > > generic way.
> >
> > I think your [1] is a special case of identity mappings where the
> > mappings are already active. If you pass the information about the
> > mappings via memory-region properties, then you should be able to
> > reconstruct the identity mapping in the kernel before switching the
> > SMMU over to the new mapping for a seamless transition.
> 
> Ok, makes sense. But I don't have the full details here. So I'll let
> Bjorn comment here.
> 

It might be possible that we can install page tables and setup 1:1
mappings for the necessary resources, but it's not all devices with a
memory-region and a iommu context defined that should have this.

I will have to discuss this in more detail with the Qualcomm engineers.

PS. One detail that I did notice while distilling the downstream patches
is that unused mappings must have SMMU_S2CRx.CBNDX = 255 or I get odd
crashes when the display (CBNDX = 0) is active. I've yet to conclude
why this is.

Regards,
Bjorn
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC 0/2] iommu: arm-smmu: Add support for early direct mappings

2020-01-13 Thread Saravana Kannan via iommu
I added everyone from the other thread, but somehow managed to miss
the Bjorn who sent the emails! Fixing that now.

On Mon, Jan 13, 2020 at 6:07 AM Thierry Reding  wrote:
>
> On Fri, Jan 10, 2020 at 08:56:39PM -0800, Saravana Kannan wrote:
> > Hi Thierry,
> >
> > I happened upon this thread while looking into another thread [1].
> >
> > > From: Thierry Reding 
> > >
> > > On some platforms, the firmware will setup hardware to read from a given
> > > region of memory. One such example is a display controller that is
> > > scanning out a splash screen from physical memory.
> > >
> > > During Linux' boot process, the ARM SMMU will configure all contexts to
> > > fault by default. This means that memory accesses that happen by an SMMU
> > > master before its driver has had a chance to properly set up the IOMMU
> > > will cause a fault. This is especially annoying for something like the
> > > display controller scanning out a splash screen because the faults will
> > > result in the display controller getting bogus data (all-ones on Tegra)
> > > and since it repeatedly scans that framebuffer, it will keep triggering
> > > such faults and spam the boot log with them.
> >
> > While I'm not an expert on IOMMUs, I have a decent high level
> > understanding of the problem you are trying to solve.
> >
> > > In order to work around such problems, scan the device tree for IOMMU
> > > masters and set up a special identity domain that will map 1:1 all of
> > > the reserved regions associated with them. This happens before the SMMU
> > > is enabled, so that the mappings are already set up before translations
> > > begin.
> >
> > I'm not sure if this RFC will solve the splash screen issue across SoCs
> > ([1] seems to have a different issue and might not have memory-regions).
>
> Looking at the proposed patches, they look like they're solving a
> different, although related, problem. In your case you seem to have a
> bootloader that already sets up the SMMU to translate for a given
> master. The case that I'm trying to solve here is where the bootloader
> has not yet setup the SMMU but has instead pointed some device to read
> memory from a physical address.

Ah, thanks for explaining your scenario.

> So what this patch is trying to solve is to create the mappings that a
> given device needs in order to transparently keep scanning out from an
> address region that it's using, even when the kernel enables address
> translation.

Ok, makes sense.

> In the case where you're trying to inherit the bootloader configuration
> of the SMMU, how do you solve the problem of passing the page tables to
> the kernel? You must have some way of reserving that memory in order to
> prevent the kernel from reusing it.

Maybe "inherit" makes it sound a lot more complicated than it is?
Bjorn will know the details of what the bootloader sets up. But
AFAICT, it looks like a simple "bypass"/identity mapping too. I don't
think it actually sets up page tables.

> > > One thing that was pointed out earlier, and which I don't have a good
> > > idea on how to solve it, is that the early identity domain is not
> > > discarded. The assumption is that the standard direct mappings code of
> > > the IOMMU framework will replace the early identity domain once devices
> > > are properly attached to domains, but we don't have a good point in time
> > > when it would be safe to remove the early identity domain.
> >
> > You are in luck! I added sync_state() driver callbacks [2] exactly for
> > cases like this. Heck, I even listed IOMMUs as an example use case. :)
> > sync_state() works even with modules if one enables of_devlink [3] kernel
> > parameter (which already supports iommus DT bindings). I'd be happy to
> > answer any question you have on sync_state() and of_devlink.
>
> I wasn't aware of of_devlink, but I like it! It does have the drawback
> that you need to reimplement a lot of the (phandle, specifier) parsing
> code, but I don't think anybody was ever able to solve anyway.
>
> Looking at struct supplier_bindings, I think it might be possible to
> share the property parsing code with the subsystems, though. But I
> digress...

Yeah, I don't want to digress either. But as of now, iommus are
already supported.

> Regarding sync_state(), I'm not sure it would be useful in my case. One
> of the drivers I'm dealing with, for example, is a composite driver that
> is created by tying together multiple devices.

If you can give additional details about this, I can give a better
answer. But with the limited info, there's one way I can think of to
handle this. To make the explanation easier, let's call the device
that references the IOMMU in DT as the "direct consumer device". The
driver __probe function__ for the direct consumer device can add a
device link from the composite device to the iommu device. This will
ensure the iommu device doesn't get the sync_state() callback before
the composite device probes. Keep in mind that devices do not need to
be 

Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

2020-01-13 Thread Lorenzo Pieralisi
On Mon, Jan 13, 2020 at 12:01:13PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 13, 2020 at 05:13:38PM +, Derrick, Jonathan wrote:
> > On Mon, 2020-01-13 at 12:08 +, Lorenzo Pieralisi wrote:
> > > On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > > > v2 Set: 
> > > > https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derr...@intel.com/T/#t
> > > > v1 Set: 
> > > > https://lore.kernel.org/linux-iommu/20200107134125.gd30...@8bytes.org/T/#t
> > > > 
> > > > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions 
> > > > at the
> > > > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > > > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > > > 
> > > > This set cleans up VMD by removing the override that redirects DMA map
> > > > operations to the VMD endpoint. Instead it introduces a new DMA alias 
> > > > mechanism
> > > > into the existing DMA alias infrastructure.
> > > > 
> > > > v1 added a pointer in struct pci_dev that pointed to the DMA alias' 
> > > > struct
> > > > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > > > 
> > > > v2 introduced a new weak function to reference the 'Direct DMA Alias', 
> > > > and
> > > > removed the need to add a pointer in struct device or pci_dev. Weak 
> > > > functions
> > > > are generally frowned upon when it's a single architecture 
> > > > implementation, so I
> > > > am open to alternatives.
> > > > 
> > > > v3 references the pci_dev rather than the struct device for the PCI
> > > > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > > > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA 
> > > > alias
> > > > device, though I don't expect the VMD endpoint to need intra-bus DMA 
> > > > aliases.
> > > > 
> > > > Changes from v2:
> > > > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' 
> > > > (pci_direct_dma_alias)
> > > > Allows pci_for_each_dma_alias to iterate over the alias mask of the 
> > > > 'Direct DMA alias'
> > > > 
> > > > Changes from v1:
> > > > Removed 1/5 & 2/5 misc fix patches that were merged
> > > > Uses Christoph's staging/cleanup patches
> > > > Introduce weak function rather than including pointer in struct device 
> > > > or pci_dev.
> > > > 
> > > > Based on Joerg's next:
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > > > 
> > > > Jon Derrick (5):
> > > >   x86/pci: Add a to_pci_sysdata helper
> > > >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> > > >   PCI: Introduce pci_direct_dma_alias()
> > > >   PCI: vmd: Stop overriding dma_map_ops
> > > >   x86/pci: Remove X86_DEV_DMA_OPS
> > > > 
> > > >  arch/x86/Kconfig   |   3 -
> > > >  arch/x86/include/asm/device.h  |  10 ---
> > > >  arch/x86/include/asm/pci.h |  31 -
> > > >  arch/x86/pci/common.c  |  45 ++--
> > > >  drivers/iommu/intel-iommu.c|  18 +++--
> > > >  drivers/pci/controller/Kconfig |   1 -
> > > >  drivers/pci/controller/vmd.c   | 152 
> > > > +
> > > >  drivers/pci/pci.c  |  19 +-
> > > >  drivers/pci/search.c   |   7 ++
> > > >  include/linux/pci.h|   1 +
> > > >  10 files changed, 61 insertions(+), 226 deletions(-)
> > > 
> > > Jon, Christoph,
> > > 
> > > AFAICS this series supersedes/overrides:
> > > 
> > > https://patchwork.kernel.org/patch/4831/
> > > 
> > > Please let me know if that's correct, actually I was waiting to
> > > see consensus on the patch above but if this series supersedes
> > > it I would drop it from the PCI review queue.
> > > 
> > > Thanks,
> > > Lorenzo
> > 
> > It does supercede it (with Christoph's blessing). By the way, I have
> > been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
> > there's more pci work at this point, should I base it on Bjorn's repo
> > instead?
> 
> In general if I'm going to apply something, I prefer it based on my
> "master" branch unless there's a reason to the contrary.  I think
> Lorenzo works pretty much the same way.
> 
> Lorenzo will probably handle this series, but I applied it
> experimentally to check out the brace thing, and it applied fine to my
> "master" branch.  So I think everything's fine as-is.

I think it is mostly x86+core PCI code, VMD code is mostly deletions
so I think you can take it, let me know, I can ACK the VMD changes
straight away.

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

2020-01-13 Thread Bjorn Helgaas
On Mon, Jan 13, 2020 at 05:13:38PM +, Derrick, Jonathan wrote:
> On Mon, 2020-01-13 at 12:08 +, Lorenzo Pieralisi wrote:
> > On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > > v2 Set: 
> > > https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derr...@intel.com/T/#t
> > > v1 Set: 
> > > https://lore.kernel.org/linux-iommu/20200107134125.gd30...@8bytes.org/T/#t
> > > 
> > > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions 
> > > at the
> > > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > > 
> > > This set cleans up VMD by removing the override that redirects DMA map
> > > operations to the VMD endpoint. Instead it introduces a new DMA alias 
> > > mechanism
> > > into the existing DMA alias infrastructure.
> > > 
> > > v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> > > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > > 
> > > v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> > > removed the need to add a pointer in struct device or pci_dev. Weak 
> > > functions
> > > are generally frowned upon when it's a single architecture 
> > > implementation, so I
> > > am open to alternatives.
> > > 
> > > v3 references the pci_dev rather than the struct device for the PCI
> > > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> > > device, though I don't expect the VMD endpoint to need intra-bus DMA 
> > > aliases.
> > > 
> > > Changes from v2:
> > > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' 
> > > (pci_direct_dma_alias)
> > > Allows pci_for_each_dma_alias to iterate over the alias mask of the 
> > > 'Direct DMA alias'
> > > 
> > > Changes from v1:
> > > Removed 1/5 & 2/5 misc fix patches that were merged
> > > Uses Christoph's staging/cleanup patches
> > > Introduce weak function rather than including pointer in struct device or 
> > > pci_dev.
> > > 
> > > Based on Joerg's next:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > > 
> > > Jon Derrick (5):
> > >   x86/pci: Add a to_pci_sysdata helper
> > >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> > >   PCI: Introduce pci_direct_dma_alias()
> > >   PCI: vmd: Stop overriding dma_map_ops
> > >   x86/pci: Remove X86_DEV_DMA_OPS
> > > 
> > >  arch/x86/Kconfig   |   3 -
> > >  arch/x86/include/asm/device.h  |  10 ---
> > >  arch/x86/include/asm/pci.h |  31 -
> > >  arch/x86/pci/common.c  |  45 ++--
> > >  drivers/iommu/intel-iommu.c|  18 +++--
> > >  drivers/pci/controller/Kconfig |   1 -
> > >  drivers/pci/controller/vmd.c   | 152 
> > > +
> > >  drivers/pci/pci.c  |  19 +-
> > >  drivers/pci/search.c   |   7 ++
> > >  include/linux/pci.h|   1 +
> > >  10 files changed, 61 insertions(+), 226 deletions(-)
> > 
> > Jon, Christoph,
> > 
> > AFAICS this series supersedes/overrides:
> > 
> > https://patchwork.kernel.org/patch/4831/
> > 
> > Please let me know if that's correct, actually I was waiting to
> > see consensus on the patch above but if this series supersedes
> > it I would drop it from the PCI review queue.
> > 
> > Thanks,
> > Lorenzo
> 
> It does supercede it (with Christoph's blessing). By the way, I have
> been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
> there's more pci work at this point, should I base it on Bjorn's repo
> instead?

In general if I'm going to apply something, I prefer it based on my
"master" branch unless there's a reason to the contrary.  I think
Lorenzo works pretty much the same way.

Lorenzo will probably handle this series, but I applied it
experimentally to check out the brace thing, and it applied fine to my
"master" branch.  So I think everything's fine as-is.

Bjorn
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

2020-01-13 Thread Derrick, Jonathan
Hi Lorenzo,

On Mon, 2020-01-13 at 12:08 +, Lorenzo Pieralisi wrote:
> On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> > v2 Set: 
> > https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derr...@intel.com/T/#t
> > v1 Set: 
> > https://lore.kernel.org/linux-iommu/20200107134125.gd30...@8bytes.org/T/#t
> > 
> > VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at 
> > the
> > VMD endpoint. The problem with this approach is that the VMD endpoint's
> > device-specific attributes, such as the DMA Mask Bits, are used instead.
> > 
> > This set cleans up VMD by removing the override that redirects DMA map
> > operations to the VMD endpoint. Instead it introduces a new DMA alias 
> > mechanism
> > into the existing DMA alias infrastructure.
> > 
> > v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> > pci_dev and did the necessary DMA alias and IOMMU modifications.
> > 
> > v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> > removed the need to add a pointer in struct device or pci_dev. Weak 
> > functions
> > are generally frowned upon when it's a single architecture implementation, 
> > so I
> > am open to alternatives.
> > 
> > v3 references the pci_dev rather than the struct device for the PCI
> > 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> > pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> > device, though I don't expect the VMD endpoint to need intra-bus DMA 
> > aliases.
> > 
> > Changes from v2:
> > Uses struct pci_dev for PCI Device 'Direct DMA aliasing' 
> > (pci_direct_dma_alias)
> > Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct 
> > DMA alias'
> > 
> > Changes from v1:
> > Removed 1/5 & 2/5 misc fix patches that were merged
> > Uses Christoph's staging/cleanup patches
> > Introduce weak function rather than including pointer in struct device or 
> > pci_dev.
> > 
> > Based on Joerg's next:
> > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> > 
> > Jon Derrick (5):
> >   x86/pci: Add a to_pci_sysdata helper
> >   x86/PCI: Expose VMD's PCI Device in pci_sysdata
> >   PCI: Introduce pci_direct_dma_alias()
> >   PCI: vmd: Stop overriding dma_map_ops
> >   x86/pci: Remove X86_DEV_DMA_OPS
> > 
> >  arch/x86/Kconfig   |   3 -
> >  arch/x86/include/asm/device.h  |  10 ---
> >  arch/x86/include/asm/pci.h |  31 -
> >  arch/x86/pci/common.c  |  45 ++--
> >  drivers/iommu/intel-iommu.c|  18 +++--
> >  drivers/pci/controller/Kconfig |   1 -
> >  drivers/pci/controller/vmd.c   | 152 
> > +
> >  drivers/pci/pci.c  |  19 +-
> >  drivers/pci/search.c   |   7 ++
> >  include/linux/pci.h|   1 +
> >  10 files changed, 61 insertions(+), 226 deletions(-)
> 
> Jon, Christoph,
> 
> AFAICS this series supersedes/overrides:
> 
> https://patchwork.kernel.org/patch/4831/
> 
> Please let me know if that's correct, actually I was waiting to
> see consensus on the patch above but if this series supersedes
> it I would drop it from the PCI review queue.
> 
> Thanks,
> Lorenzo

It does supercede it (with Christoph's blessing). By the way, I have
been basing on Joerg's repo  due to the v1/RFC IOMMU modifcations. As
there's more pci work at this point, should I base it on Bjorn's repo
instead?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 1/5] x86/pci: Add a to_pci_sysdata helper

2020-01-13 Thread Bjorn Helgaas
On Fri, Jan 10, 2020 at 10:21:09AM -0700, Jon Derrick wrote:
> From: Christoph Hellwig 
> 
> Various helpers need the pci_sysdata just to dereference a single field
> in it.  Add a little helper that returns the properly typed sysdata
> pointer to require a little less boilerplate code.
> 
> Signed-off-by: Christoph Hellwig 
> [jonathan.derrick: added un-const cast]
> Signed-off-by: Jon Derrick 
> ---
>  arch/x86/include/asm/pci.h | 28 +---
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index 90d0731..cf680c5 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -35,12 +35,15 @@ struct pci_sysdata {
>  
>  #ifdef CONFIG_PCI
>  
> +static inline struct pci_sysdata *to_pci_sysdata(struct pci_bus *bus)
> +{
> + return bus->sysdata;
> +}
> +
>  #ifdef CONFIG_PCI_DOMAINS
>  static inline int pci_domain_nr(struct pci_bus *bus)
>  {
> - struct pci_sysdata *sd = bus->sysdata;
> -
> - return sd->domain;
> + return to_pci_sysdata(bus)->domain;
>  }
>  
>  static inline int pci_proc_domain(struct pci_bus *bus)
> @@ -52,23 +55,20 @@ static inline int pci_proc_domain(struct pci_bus *bus)
>  #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
>  static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
>  {
> - struct pci_sysdata *sd = bus->sysdata;
> -
> - return sd->fwnode;
> + return to_pci_sysdata(bus)->fwnode;
>  }
>  
>  #define pci_root_bus_fwnode  _pci_root_bus_fwnode
>  #endif
>  
> +#if IS_ENABLED(CONFIG_VMD)
>  static inline bool is_vmd(struct pci_bus *bus)
>  {
> -#if IS_ENABLED(CONFIG_VMD)
> - struct pci_sysdata *sd = bus->sysdata;
> -
> - return sd->vmd_domain;
> + return to_pci_sysdata(bus)->vmd_domain;
> +}
>  #else
> - return false;
> -#endif
> +#define is_vmd(bus)  false
> +#endif /* CONFIG_VMD */
>  }

I think this patch leaves this stray close brace here (it's cleaned up
in the next patch, but looks like it will break bisection).

Also, when you fix this, can you update the subject lines?  There's a
mix of "x86/PCI" and "x86/pci" (the convention per "git log --oneline"
is "x86/PCI").

>  /* Can be used to override the logic in pci_scan_bus for skipping
> @@ -124,9 +124,7 @@ static inline void early_quirks(void) { }
>  /* Returns the node based on pci bus */
>  static inline int __pcibus_to_node(const struct pci_bus *bus)
>  {
> - const struct pci_sysdata *sd = bus->sysdata;
> -
> - return sd->node;
> + return to_pci_sysdata((struct pci_bus *) bus)->node;
>  }
>  
>  static inline const struct cpumask *
> -- 
> 1.8.3.1
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH] iommu/arm-smmu-v3: Add SMMUv3.2 range invalidation support

2020-01-13 Thread Rob Herring
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 and then adjust the granule to
32x that size.

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 | 53 +
 1 file changed, 53 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e91b4a098215..8b6b3e2aa383 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -70,6 +70,9 @@
 #define IDR1_SSIDSIZE  GENMASK(10, 6)
 #define IDR1_SIDSIZE   GENMASK(5, 0)
 
+#define ARM_SMMU_IDR3  0xc
+#define IDR3_RIL   (1 << 10)
+
 #define ARM_SMMU_IDR5  0x14
 #define IDR5_STALL_MAX GENMASK(31, 16)
 #define IDR5_GRAN64K   (1 << 6)
@@ -327,9 +330,14 @@
 #define CMDQ_CFGI_1_LEAF   (1UL << 0)
 #define CMDQ_CFGI_1_RANGE  GENMASK_ULL(4, 0)
 
+#define CMDQ_TLBI_0_NUMGENMASK_ULL(16, 12)
+#define CMDQ_TLBI_RANGE_NUM_MAX32
+#define CMDQ_TLBI_0_SCALE  GENMASK_ULL(24, 20)
 #define CMDQ_TLBI_0_VMID   GENMASK_ULL(47, 32)
 #define CMDQ_TLBI_0_ASID   GENMASK_ULL(63, 48)
 #define CMDQ_TLBI_1_LEAF   (1UL << 0)
+#define CMDQ_TLBI_1_TTLGENMASK_ULL(9, 8)
+#define CMDQ_TLBI_1_TG GENMASK_ULL(11, 10)
 #define CMDQ_TLBI_1_VA_MASKGENMASK_ULL(63, 12)
 #define CMDQ_TLBI_1_IPA_MASK   GENMASK_ULL(51, 12)
 
@@ -455,9 +463,13 @@ struct arm_smmu_cmdq_ent {
#define CMDQ_OP_TLBI_S2_IPA 0x2a
#define CMDQ_OP_TLBI_NSNH_ALL   0x30
struct {
+   u8  num;
+   u8  scale;
u16 asid;
u16 vmid;
boolleaf;
+   u8  ttl;
+   u8  tg;
u64 addr;
} tlbi;
 
@@ -595,6 +607,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_FEAT_HYP  (1 << 12)
 #define ARM_SMMU_FEAT_STALL_FORCE  (1 << 13)
 #define ARM_SMMU_FEAT_VAX  (1 << 14)
+#define ARM_SMMU_FEAT_RANGE_INV(1 << 15)
u32 features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
@@ -856,13 +869,21 @@ static int arm_smmu_cmdq_build_cmd(u64 *cmd, struct 
arm_smmu_cmdq_ent *ent)
cmd[1] |= FIELD_PREP(CMDQ_CFGI_1_RANGE, 31);
break;
case CMDQ_OP_TLBI_NH_VA:
+   cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_NUM, ent->tlbi.num);
+   cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_SCALE, ent->tlbi.scale);
cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_ASID, ent->tlbi.asid);
cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_LEAF, ent->tlbi.leaf);
+   cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_TTL, ent->tlbi.ttl);
+   cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_TG, ent->tlbi.tg);
cmd[1] |= ent->tlbi.addr & CMDQ_TLBI_1_VA_MASK;
break;
case CMDQ_OP_TLBI_S2_IPA:
+   cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_NUM, ent->tlbi.num);
+   cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_SCALE, ent->tlbi.scale);
cmd[0] |= FIELD_PREP(CMDQ_TLBI_0_VMID, ent->tlbi.vmid);
cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_LEAF, ent->tlbi.leaf);
+   cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_TTL, ent->tlbi.ttl);
+   cmd[1] |= FIELD_PREP(CMDQ_TLBI_1_TG, ent->tlbi.tg);
cmd[1] |= ent->tlbi.addr & CMDQ_TLBI_1_IPA_MASK;
break;
case CMDQ_OP_TLBI_NH_ASID:
@@ -2022,12 +2043,39 @@ 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) {
+   unsigned long tg, scale;
+
+   /* Get the leaf page size */
+   tg = __ffs(smmu_domain->domain.pgsize_bitmap);
+
+   /* Determine the power of 2 multiple number of pages */
+   scale = __ffs(size / (1UL << tg));
+   cmd.tlbi.scale = scale;
+
+   cmd.tlbi.num = CMDQ_TLBI_RANGE_NUM_MAX - 1;
+
+   /* 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 */
+   

[PATCH] iommu/arm-smmu-v3: Batch ATC invalidation commands

2020-01-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.

Cc: Jean-Philippe Brucker 
Cc: Will Deacon 
Cc: Robin Murphy 
Cc: Joerg Roedel 
Signed-off-by: Rob Herring 
---
 drivers/iommu/arm-smmu-v3.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index effe72eb89e7..e91b4a098215 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1919,10 +1919,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;
@@ -1947,11 +1948,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 */
-- 
2.20.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] drm/rockchip: Add missing vmalloc header

2020-01-13 Thread Heiko Stuebner
Am Dienstag, 31. Dezember 2019, 09:12:36 CET schrieb Krzysztof Kozlowski:
> The Rockship DRM GEM code uses vmap()/vunmap() so vmalloc header must be
> included to avoid warnings like (on IA64, compile tested):
> 
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c: In function 
> ‘rockchip_gem_alloc_iommu’:
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c:134:20: error:
> implicit declaration of function ‘vmap’ 
> [-Werror=implicit-function-declaration]
> 
> Reported-by: kbuild test robot 
> Signed-off-by: Krzysztof Kozlowski 

applied to drm-misc-next

Thanks
Heiko


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC 2/5] iommu/vt-d: Unlink device if failed to add to group

2020-01-13 Thread Joerg Roedel
On Sun, Jan 12, 2020 at 09:36:56AM +0800, Lu Baolu wrote:
> On 1/1/20 4:24 AM, Jon Derrick wrote:
> > If the device fails to be added to the group, make sure to unlink the
> > reference before returning.
> > 
> > Signed-off-by: Jon Derrick
> 
> Queued for v5.6.

No need to do so, I sent it upstream with the last pile of iommu fixes.


Thanks,

Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v3 0/5] Clean up VMD DMA Map Ops

2020-01-13 Thread Lorenzo Pieralisi
On Fri, Jan 10, 2020 at 10:21:08AM -0700, Jon Derrick wrote:
> v2 Set: 
> https://lore.kernel.org/linux-iommu/1578580256-3483-1-git-send-email-jonathan.derr...@intel.com/T/#t
> v1 Set: 
> https://lore.kernel.org/linux-iommu/20200107134125.gd30...@8bytes.org/T/#t
> 
> VMD currently works with VT-d enabled by pointing DMA and IOMMU actions at the
> VMD endpoint. The problem with this approach is that the VMD endpoint's
> device-specific attributes, such as the DMA Mask Bits, are used instead.
> 
> This set cleans up VMD by removing the override that redirects DMA map
> operations to the VMD endpoint. Instead it introduces a new DMA alias 
> mechanism
> into the existing DMA alias infrastructure.
> 
> v1 added a pointer in struct pci_dev that pointed to the DMA alias' struct
> pci_dev and did the necessary DMA alias and IOMMU modifications.
> 
> v2 introduced a new weak function to reference the 'Direct DMA Alias', and
> removed the need to add a pointer in struct device or pci_dev. Weak functions
> are generally frowned upon when it's a single architecture implementation, so 
> I
> am open to alternatives.
> 
> v3 references the pci_dev rather than the struct device for the PCI
> 'Direct DMA Alias' (pci_direct_dma_alias()). This revision also allows
> pci_for_each_dma_alias() to call any DMA aliases for the Direct DMA alias
> device, though I don't expect the VMD endpoint to need intra-bus DMA aliases.
> 
> Changes from v2:
> Uses struct pci_dev for PCI Device 'Direct DMA aliasing' 
> (pci_direct_dma_alias)
> Allows pci_for_each_dma_alias to iterate over the alias mask of the 'Direct 
> DMA alias'
> 
> Changes from v1:
> Removed 1/5 & 2/5 misc fix patches that were merged
> Uses Christoph's staging/cleanup patches
> Introduce weak function rather than including pointer in struct device or 
> pci_dev.
> 
> Based on Joerg's next:
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/
> 
> Jon Derrick (5):
>   x86/pci: Add a to_pci_sysdata helper
>   x86/PCI: Expose VMD's PCI Device in pci_sysdata
>   PCI: Introduce pci_direct_dma_alias()
>   PCI: vmd: Stop overriding dma_map_ops
>   x86/pci: Remove X86_DEV_DMA_OPS
> 
>  arch/x86/Kconfig   |   3 -
>  arch/x86/include/asm/device.h  |  10 ---
>  arch/x86/include/asm/pci.h |  31 -
>  arch/x86/pci/common.c  |  45 ++--
>  drivers/iommu/intel-iommu.c|  18 +++--
>  drivers/pci/controller/Kconfig |   1 -
>  drivers/pci/controller/vmd.c   | 152 
> +
>  drivers/pci/pci.c  |  19 +-
>  drivers/pci/search.c   |   7 ++
>  include/linux/pci.h|   1 +
>  10 files changed, 61 insertions(+), 226 deletions(-)

Jon, Christoph,

AFAICS this series supersedes/overrides:

https://patchwork.kernel.org/patch/4831/

Please let me know if that's correct, actually I was waiting to
see consensus on the patch above but if this series supersedes
it I would drop it from the PCI review queue.

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu