[PATCH 4/4] iommu/vt-d: Remove unnecessary parentheses

[Lu Baolu]

Remove unnecessary parentheses to comply with preferred coding
style.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e8cd984..45f6e58 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -319,7 +319,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
} else
pasid_max = 1 << 20;
 
-   if ((flags & SVM_FLAG_SUPERVISOR_MODE)) {
+   if (flags & SVM_FLAG_SUPERVISOR_MODE) {
if (!ecap_srs(iommu->ecap))
return -EINVAL;
} else if (pasid) {
-- 
2.7.4

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

[PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc

[Lu Baolu]

@name parameter has been removed.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/iommu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d2aa2320..d87e7c2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -322,7 +322,6 @@ static struct kobj_type iommu_group_ktype = {
 
 /**
  * iommu_group_alloc - Allocate a new group
- * @name: Optional name to associate with group, visible in sysfs
  *
  * This function is called by an iommu driver to allocate a new iommu
  * group.  The iommu group represents the minimum granularity of the iommu.
-- 
2.7.4

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

Re: [PATCH 11/13] mips, unicore32: swiotlb doesn't need sg->dma_length

[James Hogan via iommu]

On Thu, May 03, 2018 at 05:56:43AM +0200, Christoph Hellwig wrote:
> On Wed, May 02, 2018 at 11:20:18PM +0100, James Hogan wrote:
> > On Wed, Apr 25, 2018 at 07:15:37AM +0200, Christoph Hellwig wrote:
> > > Only mips and unicore32 select CONFIG_NEED_SG_DMA_LENGTH when building
> > > swiotlb.  swiotlb itself never merges segements and doesn't accesses the
> > > dma_length field directly, so drop the dependency.
> > 
> > Is that at odds with Documentation/DMA-API-HOWTO.txt, which seems to
> > suggest arch ports should enable it for IOMMUs?
> 
> swiotlb isn't really an iommu..  That being said iommus don't have to
> merge segments either if they don't want to, and we have various
> implementations that don't.  The whole dma api documentation needs
> a major overhaul, including merging the various files and dropping a lot
> of dead wood.  It has been on my todo list for a while, with an inner
> hope that someone else would do it before me.

Okay, for MIPS:
Acked-by: James Hogan 

Cheers
James


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

[GIT PULL] dma mapping fix for 4.17-rc4

[Christoph Hellwig]

The following changes since commit 2d618bdf71635463a4aa4ad0fe46ec852292bc0c:

  Merge branch 'for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/rkuo/linux-hexagon-kernel 
(2018-05-01 19:54:22 -0700)

are available in the Git repository at:

  git://git.infradead.org/users/hch/dma-mapping.git tags/dma-mapping-4.17-4

for you to fetch changes up to 892a0be43edd63e1cd228af3453a064e9e94f08e:

  swiotlb: fix inversed DMA_ATTR_NO_WARN test (2018-05-02 14:48:55 +0200)


Fix an incorrect warning selection introduces in the last merge window.


Michel Dänzer (1):
  swiotlb: fix inversed DMA_ATTR_NO_WARN test

 lib/swiotlb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu/vt-d: fix shift-out-of-bounds in bug checking

[Joerg Roedel]

On Fri, Apr 20, 2018 at 01:29:55PM +0800, changbin...@intel.com wrote:
> 
> Signed-off-by: Changbin Du 
> ---
>  drivers/iommu/dmar.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied and put a patch on-top to change the BUG_ON to a WARN_ON_ONCE.

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

Re: [PATCH v4 1/2] iommu - Enable debugfs exposure of the IOMMU

[Joerg Roedel]

On Mon, Apr 30, 2018 at 02:58:49PM -0500, Gary R Hook wrote:
> Provide base enablement for using debugfs to expose internal data of
> an IOMMU driver. When called, create the /sys/kernel/debug/iommu
> directory.  Emit a strong warning at boot time to indicate that this
> feature is enabled.
> 
> This patch adds a top-level function that will create the (above)
> directory, under which a driver may create a hw-specific directory for
> its use. The function
> 
>   iommu_debugfs_setup()

That function should be called in iommu core-code, in case we want to
export some generic information from there.

Each driver then calls a separate function like

dentry = iommu_debugfs_driver_dir("amd_iommu");

to create a driver-specific directory.

> +config IOMMU_DEBUGFS
> + bool "Enable IOMMU internals in DebugFS"

s/Enable/Export/

> + * static struct dentry *my_debugfs;
> + *
> + *struct dentry *d_top;

Missing newline

> + *if (!my_debugfs) {
> + *d_top = iommu_debugfs_setup();
> + *if (d_top)
> + *my_debugfs = debugfs_create_dir("vendor", d_top);
> + *}
> + *
> + * Since the IOMMU driver can not be removed from the running system, there
> + * is no need for an "off" function.
> + */
> +struct dentry *iommu_debugfs_setup(void)
> +{
> + if (!iommu_debugfs_dir) {
> + iommu_debugfs_dir = debugfs_create_dir("iommu", NULL);
> + if (iommu_debugfs_dir)
> + pr_warn("WARNING: IOMMU DEBUGFS SUPPORT HAS BEEN 
> ENABLED IN THIS KERNEL\n");

This warning needs to be a lot bigger. See the trace_printk() warning
for an example of what I have in mind.

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

Re: [RFC/RFT] Add noats flag to boot parameters

[Sinan Kaya]

On 5/3/2018 9:35 AM, Joerg Roedel wrote:
> On Sun, Apr 29, 2018 at 09:16:48PM +0300, Gil Kupfer wrote:
>> This patch adds noats option to the pci boot parameter.
>> When noats is selected, all ATS related functions fail immediately and
>> the IOMMU is configured to not use device-iotlb.
>>
>> Any function that checks for ATS capabilities directly against the
>> devices should also check this flag. (Currently, such functions exist
>> only in IOMMU drivers, and they are covered by this patch.)
>>
>> The motivation behind this patch is the existence of malicious devices.
>> Lots of research has been done about how to utilitize the IOMMU as a
>> protection from such devices. When ATS is supported, any I/O device can
>> access any physical access by faking device-IOTLB entries.
>> Adding the ability to ignore these entries lets sysadmins enhance system
>> security.
>>
>> Signed-off-by: Gil Kupfer 
> 
> This has also been on my list, thanks for doing that.
> 
> Acked-by: Joerg Roedel 
> 

I also like the idea in general.
Minor nit..

Shouldn't this be an iommu parameter rather than a PCI kernel command line 
parameter?
We now have an iommu.passthrough argument that prevents page translation.

Doesn't this fit into the same category especially when it is the IOMMU drivers 
that
call ATS functions for enablement not the PCI drivers.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/RFT] Add noats flag to boot parameters

[Sinan Kaya]

+Bjorn,

On 5/3/2018 9:59 AM, Joerg Roedel wrote:
> On Thu, May 03, 2018 at 09:46:34AM -0400, Sinan Kaya wrote:
>> I also like the idea in general.
>> Minor nit..
>>
>> Shouldn't this be an iommu parameter rather than a PCI kernel command line 
>> parameter?
>> We now have an iommu.passthrough argument that prevents page translation.
>>
>> Doesn't this fit into the same category especially when it is the IOMMU 
>> drivers that
>> call ATS functions for enablement not the PCI drivers.
> 
> ATS is a bit of a grey area between PCI and IOMMU, but since ATS is
> PCI-specific and the code to enable/disable it is in PCI as well, I
> think the parameter makes sense for PCI too.
> 

OK. Bjorn was interested in having a command line driven feature enables in 
driver/pci
directory with bitmasks for each optional PCI spec capability rather than noXYZ 
feature.

This would allow us to troubleshoot code breakage as well as the platform bring 
up to
turn off all optional features.

Sounds like this would be a good match for that work.


> 
>   Joerg
> 
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux 
Foundation Collaborative Project.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

[Gary R Hook]

On 05/03/2018 08:57 AM, Joerg Roedel wrote:

On Tue, May 01, 2018 at 02:52:52PM -0500, Gary R Hook wrote:

@@ -567,7 +567,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void 
*__evt)
}
  
  	if (type == EVENT_TYPE_IO_FAULT) {

-   amd_iommu_report_page_fault(devid, domid, address, flags);
+   amd_iommu_report_page_fault(devid, pasid, address, flags);


According to the spec this could be a domid or a pasid, it would be good
to make that visible in the error message, depending on the value of the
GN flag in the event entry.

But that can be done in a separate patch, I applied these two, thanks.



Yes, I didn't quite get this right. Both values should be passed along. 
Or perhaps the entire event could be passed in and decoded by 
amd_iommu_report_page_fault()?

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

Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

[Joerg Roedel]

On Tue, May 01, 2018 at 02:52:52PM -0500, Gary R Hook wrote:
> @@ -567,7 +567,7 @@ static void iommu_print_event(struct amd_iommu *iommu, 
> void *__evt)
>   }
>  
>   if (type == EVENT_TYPE_IO_FAULT) {
> - amd_iommu_report_page_fault(devid, domid, address, flags);
> + amd_iommu_report_page_fault(devid, pasid, address, flags);

According to the spec this could be a domid or a pasid, it would be good
to make that visible in the error message, depending on the value of the
GN flag in the event entry.

But that can be done in a separate patch, I applied these two, thanks.

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

Re: [PATCH] iommu/io-pgtable-arm: Use for_each_set_bit to simplify code

[Joerg Roedel]

On Thu, Apr 26, 2018 at 12:49:29PM +0800, YueHaibing wrote:
> We can use for_each_set_bit() to simplify code slightly in the
> ARM io-pgtable self tests while unmapping.
> 
> Signed-off-by: YueHaibing 
> ---
>  drivers/iommu/io-pgtable-arm-v7s.c | 5 +
>  drivers/iommu/io-pgtable-arm.c | 5 +
>  2 files changed, 2 insertions(+), 8 deletions(-)

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

Re: [RFC/RFT] Add noats flag to boot parameters

[Joerg Roedel]

On Sun, Apr 29, 2018 at 09:16:48PM +0300, Gil Kupfer wrote:
> This patch adds noats option to the pci boot parameter.
> When noats is selected, all ATS related functions fail immediately and
> the IOMMU is configured to not use device-iotlb.
> 
> Any function that checks for ATS capabilities directly against the
> devices should also check this flag. (Currently, such functions exist
> only in IOMMU drivers, and they are covered by this patch.)
> 
> The motivation behind this patch is the existence of malicious devices.
> Lots of research has been done about how to utilitize the IOMMU as a
> protection from such devices. When ATS is supported, any I/O device can
> access any physical access by faking device-IOTLB entries.
> Adding the ability to ignore these entries lets sysadmins enhance system
> security.
> 
> Signed-off-by: Gil Kupfer 

This has also been on my list, thanks for doing that.

Acked-by: Joerg Roedel 

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

Re: [RFC/RFT] Add noats flag to boot parameters

[Joerg Roedel]

On Thu, May 03, 2018 at 09:46:34AM -0400, Sinan Kaya wrote:
> I also like the idea in general.
> Minor nit..
> 
> Shouldn't this be an iommu parameter rather than a PCI kernel command line 
> parameter?
> We now have an iommu.passthrough argument that prevents page translation.
> 
> Doesn't this fit into the same category especially when it is the IOMMU 
> drivers that
> call ATS functions for enablement not the PCI drivers.

ATS is a bit of a grey area between PCI and IOMMU, but since ATS is
PCI-specific and the code to enable/disable it is in PCI as well, I
think the parameter makes sense for PCI too.


Joerg

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

Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

[Joerg Roedel]

On Thu, May 03, 2018 at 01:09:18PM -0500, Gary R Hook wrote:
> Yes, I didn't quite get this right. Both values should be passed along. Or
> perhaps the entire event could be passed in and decoded by
> amd_iommu_report_page_fault()?

You already pass in the flags, you can chose the correct message by
looking at the GN flag there and treat the value as a domid or a pasid.


Joerg

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

Re: [PATCHv4 1/2] iommu/vt-d: Ratelimit each dmar fault printing

[Dmitry Safonov via iommu]

On Thu, 2018-05-03 at 14:40 +0200, Joerg Roedel wrote:
> On Wed, May 02, 2018 at 03:22:24AM +0100, Dmitry Safonov wrote:
> > Hi Joerg,
> > 
> > is there anything I may do about those two patches?
> > In 2/2 I've limited loop cnt as discussed in v3.
> > This one solves softlockup for us, might be useful.
> 
> Applied the first patch, thanks. Please re-work the second one
> according
> to the comments.

Will do.

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

Re: [PATCH v4 2/2] drivers: remove force dma flag from buses

[Vinod Koul]

On Sat, Apr 28, 2018 at 08:21:59AM +0530, Nipun Gupta wrote:
> With each bus implementing its own DMA configuration callback,
> there is no need for bus to explicitly have force_dma in its
> global structure. This patch modifies of_dma_configure API to
> accept an input parameter which specifies if implicit DMA
> configuration is required even when it is not described by the
> firmware.
> 
> Signed-off-by: Nipun Gupta 
> Acked-by: Bjorn Helgaas   # PCI parts
> ---
> Changes in v2:
>   - This is a new change suggested by Robin and Christoph
> and is added to the series.
> 
> Changes in v3:
>   - Rebase and changes corresponding to the changes in patch 1/2
> 
> Changes in v4:
>   - Rebased on top of 4.17-rc2
> 
>  drivers/amba/bus.c| 1 -
>  drivers/base/platform.c   | 3 +--
>  drivers/bcma/main.c   | 2 +-
>  drivers/dma/qcom/hidma_mgmt.c | 2 +-

This one:
Acked-by: Vinod Koul 

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

Re: [RFC/RFT] Add noats flag to boot parameters

[Bjorn Helgaas]

On Thu, May 03, 2018 at 10:23:02AM -0400, Sinan Kaya wrote:
> +Bjorn,
> 
> On 5/3/2018 9:59 AM, Joerg Roedel wrote:
> > On Thu, May 03, 2018 at 09:46:34AM -0400, Sinan Kaya wrote:
> >> I also like the idea in general.
> >> Minor nit..
> >>
> >> Shouldn't this be an iommu parameter rather than a PCI kernel command line 
> >> parameter?
> >> We now have an iommu.passthrough argument that prevents page translation.
> >>
> >> Doesn't this fit into the same category especially when it is the IOMMU 
> >> drivers that
> >> call ATS functions for enablement not the PCI drivers.
> > 
> > ATS is a bit of a grey area between PCI and IOMMU, but since ATS is
> > PCI-specific and the code to enable/disable it is in PCI as well, I
> > think the parameter makes sense for PCI too.
> 
> OK. Bjorn was interested in having a command line driven feature enables
> in driver/pci directory with bitmasks for each optional PCI spec
> capability rather than noXYZ feature.

It's true that I try to avoid adding *any* kernel parameters as much
as possible because they're usually not practical for end-users.

I think it's unreasonable to expect users to use "pci=" parameters
based on what specific hardware they have.  That's too hard to
discover and too hard to use.  I did wonder about a "pci=safe"
parameter that would disable potentially risky features just as a
debugging feature [1].

This ATS case is a security question and the parameter is not
something that would have to be used to get certain hardware to work,
so I think it's probably reasonable to add.  I would maybe expand the
documentation so it includes the reason somebody might want it, i.e.,
to defend against malicious PCIe devices.

A parameter using bitmasks could be conceivable for developers but
sounds too unwieldy for end-users.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=196197#c53
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/RFT] Add noats flag to boot parameters

[Nadav Amit]

Sinan Kaya  wrote:

> +Bjorn,
> 
> On 5/3/2018 9:59 AM, Joerg Roedel wrote:
>> On Thu, May 03, 2018 at 09:46:34AM -0400, Sinan Kaya wrote:
>>> I also like the idea in general.
>>> Minor nit..
>>> 
>>> Shouldn't this be an iommu parameter rather than a PCI kernel command line 
>>> parameter?
>>> We now have an iommu.passthrough argument that prevents page translation.
>>> 
>>> Doesn't this fit into the same category especially when it is the IOMMU 
>>> drivers that
>>> call ATS functions for enablement not the PCI drivers.
>> 
>> ATS is a bit of a grey area between PCI and IOMMU, but since ATS is
>> PCI-specific and the code to enable/disable it is in PCI as well, I
>> think the parameter makes sense for PCI too.
> 
> OK. Bjorn was interested in having a command line driven feature enables in 
> driver/pci
> directory with bitmasks for each optional PCI spec capability rather than 
> noXYZ feature.
> 
> This would allow us to troubleshoot code breakage as well as the platform 
> bring up to
> turn off all optional features.
> 
> Sounds like this would be a good match for that work.

I think that since this feature (ATS) has security implications, it should
be controllable through the kernel boot parameters. Otherwise, it can be
potentially too late to turn it off.

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

[PATCH v2 4/9] iommu/vt-d: Move device_domain_info to header

[Lu Baolu]

This allows the per device iommu data to be accessed from other
files.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-iommu.c | 62 +++--
 include/linux/intel-iommu.h | 68 +
 2 files changed, 72 insertions(+), 58 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 98c5ae9..caa0b5c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -381,60 +381,6 @@ static int hw_pass_through = 1;
for (idx = 0; idx < g_num_of_iommus; idx++) \
if (domain->iommu_refcnt[idx])
 
-struct dmar_domain {
-   int nid;/* node id */
-
-   unsignediommu_refcnt[DMAR_UNITS_SUPPORTED];
-   /* Refcount of devices per iommu */
-
-
-   u16 iommu_did[DMAR_UNITS_SUPPORTED];
-   /* Domain ids per IOMMU. Use u16 since
-* domain ids are 16 bit wide according
-* to VT-d spec, section 9.3 */
-
-   bool has_iotlb_device;
-   struct list_head devices;   /* all devices' list */
-   struct iova_domain iovad;   /* iova's that belong to this domain */
-
-   struct dma_pte  *pgd;   /* virtual address */
-   int gaw;/* max guest address width */
-
-   /* adjusted guest address width, 0 is level 2 30-bit */
-   int agaw;
-
-   int flags;  /* flags to find out type of domain */
-
-   int iommu_coherency;/* indicate coherency of iommu access */
-   int iommu_snooping; /* indicate snooping control feature*/
-   int iommu_count;/* reference count of iommu */
-   int iommu_superpage;/* Level of superpages supported:
-  0 == 4KiB (no superpages), 1 == 2MiB,
-  2 == 1GiB, 3 == 512GiB, 4 == 1TiB */
-   u64 max_addr;   /* maximum mapped address */
-
-   struct iommu_domain domain; /* generic domain data structure for
-  iommu core */
-};
-
-/* PCI domain-device relationship */
-struct device_domain_info {
-   struct list_head link;  /* link to domain siblings */
-   struct list_head global; /* link to global list */
-   u8 bus; /* PCI bus number */
-   u8 devfn;   /* PCI devfn number */
-   u8 pasid_supported:3;
-   u8 pasid_enabled:1;
-   u8 pri_supported:1;
-   u8 pri_enabled:1;
-   u8 ats_supported:1;
-   u8 ats_enabled:1;
-   u8 ats_qdep;
-   struct device *dev; /* it's NULL for PCIe-to-PCI bridge */
-   struct intel_iommu *iommu; /* IOMMU used by this device */
-   struct dmar_domain *domain; /* pointer to domain */
-};
-
 struct dmar_rmrr_unit {
struct list_head list;  /* list of rmrr units   */
struct acpi_dmar_header *hdr;   /* ACPI header  */
@@ -631,7 +577,7 @@ static void set_iommu_domain(struct intel_iommu *iommu, u16 
did,
domains[did & 0xff] = domain;
 }
 
-static inline void *alloc_pgtable_page(int node)
+void *alloc_pgtable_page(int node)
 {
struct page *page;
void *vaddr = NULL;
@@ -642,7 +588,7 @@ static inline void *alloc_pgtable_page(int node)
return vaddr;
 }
 
-static inline void free_pgtable_page(void *vaddr)
+void free_pgtable_page(void *vaddr)
 {
free_page((unsigned long)vaddr);
 }
@@ -725,7 +671,7 @@ int iommu_calculate_agaw(struct intel_iommu *iommu)
 }
 
 /* This functionin only returns single iommu in a domain */
-static struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
+struct intel_iommu *domain_get_iommu(struct dmar_domain *domain)
 {
int iommu_id;
 
@@ -3500,7 +3446,7 @@ static unsigned long intel_alloc_iova(struct device *dev,
return iova_pfn;
 }
 
-static struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
+struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
 {
struct dmar_domain *domain, *tmp;
struct dmar_rmrr_unit *rmrr;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 6b5ef6c..a4463f0 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -385,6 +386,50 @@ struct pasid_entry;
 struct pasid_state_entry;
 struct page_req_dsc;
 
+struct dmar_domain {
+   int nid;/* node id */
+
+   unsigned int

[PATCH v2 1/9] iommu/vt-d: Global PASID name space

[Lu Baolu]

This adds the system wide PASID name space for the PASID
allocation. Currently we are using per IOMMU PASID name
spaces which are not suitable for some use cases. For an
example, one application (associated with a PASID) might
talk to two physical devices simultaneously while the two
devices could reside behind two different IOMMU units.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Suggested-by: Ashok Raj 
Signed-off-by: Lu Baolu 
Reviewed-by: Kevin Tian 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/Makefile  |  2 +-
 drivers/iommu/intel-iommu.c | 13 ++
 drivers/iommu/intel-pasid.c | 60 +
 drivers/iommu/intel-pasid.h | 30 +++
 4 files changed, 104 insertions(+), 1 deletion(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 1fb6958..0a190b4 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -14,7 +14,7 @@ obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
 obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
 obj-$(CONFIG_ARM_SMMU_V3) += arm-smmu-v3.o
 obj-$(CONFIG_DMAR_TABLE) += dmar.o
-obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o
+obj-$(CONFIG_INTEL_IOMMU) += intel-iommu.o intel-pasid.o
 obj-$(CONFIG_INTEL_IOMMU_SVM) += intel-svm.o
 obj-$(CONFIG_IPMMU_VMSA) += ipmmu-vmsa.o
 obj-$(CONFIG_IRQ_REMAP) += intel_irq_remapping.o irq_remapping.o
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..98c5ae9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -53,6 +53,7 @@
 #include 
 
 #include "irq_remapping.h"
+#include "intel-pasid.h"
 
 #define ROOT_SIZE  VTD_PAGE_SIZE
 #define CONTEXT_SIZE   VTD_PAGE_SIZE
@@ -3265,6 +3266,18 @@ static int __init init_dmars(void)
}
 
for_each_active_iommu(iommu, drhd) {
+   /*
+* Find the max pasid size of all IOMMU's in the system.
+* we need to ensure the system pasid table is no bigger
+* than the smallest supported.
+*/
+   if (pasid_enabled(iommu)) {
+   u32 temp = 2 << ecap_pss(iommu->ecap);
+
+   intel_pasid_max_id = min_t(u32, temp,
+  intel_pasid_max_id);
+   }
+
g_iommus[iommu->seq_id] = iommu;
 
intel_iommu_init_qi(iommu);
diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
new file mode 100644
index 000..0690f39
--- /dev/null
+++ b/drivers/iommu/intel-pasid.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * intel-pasid.c - PASID idr, table and entry manipulation
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ */
+
+#define pr_fmt(fmt)"DMAR: " fmt
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "intel-pasid.h"
+
+/*
+ * Intel IOMMU global PASID pool:
+ */
+static DEFINE_SPINLOCK(pasid_lock);
+u32 intel_pasid_max_id = PASID_MAX;
+static DEFINE_IDR(pasid_idr);
+
+int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp)
+{
+   int ret, min, max;
+
+   min = max_t(int, start, PASID_MIN);
+   max = min_t(int, end, intel_pasid_max_id);
+
+   WARN_ON(in_interrupt());
+   idr_preload(gfp);
+   spin_lock(_lock);
+   ret = idr_alloc(_idr, ptr, min, max, GFP_ATOMIC);
+   spin_unlock(_lock);
+   idr_preload_end();
+
+   return ret;
+}
+
+void intel_pasid_free_id(int pasid)
+{
+   spin_lock(_lock);
+   idr_remove(_idr, pasid);
+   spin_unlock(_lock);
+}
+
+void *intel_pasid_lookup_id(int pasid)
+{
+   void *p;
+
+   spin_lock(_lock);
+   p = idr_find(_idr, pasid);
+   spin_unlock(_lock);
+
+   return p;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
new file mode 100644
index 000..0c36af0
--- /dev/null
+++ b/drivers/iommu/intel-pasid.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * intel-pasid.h - PASID idr, table and entry header
+ *
+ * Copyright (C) 2018 Intel Corporation
+ *
+ * Author: Lu Baolu 
+ */
+
+#ifndef __INTEL_PASID_H
+#define __INTEL_PASID_H
+
+/*
+ * Eventually I'm promised we will get a multi-level PASID table
+ * and it won't have to be physically contiguous. Until then,
+ * limit the size because 8MiB contiguous allocations can be hard
+ * to come by. The limit of 0x2, which is 1MiB for each of
+ * the PASID and PASID-state tables, is somewhat arbitrary.
+ *
+ * PASID 0 is reserved in caching mode (virtualised IOMMU).
+ */
+#define PASID_MIN  0x1
+#define PASID_MAX  

[PATCH v2 3/9] iommu/vt-d: Use global PASID for SVM usage

[Lu Baolu]

This patch switches PASID management for SVM from per iommu
idr to the global idr.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Kevin Tian 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-svm.c   | 22 +++---
 include/linux/intel-iommu.h |  1 -
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 983af0c..24d0ea1 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -26,6 +26,8 @@
 #include 
 #include 
 
+#include "intel-pasid.h"
+
 #define PASID_ENTRY_P  BIT_ULL(0)
 #define PASID_ENTRY_FLPM_5LP   BIT_ULL(9)
 #define PASID_ENTRY_SREBIT_ULL(11)
@@ -85,8 +87,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
iommu->name);
}
 
-   idr_init(>pasid_idr);
-
return 0;
 }
 
@@ -102,7 +102,7 @@ int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
free_pages((unsigned long)iommu->pasid_state_table, order);
iommu->pasid_state_table = NULL;
}
-   idr_destroy(>pasid_idr);
+
return 0;
 }
 
@@ -392,9 +392,9 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
pasid_max = iommu->pasid_max;
 
/* Do not use PASID 0 in caching mode (virtualised IOMMU) */
-   ret = idr_alloc(>pasid_idr, svm,
-   !!cap_caching_mode(iommu->cap),
-   pasid_max - 1, GFP_KERNEL);
+   ret = intel_pasid_alloc_id(svm,
+  !!cap_caching_mode(iommu->cap),
+  pasid_max - 1, GFP_KERNEL);
if (ret < 0) {
kfree(svm);
kfree(sdev);
@@ -410,7 +410,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
if (mm) {
ret = mmu_notifier_register(>notifier, mm);
if (ret) {
-   idr_remove(>iommu->pasid_idr, svm->pasid);
+   intel_pasid_free_id(svm->pasid);
kfree(svm);
kfree(sdev);
goto out;
@@ -460,7 +460,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
if (!iommu || !iommu->pasid_table)
goto out;
 
-   svm = idr_find(>pasid_idr, pasid);
+   svm = intel_pasid_lookup_id(pasid);
if (!svm)
goto out;
 
@@ -485,7 +485,7 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
svm->iommu->pasid_table[svm->pasid].val 
= 0;
wmb();
 
-   idr_remove(>iommu->pasid_idr, 
svm->pasid);
+   intel_pasid_free_id(svm->pasid);
if (svm->mm)

mmu_notifier_unregister(>notifier, svm->mm);
 
@@ -520,7 +520,7 @@ int intel_svm_is_pasid_valid(struct device *dev, int pasid)
if (!iommu || !iommu->pasid_table)
goto out;
 
-   svm = idr_find(>pasid_idr, pasid);
+   svm = intel_pasid_lookup_id(pasid);
if (!svm)
goto out;
 
@@ -618,7 +618,7 @@ static irqreturn_t prq_event_thread(int irq, void *d)
 
if (!svm || svm->pasid != req->pasid) {
rcu_read_lock();
-   svm = idr_find(>pasid_idr, req->pasid);
+   svm = intel_pasid_lookup_id(req->pasid);
/* It *can't* go away, because the driver is not 
permitted
 * to unbind the mm while any page faults are 
outstanding.
 * So we only need RCU to protect the internal idr 
code. */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 795717e..6b5ef6c 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -418,7 +418,6 @@ struct intel_iommu {
struct pasid_state_entry *pasid_state_table;
struct page_req_dsc *prq;
unsigned char prq_name[16];/* Name for PRQ interrupt */
-   struct idr pasid_idr;
u32 pasid_max;
 #endif
struct q_inval  *qi;/* Queued invalidation info */
-- 
2.7.4

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

[PATCH v2 2/9] iommu/vt-d: Decouple idr bond pointer from svm

[Lu Baolu]

As we move the PASID idr out of SVM code and make it serving
as a global PASID name space, the consumer can specify a ptr
to bind it with a PASID. We shouldn't assume that each PASID
will be bond with a ptr of struct intel_svm anymore.

This patch cleans up a idr_for_each_entry() usage in the SVM
code. It's required to replace the SVM-specific idr with the
global PASID idr.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Kevin Tian 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-svm.c   | 14 ++
 include/linux/intel-iommu.h |  1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e8cd984..983af0c 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -298,6 +298,7 @@ static const struct mmu_notifier_ops intel_mmuops = {
 };
 
 static DEFINE_MUTEX(pasid_mutex);
+static LIST_HEAD(global_svm_list);
 
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct 
svm_dev_ops *ops)
 {
@@ -329,13 +330,13 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
 
mutex_lock(_mutex);
if (pasid && !(flags & SVM_FLAG_PRIVATE_PASID)) {
-   int i;
+   struct intel_svm *t;
 
-   idr_for_each_entry(>pasid_idr, svm, i) {
-   if (svm->mm != mm ||
-   (svm->flags & SVM_FLAG_PRIVATE_PASID))
+   list_for_each_entry(t, _svm_list, list) {
+   if (t->mm != mm || (t->flags & SVM_FLAG_PRIVATE_PASID))
continue;
 
+   svm = t;
if (svm->pasid >= pasid_max) {
dev_warn(dev,
 "Limited PASID width. Cannot use 
existing PASID %d\n",
@@ -404,6 +405,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
svm->mm = mm;
svm->flags = flags;
INIT_LIST_HEAD_RCU(>devs);
+   INIT_LIST_HEAD(>list);
ret = -ENOMEM;
if (mm) {
ret = mmu_notifier_register(>notifier, mm);
@@ -430,6 +432,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
 */
if (cap_caching_mode(iommu->cap))
intel_flush_pasid_dev(svm, sdev, svm->pasid);
+
+   list_add_tail(>list, _svm_list);
}
list_add_rcu(>list, >devs);
 
@@ -485,6 +489,8 @@ int intel_svm_unbind_mm(struct device *dev, int pasid)
if (svm->mm)

mmu_notifier_unregister(>notifier, svm->mm);
 
+   list_del(>list);
+
/* We mandate that no page faults may 
be outstanding
 * for the PASID when 
intel_svm_unbind_mm() is called.
 * If that is not obeyed, subtle errors 
will happen.
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ef169d6..795717e 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -486,6 +486,7 @@ struct intel_svm {
int flags;
int pasid;
struct list_head devs;
+   struct list_head list;
 };
 
 extern int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct 
intel_svm_dev *sdev);
-- 
2.7.4

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

[PATCH v2 0/9] iommu/vt-d: Improve PASID id and table management

[Lu Baolu]

This patch set improves the PASID id and PASID table management
for Intel IOMMU driver.

PATCH 1~3 replace per IOMMU idr name space with a global one.
Current per IOMMU idr doesn't work in some cases where one
application (associated with a PASID) might talk to two physical
devices simultaneously while the two devices could reside behind
two different IOMMU units.

PATCH 4~9 implement per domain PASID table. Current per IOMMU
PASID table implementation is insecure in the cases where
multiple devices under one single IOMMU unit support PASID
feature. With per domain PASID table, we can achieve finer
protection and isolation granularity.

Best regards,
Lu Baolu

Change log:
v1->v2:
  - Patches have been reviewed by "Liu Yi L ".
  - An error case handling was added in PATCH 6/9.
  - Some commit messages are refined to be more accurate.

Lu Baolu (9):
  iommu/vt-d: Global PASID name space
  iommu/vt-d: Decouple idr bond pointer from svm
  iommu/vt-d: Use global PASID for SVM usage
  iommu/vt-d: Move device_domain_info to header
  iommu/vt-d: Per domain pasid table interfaces
  iommu/vt-d: Allocate and free pasid table
  iommu/vt-d: Calculate PTS value
  iommu/vt-d: Use per-domain pasid table
  iommu/vt-d: Clean up PASID talbe management for SVM

 drivers/iommu/Makefile  |   2 +-
 drivers/iommu/intel-iommu.c | 128 -
 drivers/iommu/intel-pasid.c | 135 
 drivers/iommu/intel-pasid.h |  34 +++
 drivers/iommu/intel-svm.c   |  98 
 include/linux/intel-iommu.h |  90 +++--
 6 files changed, 352 insertions(+), 135 deletions(-)
 create mode 100644 drivers/iommu/intel-pasid.c
 create mode 100644 drivers/iommu/intel-pasid.h

-- 
2.7.4

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

[PATCH v2 7/9] iommu/vt-d: Calculate PTS value

[Lu Baolu]

Calculate PTS (PASID Table Size) value for the extended
context entry from the real size of the PASID table for
a domain.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-iommu.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f86302d..5602ccd 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5152,22 +5152,16 @@ static void intel_iommu_put_resv_regions(struct device 
*dev,
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
 #define MAX_NR_PASID_BITS (20)
-static inline unsigned long intel_iommu_get_pts(struct intel_iommu *iommu)
+static inline unsigned long intel_iommu_get_pts(struct dmar_domain *domain)
 {
-   /*
-* Convert ecap_pss to extend context entry pts encoding, also
-* respect the soft pasid_max value set by the iommu.
-* - number of PASID bits = ecap_pss + 1
-* - number of PASID table entries = 2^(pts + 5)
-* Therefore, pts = ecap_pss - 4
-* e.g. KBL ecap_pss = 0x13, PASID has 20 bits, pts = 15
-*/
-   if (ecap_pss(iommu->ecap) < 5)
+   int pts;
+
+   pts = find_first_bit((unsigned long *)>max_pasid,
+MAX_NR_PASID_BITS);
+   if (pts < 5)
return 0;
 
-   /* pasid_max is encoded as actual number of entries not the bits */
-   return find_first_bit((unsigned long *)>pasid_max,
-   MAX_NR_PASID_BITS) - 5;
+   return pts - 5;
 }
 
 int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct intel_svm_dev 
*sdev)
@@ -5204,7 +5198,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, 
struct intel_svm_dev *sd
if (iommu->pasid_state_table)
context[1].hi = 
(u64)virt_to_phys(iommu->pasid_state_table);
context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
-   intel_iommu_get_pts(iommu);
+   intel_iommu_get_pts(domain);
 
wmb();
/* CONTEXT_TT_MULTI_LEVEL and CONTEXT_TT_DEV_IOTLB are both
-- 
2.7.4

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

[PATCH v2 8/9] iommu/vt-d: Use per-domain pasid table

[Lu Baolu]

This patch replaces current per iommu pasid table with
the new added per domain pasid table. Each svm-capable
PCI device will be configured with a pasid table which
shares with other svm-capable devices within its iommu
domain.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-iommu.c |  6 +++---
 drivers/iommu/intel-svm.c   | 37 +
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5602ccd..fa6052a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5197,7 +5197,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, 
struct intel_svm_dev *sd
if (!(ctx_lo & CONTEXT_PASIDE)) {
if (iommu->pasid_state_table)
context[1].hi = 
(u64)virt_to_phys(iommu->pasid_state_table);
-   context[1].lo = (u64)virt_to_phys(iommu->pasid_table) |
+   context[1].lo = (u64)virt_to_phys(domain->pasid_table) |
intel_iommu_get_pts(domain);
 
wmb();
@@ -5265,8 +5265,8 @@ struct intel_iommu *intel_svm_device_to_iommu(struct 
device *dev)
return NULL;
}
 
-   if (!iommu->pasid_table) {
-   dev_err(dev, "PASID not enabled on IOMMU; cannot enable SVM\n");
+   if (!intel_pasid_get_table(dev)) {
+   dev_err(dev, "No PASID table for device; cannot enable SVM\n");
return NULL;
}
 
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 3abc94f..3b14819 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -256,6 +256,7 @@ static void intel_flush_pasid_dev(struct intel_svm *svm, 
struct intel_svm_dev *s
 static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 {
struct intel_svm *svm = container_of(mn, struct intel_svm, notifier);
+   struct pasid_entry *pasid_table;
struct intel_svm_dev *sdev;
 
/* This might end up being called from exit_mmap(), *before* the page
@@ -270,11 +271,16 @@ static void intel_mm_release(struct mmu_notifier *mn, 
struct mm_struct *mm)
 * page) so that we end up taking a fault that the hardware really
 * *has* to handle gracefully without affecting other processes.
 */
-   svm->iommu->pasid_table[svm->pasid].val = 0;
-   wmb();
-
rcu_read_lock();
list_for_each_entry_rcu(sdev, >devs, list) {
+   pasid_table = intel_pasid_get_table(sdev->dev);
+   if (!pasid_table)
+   continue;
+
+   pasid_table[svm->pasid].val = 0;
+   /* Make sure the entry update is visible before translation. */
+   wmb();
+
intel_flush_pasid_dev(svm, sdev, svm->pasid);
intel_flush_svm_range_dev(svm, sdev, 0, -1, 0, !svm->mm);
}
@@ -295,6 +301,7 @@ static LIST_HEAD(global_svm_list);
 int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct 
svm_dev_ops *ops)
 {
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
+   struct pasid_entry *pasid_table;
struct intel_svm_dev *sdev;
struct intel_svm *svm = NULL;
struct mm_struct *mm = NULL;
@@ -302,7 +309,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
int pasid_max;
int ret;
 
-   if (WARN_ON(!iommu || !iommu->pasid_table))
+   pasid_table = intel_pasid_get_table(dev);
+   if (WARN_ON(!iommu || !pasid_table))
return -EINVAL;
 
if (dev_is_pci(dev)) {
@@ -380,8 +388,8 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
}
svm->iommu = iommu;
 
-   if (pasid_max > iommu->pasid_max)
-   pasid_max = iommu->pasid_max;
+   if (pasid_max > intel_pasid_max_id)
+   pasid_max = intel_pasid_max_id;
 
/* Do not use PASID 0 in caching mode (virtualised IOMMU) */
ret = intel_pasid_alloc_id(svm,
@@ -414,7 +422,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int 
flags, struct svm_dev_
if (cpu_feature_enabled(X86_FEATURE_LA57))
pasid_entry_val |= PASID_ENTRY_FLPM_5LP;
 
-   iommu->pasid_table[svm->pasid].val = pasid_entry_val;
+   pasid_table[svm->pasid].val = pasid_entry_val;
 
wmb();
 
@@ -442,6 +450,7 @@ EXPORT_SYMBOL_GPL(intel_svm_bind_mm);
 
 int intel_svm_unbind_mm(struct device *dev, int pasid)
 {
+   struct pasid_entry *pasid_table;
struct intel_svm_dev *sdev;
struct intel_iommu *iommu;

[PATCH v2 5/9] iommu/vt-d: Per domain pasid table interfaces

[Lu Baolu]

This patch adds the interfaces for per domain pasid table
management. Currently we allocate one pasid table for all
devices under the scope of an IOMMU. It's insecure in the
cases where multiple devices under one single IOMMU unit
support PASID feature. With per domain pasid table, we can
achieve finer protection and isolation granularity.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Suggested-by: Ashok Raj 
Signed-off-by: Lu Baolu 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-pasid.c | 75 +
 drivers/iommu/intel-pasid.h |  4 +++
 include/linux/intel-iommu.h |  5 +++
 3 files changed, 84 insertions(+)

diff --git a/drivers/iommu/intel-pasid.c b/drivers/iommu/intel-pasid.c
index 0690f39..b8691a6 100644
--- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "intel-pasid.h"
@@ -58,3 +59,77 @@ void *intel_pasid_lookup_id(int pasid)
 
return p;
 }
+
+/*
+ * Interfaces for per domain pasid table management:
+ */
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+   size_t entry_count)
+{
+   struct device_domain_info *info;
+   struct dmar_domain *domain;
+   struct page *pages;
+   int order;
+
+   info = dev->archdata.iommu;
+   if (WARN_ON(!info || !dev_is_pci(dev) ||
+   !info->pasid_supported ||
+   !info->domain))
+   return -EINVAL;
+
+   domain = info->domain;
+
+   if (entry_count > intel_pasid_max_id)
+   entry_count = intel_pasid_max_id;
+
+   order = get_order(entry_size * entry_count);
+   pages = alloc_pages_node(domain->nid, GFP_KERNEL | __GFP_ZERO, order);
+   if (!pages)
+   return -ENOMEM;
+
+   spin_lock(_lock);
+   if (domain->pasid_table) {
+   __free_pages(pages, order);
+   } else {
+   domain->pasid_table = page_address(pages);
+   domain->order   = order;
+   domain->max_pasid   = entry_count;
+   }
+   domain->pasid_users++;
+   spin_unlock(_lock);
+
+   return 0;
+}
+
+void intel_pasid_free_table(struct device *dev)
+{
+   struct dmar_domain *domain;
+
+   domain = get_valid_domain_for_dev(dev);
+   if (!domain || !dev_is_pci(dev))
+   return;
+
+   spin_lock(_lock);
+   if (domain->pasid_table) {
+   domain->pasid_users--;
+   if (!domain->pasid_users) {
+   free_pages((unsigned long)domain->pasid_table,
+  domain->order);
+   domain->pasid_table = NULL;
+   domain->order   = 0;
+   domain->max_pasid   = 0;
+   }
+   }
+   spin_unlock(_lock);
+}
+
+void *intel_pasid_get_table(struct device *dev)
+{
+   struct dmar_domain *domain;
+
+   domain = get_valid_domain_for_dev(dev);
+   if (!domain)
+   return NULL;
+
+   return domain->pasid_table;
+}
diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
index 0c36af0..a90c60b 100644
--- a/drivers/iommu/intel-pasid.h
+++ b/drivers/iommu/intel-pasid.h
@@ -26,5 +26,9 @@ extern u32 intel_pasid_max_id;
 int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
 void intel_pasid_free_id(int pasid);
 void *intel_pasid_lookup_id(int pasid);
+int intel_pasid_alloc_table(struct device *dev, size_t entry_size,
+   size_t entry_count);
+void intel_pasid_free_table(struct device *dev);
+void *intel_pasid_get_table(struct device *dev);
 
 #endif /* __INTEL_PASID_H */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index a4463f0..bee7a3f 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -424,6 +424,11 @@ struct dmar_domain {
 */
u64 max_addr;   /* maximum mapped address */
 
+   void*pasid_table;   /* pointer of pasid table */
+   unsigned intpasid_users;/* User number of pasid table */
+   int order;  /* the page order of tables */
+   int max_pasid;  /* max pasid */
+
struct iommu_domain domain; /*
 * generic domain data structure for
 * iommu core
-- 
2.7.4

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

[PATCH v2 9/9] iommu/vt-d: Clean up PASID talbe management for SVM

[Lu Baolu]

The previous per iommu pasid table alloc/free interfaces
are no longer used. Clean up the driver by removing it.

Cc: Ashok Raj 
Cc: Jacob Pan 
Cc: Kevin Tian 
Cc: Liu Yi L 
Signed-off-by: Lu Baolu 
Reviewed-by: Liu Yi L 
---
 drivers/iommu/intel-iommu.c |  6 +++---
 drivers/iommu/intel-svm.c   | 17 ++---
 include/linux/intel-iommu.h |  5 ++---
 3 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index fa6052a..010871d 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1736,7 +1736,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
if (pasid_enabled(iommu)) {
if (ecap_prs(iommu->ecap))
intel_svm_finish_prq(iommu);
-   intel_svm_free_pasid_tables(iommu);
+   intel_svm_exit(iommu);
}
 #endif
 }
@@ -3297,7 +3297,7 @@ static int __init init_dmars(void)
hw_pass_through = 0;
 #ifdef CONFIG_INTEL_IOMMU_SVM
if (pasid_enabled(iommu))
-   intel_svm_alloc_pasid_tables(iommu);
+   intel_svm_init(iommu);
 #endif
}
 
@@ -4274,7 +4274,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
if (pasid_enabled(iommu))
-   intel_svm_alloc_pasid_tables(iommu);
+   intel_svm_init(iommu);
 #endif
 
if (dmaru->ignored) {
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index 3b14819..38cae65 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -34,7 +34,7 @@
 
 static irqreturn_t prq_event_thread(int irq, void *d);
 
-int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_init(struct intel_iommu *iommu)
 {
struct page *pages;
int order;
@@ -59,15 +59,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
iommu->pasid_max = 0x2;
 
order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
-   pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
-   if (!pages) {
-   pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
-   iommu->name);
-   return -ENOMEM;
-   }
-   iommu->pasid_table = page_address(pages);
-   pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
-
if (ecap_dis(iommu->ecap)) {
/* Just making it explicit... */
BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct 
pasid_state_entry));
@@ -82,14 +73,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
return 0;
 }
 
-int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_exit(struct intel_iommu *iommu)
 {
int order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
 
-   if (iommu->pasid_table) {
-   free_pages((unsigned long)iommu->pasid_table, order);
-   iommu->pasid_table = NULL;
-   }
if (iommu->pasid_state_table) {
free_pages((unsigned long)iommu->pasid_state_table, order);
iommu->pasid_state_table = NULL;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 08e5811..44c7613 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -470,7 +470,6 @@ struct intel_iommu {
 * devices away to userspace processes (e.g. for DPDK) and don't
 * want to trust that userspace will use *only* the PASID it was
 * told to. But while it's all driver-arbitrated, we're fine. */
-   struct pasid_entry *pasid_table;
struct pasid_state_entry *pasid_state_table;
struct page_req_dsc *prq;
unsigned char prq_name[16];/* Name for PRQ interrupt */
@@ -539,8 +538,8 @@ void free_pgtable_page(void *vaddr);
 struct intel_iommu *domain_get_iommu(struct dmar_domain *domain);
 
 #ifdef CONFIG_INTEL_IOMMU_SVM
-extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
-extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
+int intel_svm_init(struct intel_iommu *iommu);
+int intel_svm_exit(struct intel_iommu *iommu);
 extern int intel_svm_enable_prq(struct intel_iommu *iommu);
 extern int intel_svm_finish_prq(struct intel_iommu *iommu);
 
-- 
2.7.4

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

Re: [PATCH v2 2/3] intel-iommu: generalize __mapping_notify_one()

[Peter Xu]

On Thu, May 03, 2018 at 03:14:24PM +0200, Joerg Roedel wrote:
> On Wed, Apr 18, 2018 at 04:39:52PM +0800, Peter Xu wrote:
> > Generalize this new helper to notify one newly created mapping on one
> > single IOMMU.  We can further leverage this helper in the next patch.
> 
> You introduce the function, you do not generalize it. Please fix that in
> the subject and description.
> 
> Also, please drop patch 1, I don't like this pr_debug stuff in the
> code. And please also fix the subject line in general to match the form:
> 
>   iommu/vt-d: _I_ntroduce __mapping_notify_one()
> 
> When that is done, please re-send.

Will do.  Thanks, Joerg.

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

[PATCH v3 2/2] iommu/vt-d: Fix iotlb psi missing for mappings

[Peter Xu]

When caching mode is enabled for IOMMU, we should send explicit IOTLB
PSIs even for newly created mappings.  However these events are missing
for all intel_iommu_map() callers, e.g., iommu_map().  One direct user
is the vfio-pci driver.

To make sure we'll send the PSIs always when necessary, this patch
firstly introduced domain_mapping() helper for page mappings, then fixed
the problem by generalizing the explicit map IOTLB PSI logic into that
new helper. With that, we let iommu_domain_identity_map() to use the
simplified version to avoid sending the notifications, while for all the
rest of cases we send the notifications always.

For VM case, we send the PSIs to all the backend IOMMUs for the domain.

This patch allows the nested device assignment to work with QEMU (assign
device firstly to L1 guest, then assign it again to L2 guest).

Signed-off-by: Peter Xu 
---
 drivers/iommu/intel-iommu.c | 43 +
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 13190a54aba2..601d3789211f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2352,18 +2352,47 @@ static int __domain_mapping(struct dmar_domain *domain, 
unsigned long iov_pfn,
return 0;
 }
 
+static int domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
+ struct scatterlist *sg, unsigned long phys_pfn,
+ unsigned long nr_pages, int prot)
+{
+   int ret;
+   struct intel_iommu *iommu;
+
+   /* Do the real mapping first */
+   ret = __domain_mapping(domain, iov_pfn, sg, phys_pfn, nr_pages, prot);
+   if (ret)
+   return ret;
+
+   /* Notify about the new mapping */
+   if (domain_type_is_vm(domain)) {
+  /* VM typed domains can have more than one IOMMUs */
+  int iommu_id;
+  for_each_domain_iommu(iommu_id, domain) {
+  iommu = g_iommus[iommu_id];
+  __mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
+  }
+   } else {
+  /* General domains only have one IOMMU */
+  iommu = domain_get_iommu(domain);
+  __mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
+   }
+
+   return 0;
+}
+
 static inline int domain_sg_mapping(struct dmar_domain *domain, unsigned long 
iov_pfn,
struct scatterlist *sg, unsigned long 
nr_pages,
int prot)
 {
-   return __domain_mapping(domain, iov_pfn, sg, 0, nr_pages, prot);
+   return domain_mapping(domain, iov_pfn, sg, 0, nr_pages, prot);
 }
 
 static inline int domain_pfn_mapping(struct dmar_domain *domain, unsigned long 
iov_pfn,
 unsigned long phys_pfn, unsigned long 
nr_pages,
 int prot)
 {
-   return __domain_mapping(domain, iov_pfn, NULL, phys_pfn, nr_pages, 
prot);
+   return domain_mapping(domain, iov_pfn, NULL, phys_pfn, nr_pages, prot);
 }
 
 static void domain_context_clear_one(struct intel_iommu *iommu, u8 bus, u8 
devfn)
@@ -2668,9 +2697,9 @@ static int iommu_domain_identity_map(struct dmar_domain 
*domain,
 */
dma_pte_clear_range(domain, first_vpfn, last_vpfn);
 
-   return domain_pfn_mapping(domain, first_vpfn, first_vpfn,
- last_vpfn - first_vpfn + 1,
- DMA_PTE_READ|DMA_PTE_WRITE);
+   return __domain_mapping(domain, first_vpfn, NULL,
+   first_vpfn, last_vpfn - first_vpfn + 1,
+   DMA_PTE_READ|DMA_PTE_WRITE);
 }
 
 static int domain_prepare_identity_map(struct device *dev,
@@ -3637,8 +3666,6 @@ static dma_addr_t __intel_map_single(struct device *dev, 
phys_addr_t paddr,
if (ret)
goto error;
 
-   __mapping_notify_one(iommu, domain, mm_to_dma_pfn(iova_pfn), size);
-
start_paddr = (phys_addr_t)iova_pfn << PAGE_SHIFT;
start_paddr += paddr & ~PAGE_MASK;
return start_paddr;
@@ -3825,8 +3852,6 @@ static int intel_map_sg(struct device *dev, struct 
scatterlist *sglist, int nele
return 0;
}
 
-   __mapping_notify_one(iommu, domain, start_vpfn, size);
-
return nelems;
 }
 
-- 
2.17.0

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

[PATCH v3 0/2] iommu/vt-d: Fix mapping PSI missing for iommu_map()

[Peter Xu]

v3:
- drop the pr_debug patch [Joerg]
- rename all the subjects as suggested [Joerg]
- rebase

v2:
- cc correct people and iommu list

(PSI stands for: Page Selective Invalidations)

Intel IOMMU has the caching mode to ease emulation of the device.
When that bit is set, we need to send PSIs even for newly mapped
pages.  However current driver is not fully obey the rule.  E.g.,
iommu_map() API will only do the mapping but it never sent the PSIs
before.  That can be problematic to emulated IOMMU devices since
they'll never be able to build up the shadow page tables if without
such information.  This patchset tries to fix the problem.

Patch 1 introduces a helper to notify the MAP PSIs.

Patch 2 fixes the real problem by making sure every domain mapping
will trigger the MAP PSI notifications.

Without the patchset, nested device assignment (assign one device
firstly to L1 guest, then to L2 guest) won't work for QEMU.  After
applying the patchset, it works.

Please review.  Thanks.

Peter Xu (2):
  iommu/vt-d: Introduce __mapping_notify_one()
  iommu/vt-d: Fix iotlb psi missing for mappings

 drivers/iommu/intel-iommu.c | 65 ++---
 1 file changed, 46 insertions(+), 19 deletions(-)

-- 
2.17.0

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

[PATCH v3 1/2] iommu/vt-d: Introduce __mapping_notify_one()

[Peter Xu]

Introduce this new helper to notify one newly created mapping on one
single IOMMU.  We can further leverage this helper in the next patch.

Signed-off-by: Peter Xu 
---
 drivers/iommu/intel-iommu.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f235346..13190a54aba2 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1606,6 +1606,18 @@ static void iommu_flush_iotlb_psi(struct intel_iommu 
*iommu,
iommu_flush_dev_iotlb(domain, addr, mask);
 }
 
+/* Notification for newly created mappings */
+static inline void __mapping_notify_one(struct intel_iommu *iommu,
+   struct dmar_domain *domain,
+   unsigned long pfn, unsigned int pages)
+{
+   /* It's a non-present to present mapping. Only flush if caching mode */
+   if (cap_caching_mode(iommu->cap))
+   iommu_flush_iotlb_psi(iommu, domain, pfn, pages, 0, 1);
+   else
+   iommu_flush_write_buffer(iommu);
+}
+
 static void iommu_flush_iova(struct iova_domain *iovad)
 {
struct dmar_domain *domain;
@@ -3625,13 +3637,7 @@ static dma_addr_t __intel_map_single(struct device *dev, 
phys_addr_t paddr,
if (ret)
goto error;
 
-   /* it's a non-present to present mapping. Only flush if caching mode */
-   if (cap_caching_mode(iommu->cap))
-   iommu_flush_iotlb_psi(iommu, domain,
- mm_to_dma_pfn(iova_pfn),
- size, 0, 1);
-   else
-   iommu_flush_write_buffer(iommu);
+   __mapping_notify_one(iommu, domain, mm_to_dma_pfn(iova_pfn), size);
 
start_paddr = (phys_addr_t)iova_pfn << PAGE_SHIFT;
start_paddr += paddr & ~PAGE_MASK;
@@ -3819,11 +3825,7 @@ static int intel_map_sg(struct device *dev, struct 
scatterlist *sglist, int nele
return 0;
}
 
-   /* it's a non-present to present mapping. Only flush if caching mode */
-   if (cap_caching_mode(iommu->cap))
-   iommu_flush_iotlb_psi(iommu, domain, start_vpfn, size, 0, 1);
-   else
-   iommu_flush_write_buffer(iommu);
+   __mapping_notify_one(iommu, domain, start_vpfn, size);
 
return nelems;
 }
-- 
2.17.0

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

Re: [PATCH] iommu/vt-d: fix usage of force parameter in intel_ir_reconfigure_irte()

[Joerg Roedel]

On Tue, Mar 06, 2018 at 05:39:41PM -0500, Jagannathan Raman wrote:
> It was noticed that the IRTE configured for guest OS kernel
> was over-written while the guest was running. As a result,
> vt-d Posted Interrupts configured for the guest are not being
> delivered directly, and instead bounces off the host. Every
> interrupt delivery takes a VM Exit.
> 
> It was noticed that the following stack is doing the over-write:
> [  147.463177]  modify_irte+0x171/0x1f0
> [  147.463405]  intel_ir_set_affinity+0x5c/0x80
> [  147.463641]  msi_domain_set_affinity+0x32/0x90
> [  147.463881]  irq_do_set_affinity+0x37/0xd0
> [  147.464125]  irq_set_affinity_locked+0x9d/0xb0
> [  147.464374]  __irq_set_affinity+0x42/0x70
> [  147.464627]  write_irq_affinity.isra.5+0xe1/0x110
> [  147.464895]  proc_reg_write+0x38/0x70
> [  147.465150]  __vfs_write+0x36/0x180
> [  147.465408]  ? handle_mm_fault+0xdf/0x200
> [  147.465671]  ? _cond_resched+0x15/0x30
> [  147.465936]  vfs_write+0xad/0x1a0
> [  147.466204]  SyS_write+0x52/0xc0
> [  147.466472]  do_syscall_64+0x74/0x1a0
> [  147.466744]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> 
> reversing the sense of force check in intel_ir_reconfigure_irte()
> restores proper posted interrupt functionality
> 
> Signed-off-by: Jagannathan Raman 

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

Re: [PATCH v6 4/7] iommu/dma: Move PCI window region reservation back into dma specific path.

[Joerg Roedel]

On Wed, Apr 18, 2018 at 12:40:42PM +0100, Shameer Kolothum wrote:
> This pretty much reverts commit 273df9635385 ("iommu/dma: Make PCI
> window reservation generic")  by moving the PCI window region
> reservation back into the dma specific path so that these regions
> doesn't get exposed via the IOMMU API interface. With this change,
> the vfio interface will report only iommu specific reserved regions
> to the user space.
> 
> Cc: Joerg Roedel 
> Signed-off-by: Shameer Kolothum 
> Reviewed-by: Robin Murphy 
> ---
>  drivers/iommu/dma-iommu.c | 54 
> ++-
>  1 file changed, 25 insertions(+), 29 deletions(-)

Applied to my iommu/fixes branch.

Thanks,

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

Re: [PATCH v4 1/2] dma-mapping: move dma configuration to bus infrastructure

[Christoph Hellwig]

On Mon, Apr 30, 2018 at 12:41:24PM +0200, Thierry Reding wrote:
> On Sat, Apr 28, 2018 at 08:21:58AM +0530, Nipun Gupta wrote:
> [...]
> > diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
> > index 88a3558..a9ec99d 100644
> > --- a/drivers/gpu/host1x/bus.c
> > +++ b/drivers/gpu/host1x/bus.c
> > @@ -314,6 +314,13 @@ static int host1x_device_match(struct device *dev, 
> > struct device_driver *drv)
> > return strcmp(dev_name(dev), drv->name) == 0;
> >  }
> >  
> > +static int host1x_dma_configure(struct device *dev)
> > +{
> > +   if (dev->of_node)
> > +   return of_dma_configure(dev, dev->of_node);
> 
> The conditional here is somewhat pointless since the of_node should
> always be set. If it weren't it should be considered a bug and this
> function welcome to crash to make that very obvious.

Agreed.  Please remove the check for the resend.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v4 1/2] dma-mapping: move dma configuration to bus infrastructure

[Christoph Hellwig]

This still fails to apply against either 4.17-rc3 or current Linus
master for me.  Please resend against it, and mention the tree it
is against.

Also please resend it in a separate email thread (all your patches
were replies to the previous ones) and include a cover letter.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: rockchip: fix building without CONFIG_OF

[Joerg Roedel]

On Wed, Apr 04, 2018 at 12:23:53PM +0200, Arnd Bergmann wrote:
> We get a build error when compiling the iommu driver without CONFIG_OF:
> 
> drivers/iommu/rockchip-iommu.c: In function 'rk_iommu_of_xlate':
> drivers/iommu/rockchip-iommu.c:1101:2: error: implicit declaration of 
> function 'of_dev_put'; did you mean 'of_node_put'? 
> [-Werror=implicit-function-declaration]
> 
> This replaces the of_dev_put() with the equivalent platform_device_put(),
> and adds an error check for of_find_device_by_node() returning NULL,
> which seems to be appropriate here given that we pass the device into
> platform_get_drvdata() next, and that of_find_device_by_node() might
> theoretically return a NULL pointer.
> 
> Fixes: 5fd577c3eac3 ("iommu/rockchip: Use OF_IOMMU to attach devices 
> automatically")
> Signed-off-by: Arnd Bergmann 
> ---
> This warning appears to only have been introduced in linux-next after
> the merge window opened.
> ---
>  drivers/iommu/rockchip-iommu.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Changed patch according to Robins comments and applied it, thanks.

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

Re: [PATCH v2 2/3] intel-iommu: generalize __mapping_notify_one()

[Joerg Roedel]

On Wed, Apr 18, 2018 at 04:39:52PM +0800, Peter Xu wrote:
> Generalize this new helper to notify one newly created mapping on one
> single IOMMU.  We can further leverage this helper in the next patch.

You introduce the function, you do not generalize it. Please fix that in
the subject and description.

Also, please drop patch 1, I don't like this pr_debug stuff in the
code. And please also fix the subject line in general to match the form:

iommu/vt-d: _I_ntroduce __mapping_notify_one()

When that is done, please re-send.

Thanks,

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

Re: [PATCH v3 09/20] iommu: Remove depends on HAS_DMA in case of platform dependency

[Joerg Roedel]

On Tue, Apr 17, 2018 at 07:49:09PM +0200, Geert Uytterhoeven wrote:
> Remove dependencies on HAS_DMA where a Kconfig symbol depends on another
> symbol that implies HAS_DMA, and, optionally, on "|| COMPILE_TEST".
> In most cases this other symbol is an architecture or platform specific
> symbol, or PCI.
> 
> Generic symbols and drivers without platform dependencies keep their
> dependencies on HAS_DMA, to prevent compiling subsystems or drivers that
> cannot work anyway.
> 
> This simplifies the dependencies, and allows to improve compile-testing.
> 
> Signed-off-by: Geert Uytterhoeven 
> Reviewed-by: Mark Brown 
> Acked-by: Robin Murphy 
> Acked-by: Joerg Roedel 

Applied, thanks.

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

Re: [PATCH 24/61] iommu: simplify getting .drvdata

[Joerg Roedel]

On Thu, Apr 19, 2018 at 04:05:54PM +0200, Wolfram Sang wrote:
> We should get drvdata from struct device directly. Going via
> platform_device is an unneeded step back and forth.
> 
> Signed-off-by: Wolfram Sang 
> ---
> 
> Build tested only. buildbot is happy. Please apply individually.
> 
>  drivers/iommu/qcom_iommu.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)

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

Re: [PATCHv4 1/2] iommu/vt-d: Ratelimit each dmar fault printing

[Joerg Roedel]

On Wed, May 02, 2018 at 03:22:24AM +0100, Dmitry Safonov wrote:
> Hi Joerg,
> 
> is there anything I may do about those two patches?
> In 2/2 I've limited loop cnt as discussed in v3.
> This one solves softlockup for us, might be useful.

Applied the first patch, thanks. Please re-work the second one according
to the comments.


Thanks,

Joerg

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

Re: [PATCH] iommu: amd: hide unused iommu_table_lock

[Joerg Roedel]

On Wed, Apr 04, 2018 at 12:56:59PM +0200, Arnd Bergmann wrote:
> The newly introduced lock is only used when CONFIG_IRQ_REMAP is enabled:
> 
> drivers/iommu/amd_iommu.c:86:24: error: 'iommu_table_lock' defined but not 
> used [-Werror=unused-variable]
>  static DEFINE_SPINLOCK(iommu_table_lock);
> 
> This moves the definition next to the user, within the #ifdef protected
> section of the file.
> 
> Fixes: ea6166f4b83e ("iommu/amd: Split irq_lookup_table out of the 
> amd_iommu_devtable_lock")
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/iommu/amd_iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks.

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

Re: [PATCH v1 0/4] Tegra GART fixes and improvements

[Joerg Roedel]

On Mon, Apr 09, 2018 at 11:07:18PM +0300, Dmitry Osipenko wrote:
> GART driver wasn't ever been utilized in upstream, but finally this should
> change sometime soon with Tegra's DRM driver rework. In general GART driver
> works fine, though there are couple things that could be improved.
> 
> Dmitry Osipenko (4):
>   iommu/tegra: gart: Add debugging facility
>   iommu/tegra: gart: Fix gart_iommu_unmap()

Applied these two patches, thanks.

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

Re: [PATCH v2] iommu/rockchip: make clock handling optional

[Joerg Roedel]

On Tue, May 01, 2018 at 11:57:29AM +0200, Heiko Stuebner wrote:
> Ho Joerg,
> 
> Am Dienstag, 17. April 2018, 14:09:15 CEST schrieb Heiko Stuebner:
> > iommu clocks are optional, so the driver should not fail if they are not
> > present. Instead just set the number of clocks to 0, which the clk-blk APIs
> > can handle just fine.
> > 
> > Fixes: f2e3a5f557ad ("iommu/rockchip: Control clocks needed to access the 
> > IOMMU")
> > Signed-off-by: Heiko Stuebner 
> > Reviewed-by: Robin Murphy 
> 
> could you pick up this patch as fix for 4.17 please?
> As without it, we loose display output with old devicetrees.

Applied to the iommu/fixes branch, thanks.

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

Re: [PATCH v4 05/22] iommu: introduce iommu invalidate API function

[Jacob Pan]

On Wed, 2 May 2018 10:31:50 +0100
Jean-Philippe Brucker  wrote:

> On 01/05/18 23:58, Jacob Pan wrote:
>  Maybe this should be called "NG_PAGE_PASID",
> >>> Sure. I was thinking page range already implies non-global
> >>> pages.
>  and "DOMAIN_PAGE" should
>  instead be "PAGE_PASID". If I understood their meaning correctly,
>  it would be more consistent with the rest.
> 
> >>> I am trying not to mix granu between request w/ PASID and w/o.
> >>> DOMAIN_PAGE meant to be for request w/o PASID.
> >>
> >> Is the distinction necessary? I understand the IOMMU side might
> >> offer many possibilities for invalidation, but the user probably
> >> doesn't need all of them. It might be easier to document, upstream
> >> and maintain if we only specify what's currently needed by users
> >> (what does QEMU VT-d use?) Others can always extend it by
> >> increasing the version.
> >>
> >> Do you think that this invalidation message will be used outside of
> >> BIND_PASID_TABLE context? I can't see an other use but who knows.
> >> At the moment requests w/o PASID are managed with
> >> VFIO_IOMMU_MAP/UNMAP_DMA, which doesn't require invalidation. And
> >> in a BIND_PASID_TABLE context, IOMMUs requests w/o PASID are just a
> >> special case using PASID 0 (for Arm and AMD) so I suppose they'll
> >> use the same invalidation commands as requests w/ PASID.
> >>  
> > My understanding is that for GIOVA use case, VT-d vIOMMU creates
> > GIOVA-GPA mapping and the host shadows the 2nd level page tables to
> > create GIOVA-HPA mapping. So when assigned device in the guest can
> > do both DMA map/unmap and VFIO map/unmap, VFIO unmap is one time
> > deal (I guess invalidation can be captured in other code path), but
> > guest kernel use of DMA unmap could will trigger invalidation. QEMU
> > needs to trap those invalidation and passdown to physical IOMMU. So
> > we do need invalidation w/o PASID.  
> 
> Hm, isn't this all done by host userspace? Whether guest does DMA
> map/unmap or VFIO map/unmap, it creates/removes IOVA-GPA mappings in
> the vIOMMU. QEMU captures invalidation requests for these mappings
> from the guest, finds GPA-HVA in the shadow map and sends a VFIO
> map/unmap request for IOVA-HVA.
> 
Sorry for the delay but you are right, I have also confirmed with Yi
that we don't need second level invalidation. I will remove IOTLB
invalidation w/o PASID case from the API.

Thanks,

> Thanks,
> Jean
> 

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

[PATCH 2/4] iommu/vt-d: Clean up unused variable in find_or_alloc_domain

[Lu Baolu]

Remove it to make the code more concise.

Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..9064607 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2533,7 +2533,7 @@ static struct dmar_domain *find_or_alloc_domain(struct 
device *dev, int gaw)
struct device_domain_info *info = NULL;
struct dmar_domain *domain = NULL;
struct intel_iommu *iommu;
-   u16 req_id, dma_alias;
+   u16 dma_alias;
unsigned long flags;
u8 bus, devfn;
 
@@ -2541,8 +2541,6 @@ static struct dmar_domain *find_or_alloc_domain(struct 
device *dev, int gaw)
if (!iommu)
return NULL;
 
-   req_id = ((u16)bus << 8) | devfn;
-
if (dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(dev);
 
-- 
2.7.4

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

[PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices

[Lu Baolu]

The pasid28 quirk is needed only for some pre-production devices.
Remove it to make the code concise.

Signed-off-by: Ashok Raj 
Signed-off-by: Lu Baolu 
---
 drivers/iommu/intel-iommu.c | 32 ++--
 include/linux/intel-iommu.h |  1 -
 2 files changed, 2 insertions(+), 31 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9064607..10bce33 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -485,37 +485,14 @@ static int dmar_forcedac;
 static int intel_iommu_strict;
 static int intel_iommu_superpage = 1;
 static int intel_iommu_ecs = 1;
-static int intel_iommu_pasid28;
 static int iommu_identity_mapping;
 
 #define IDENTMAP_ALL   1
 #define IDENTMAP_GFX   2
 #define IDENTMAP_AZALIA4
 
-/* Broadwell and Skylake have broken ECS support — normal so-called "second
- * level" translation of DMA requests-without-PASID doesn't actually happen
- * unless you also set the NESTE bit in an extended context-entry. Which of
- * course means that SVM doesn't work because it's trying to do nested
- * translation of the physical addresses it finds in the process page tables,
- * through the IOVA->phys mapping found in the "second level" page tables.
- *
- * The VT-d specification was retroactively changed to change the definition
- * of the capability bits and pretend that Broadwell/Skylake never happened...
- * but unfortunately the wrong bit was changed. It's ECS which is broken, but
- * for some reason it was the PASID capability bit which was redefined (from
- * bit 28 on BDW/SKL to bit 40 in future).
- *
- * So our test for ECS needs to eschew those implementations which set the old
- * PASID capabiity bit 28, since those are the ones on which ECS is broken.
- * Unless we are working around the 'pasid28' limitations, that is, by putting
- * the device into passthrough mode for normal DMA and thus masking the bug.
- */
-#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap) && \
-   (intel_iommu_pasid28 || 
!ecap_broken_pasid(iommu->ecap)))
-/* PASID support is thus enabled if ECS is enabled and *either* of the old
- * or new capability bits are set. */
-#define pasid_enabled(iommu) (ecs_enabled(iommu) &&\
- (ecap_pasid(iommu->ecap) || 
ecap_broken_pasid(iommu->ecap)))
+#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap))
+#define pasid_enabled(iommu)   (ecs_enabled(iommu) && ecap_pasid(iommu->ecap))
 
 int intel_iommu_gfx_mapped;
 EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -578,11 +555,6 @@ static int __init intel_iommu_setup(char *str)
printk(KERN_INFO
"Intel-IOMMU: disable extended context table 
support\n");
intel_iommu_ecs = 0;
-   } else if (!strncmp(str, "pasid28", 7)) {
-   printk(KERN_INFO
-   "Intel-IOMMU: enable pre-production PASID 
support\n");
-   intel_iommu_pasid28 = 1;
-   iommu_identity_mapping |= IDENTMAP_GFX;
} else if (!strncmp(str, "tboot_noforce", 13)) {
printk(KERN_INFO
"Intel-IOMMU: not forcing on after tboot. This 
could expose security risk for tboot\n");
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index ef169d6..1df9401 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -121,7 +121,6 @@
 #define ecap_srs(e)((e >> 31) & 0x1)
 #define ecap_ers(e)((e >> 30) & 0x1)
 #define ecap_prs(e)((e >> 29) & 0x1)
-#define ecap_broken_pasid(e)   ((e >> 28) & 0x1)
 #define ecap_dis(e)((e >> 27) & 0x1)
 #define ecap_nest(e)   ((e >> 26) & 0x1)
 #define ecap_mts(e)((e >> 25) & 0x1)
-- 
2.7.4

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

[PATCH 0/4] iommu/vt-d: Several cleanup patches

[Lu Baolu]

Hi,

This includes several cleanup patches which aim to make the
code more concise and easier for reading. There aren't any
functionality changes.

Best regards,
Lu Baolu

Lu Baolu (4):
  iommu: Clean up the comments for iommu_group_alloc
  iommu/vt-d: Clean up unused variable in find_or_alloc_domain
  iommu/vt-d: Clean up pasid quirk for pre-production devices
  iommu/vt-d: Remove unnecessary parentheses

 drivers/iommu/intel-iommu.c | 36 +++-
 drivers/iommu/intel-svm.c   |  2 +-
 drivers/iommu/iommu.c   |  1 -
 include/linux/intel-iommu.h |  1 -
 4 files changed, 4 insertions(+), 36 deletions(-)

-- 
2.7.4

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