Re: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node

2022-05-04 Thread Hanjun Guo via iommu

On 2022/5/4 0:33, Shameer Kolothum wrote:

Hi

v11 --> v12
   -Minor fix in patch #4 to address the issue reported by the kernel test 
robot.
   -Added R-by tags by Christoph(patch #1) and Lorenzo(patch #4).
   -Added T-by from Steve to all relevant patches. Many thanks!.

Please note, this series has a dependency on the ACPICA header patch
here[1].


Tested on a Kunpeng920 server machine with SMMUv3, the 3408iMR RAID
controller card works as expected,

Tested-by: Hanjun Guo 

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


Re: [PATCH v12 5/9] ACPI/IORT: Add a helper to retrieve RMR info directly

2022-05-04 Thread Hanjun Guo via iommu

On 2022/5/4 0:33, Shameer Kolothum wrote:

This will provide a way for SMMU drivers to retrieve StreamIDs
associated with IORT RMR nodes and use that to set bypass settings
for those IDs.

Tested-by: Steven Price 
Signed-off-by: Shameer Kolothum 


Reviewed-by: Hanjun Guo 

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


Re: [PATCH v12 4/9] ACPI/IORT: Add support to retrieve IORT RMR reserved regions

2022-05-04 Thread Hanjun Guo via iommu

On 2022/5/4 0:33, Shameer Kolothum wrote:

Parse through the IORT RMR nodes and populate the reserve region list
corresponding to a given IOMMU and device(optional). Also, go through
the ID mappings of the RMR node and retrieve all the SIDs associated
with it.

Reviewed-by: Lorenzo Pieralisi 
Tested-by: Steven Price 
Signed-off-by: Shameer Kolothum 


Reviewed-by: Hanjun Guo 

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


[PATCH v2 4/4] iommu/vt-d: Remove hard coding PGSNP bit in PASID entries

2022-05-04 Thread Lu Baolu
As enforce_cache_coherency has been introduced into the iommu_domain_ops,
the kernel component which owns the iommu domain is able to opt-in its
requirement for force snooping support. The iommu driver has no need to
hard code the page snoop control bit in the PASID table entries anymore.

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

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 41a0e3b02c79..0abfa7fc7fb0 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -710,9 +710,6 @@ int intel_pasid_setup_second_level(struct intel_iommu 
*iommu,
pasid_set_fault_enable(pte);
pasid_set_page_snoop(pte, !!ecap_smpwc(iommu->ecap));
 
-   if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
-   pasid_set_pgsnp(pte);
-
/*
 * Since it is a second level only translation setup, we should
 * set SRE bit as well (addresses are expected to be GPAs).
-- 
2.25.1

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


[PATCH v2 3/4] iommu/vt-d: Remove domain_update_iommu_snooping()

2022-05-04 Thread Lu Baolu
The IOMMU force snooping capability is not required to be consistent
among all the IOMMUs anymore. Remove force snooping capability check
in the IOMMU hot-add path and domain_update_iommu_snooping() becomes
a dead code now.

Signed-off-by: Lu Baolu 
Reviewed-by: Jason Gunthorpe 
---
 drivers/iommu/intel/iommu.c | 34 +-
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 98112228ae93..da4bfb34ae4b 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -533,33 +533,6 @@ static void domain_update_iommu_coherency(struct 
dmar_domain *domain)
rcu_read_unlock();
 }
 
-static bool domain_update_iommu_snooping(struct intel_iommu *skip)
-{
-   struct dmar_drhd_unit *drhd;
-   struct intel_iommu *iommu;
-   bool ret = true;
-
-   rcu_read_lock();
-   for_each_active_iommu(iommu, drhd) {
-   if (iommu != skip) {
-   /*
-* If the hardware is operating in the scalable mode,
-* the snooping control is always supported since we
-* always set PASID-table-entry.PGSNP bit if the domain
-* is managed outside (UNMANAGED).
-*/
-   if (!sm_supported(iommu) &&
-   !ecap_sc_support(iommu->ecap)) {
-   ret = false;
-   break;
-   }
-   }
-   }
-   rcu_read_unlock();
-
-   return ret;
-}
-
 static int domain_update_iommu_superpage(struct dmar_domain *domain,
 struct intel_iommu *skip)
 {
@@ -3593,12 +3566,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
iommu->name);
return -ENXIO;
}
-   if (!ecap_sc_support(iommu->ecap) &&
-   domain_update_iommu_snooping(iommu)) {
-   pr_warn("%s: Doesn't support snooping.\n",
-   iommu->name);
-   return -ENXIO;
-   }
+
sp = domain_update_iommu_superpage(NULL, iommu) - 1;
if (sp >= 0 && !(cap_super_page_val(iommu->cap) & (1 << sp))) {
pr_warn("%s: Doesn't support large page.\n",
-- 
2.25.1

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


[PATCH v2 2/4] iommu/vt-d: Check domain force_snooping against attached devices

2022-05-04 Thread Lu Baolu
As domain->force_snooping only impacts the devices attached with the
domain, there's no need to check against all IOMMU units. At the same
time, for a brand new domain (hasn't been attached to any device), the
force_snooping field could be set, but the attach_dev callback will
return failure if it wants to attach to a device which IOMMU has no
snoop control capability.

Signed-off-by: Lu Baolu 
---
 include/linux/intel-iommu.h |  1 +
 drivers/iommu/intel/pasid.h |  2 ++
 drivers/iommu/intel/iommu.c | 51 ++---
 drivers/iommu/intel/pasid.c | 23 +
 4 files changed, 74 insertions(+), 3 deletions(-)

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 72e5d7900e71..4f29139bbfc3 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -540,6 +540,7 @@ struct dmar_domain {
u8 has_iotlb_device: 1;
u8 iommu_coherency: 1;  /* indicate coherency of iommu access */
u8 force_snooping : 1;  /* Create IOPTEs with snoop control */
+   u8 set_pte_snp:1;
 
struct list_head devices;   /* all devices' list */
struct iova_domain iovad;   /* iova's that belong to this domain */
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index ab4408c824a5..583ea67fc783 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -123,4 +123,6 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu,
 bool fault_ignore);
 int vcmd_alloc_pasid(struct intel_iommu *iommu, u32 *pasid);
 void vcmd_free_pasid(struct intel_iommu *iommu, u32 pasid);
+void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
+ struct device *dev, u32 pasid);
 #endif /* __INTEL_PASID_H */
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index d68f5bbf3e93..98112228ae93 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2459,7 +2459,7 @@ static int domain_setup_first_level(struct intel_iommu 
*iommu,
if (level == 5)
flags |= PASID_FLAG_FL5LP;
 
-   if (domain->domain.type == IOMMU_DOMAIN_UNMANAGED)
+   if (domain->force_snooping)
flags |= PASID_FLAG_PAGE_SNOOP;
 
return intel_pasid_setup_first_level(iommu, dev, (pgd_t *)pgd, pasid,
@@ -4431,7 +4431,7 @@ static int intel_iommu_map(struct iommu_domain *domain,
prot |= DMA_PTE_READ;
if (iommu_prot & IOMMU_WRITE)
prot |= DMA_PTE_WRITE;
-   if (dmar_domain->force_snooping)
+   if (dmar_domain->set_pte_snp)
prot |= DMA_PTE_SNP;
 
max_addr = iova + size;
@@ -4554,13 +4554,58 @@ static phys_addr_t intel_iommu_iova_to_phys(struct 
iommu_domain *domain,
return phys;
 }
 
+static bool domain_support_force_snooping(struct dmar_domain *domain)
+{
+   struct device_domain_info *info;
+   unsigned long flags;
+   bool support = true;
+
+   spin_lock_irqsave(_domain_lock, flags);
+   list_for_each_entry(info, >devices, link) {
+   if (!ecap_sc_support(info->iommu->ecap)) {
+   support = false;
+   break;
+   }
+   }
+   spin_unlock_irqrestore(_domain_lock, flags);
+
+   return support;
+}
+
+static void domain_set_force_snooping(struct dmar_domain *domain)
+{
+   struct device_domain_info *info;
+   unsigned long flags;
+
+   /*
+* Second level page table supports per-PTE snoop control. The
+* iommu_map() interface will handle this by setting SNP bit.
+*/
+   if (!domain_use_first_level(domain)) {
+   domain->set_pte_snp = true;
+   return;
+   }
+
+   spin_lock_irqsave(_domain_lock, flags);
+   list_for_each_entry(info, >devices, link)
+   intel_pasid_setup_page_snoop_control(info->iommu, info->dev,
+PASID_RID2PASID);
+   spin_unlock_irqrestore(_domain_lock, flags);
+}
+
 static bool intel_iommu_enforce_cache_coherency(struct iommu_domain *domain)
 {
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
 
-   if (!domain_update_iommu_snooping(NULL))
+   if (dmar_domain->force_snooping)
+   return true;
+
+   if (!domain_support_force_snooping(dmar_domain))
return false;
+
+   domain_set_force_snooping(dmar_domain);
dmar_domain->force_snooping = true;
+
return true;
 }
 
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index f8d215d85695..41a0e3b02c79 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -762,3 +762,26 @@ int intel_pasid_setup_pass_through(struct intel_iommu 
*iommu,
 
return 0;
 }
+
+/*
+ * Set the page snoop control for a pasid entry which has been set up.
+ */
+void 

[PATCH v2 1/4] iommu/vt-d: Block force-snoop domain attaching if no SC support

2022-05-04 Thread Lu Baolu
In the attach_dev callback of the default domain ops, if the domain has
been set force_snooping, but the iommu hardware of the device does not
support SC(Snoop Control) capability, the callback should block it and
return a corresponding error code.

Signed-off-by: Lu Baolu 
Reviewed-by: Jason Gunthorpe 
---
 drivers/iommu/intel/iommu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index cf43e8f9091b..d68f5bbf3e93 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4354,6 +4354,9 @@ static int prepare_domain_attach_device(struct 
iommu_domain *domain,
if (!iommu)
return -ENODEV;
 
+   if (dmar_domain->force_snooping && !ecap_sc_support(iommu->ecap))
+   return -EOPNOTSUPP;
+
/* check if this iommu agaw is sufficient for max mapped address */
addr_width = agaw_to_width(iommu->agaw);
if (addr_width > cap_mgaw(iommu->cap))
-- 
2.25.1

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


[PATCH v2 0/4] iommu/vt-d: Force snooping improvement

2022-05-04 Thread Lu Baolu
Hi folks,

Previously, the IOMMU capability of enforcing cache coherency was queried
through iommu_capable(IOMMU_CAP_CACHE_COHERENCY). This is a global
capability, hence the IOMMU driver reports support for this capability
only when all IOMMUs in the system has this support.

Commit 6043257b1de06 ("iommu: Introduce the domain op
enforce_cache_coherency()") converts this into a per-domain test-and-set
option, and the previous iommu_capable(IOMMU_CAP_CACHE_COHERENCY) is
deprecated.

This is a follow-up series which improves the Intel IOMMU driver to
support the per-domain scheme better.

Best regards,
baolu

Change log:
v2:
 - Check whether force_snooping has already been set in
   intel_iommu_enforce_cache_coherency().
 - Set PGSNP pasid bit field during domain attaching if forcing_snooping
   is set.
 - Remove redundant list_empty() checks.
 - Add dmar_domain->set_pte_snp and set it if force snooping is enforced
   on a domain with 2nd-level translation.

v1:
 - 
https://lore.kernel.org/linux-iommu/20220501112434.874236-1-baolu...@linux.intel.com
 - Initial post.

Lu Baolu (4):
  iommu/vt-d: Block force-snoop domain attaching if no SC support
  iommu/vt-d: Check domain force_snooping against attached devices
  iommu/vt-d: Remove domain_update_iommu_snooping()
  iommu/vt-d: Remove hard coding PGSNP bit in PASID entries

 include/linux/intel-iommu.h |  1 +
 drivers/iommu/intel/pasid.h |  2 +
 drivers/iommu/intel/iommu.c | 88 ++---
 drivers/iommu/intel/pasid.c | 26 +--
 4 files changed, 78 insertions(+), 39 deletions(-)

-- 
2.25.1

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


Re: [PATCH v12 3/9] ACPI/IORT: Provide a generic helper to retrieve reserve regions

2022-05-04 Thread Hanjun Guo via iommu

On 2022/5/4 0:33, Shameer Kolothum wrote:

Currently IORT provides a helper to retrieve HW MSI reserve regions.
Change this to a generic helper to retrieve any IORT related reserve
regions. This will be useful when we add support for RMR nodes in
subsequent patches.

[Lorenzo: For ACPI IORT]
Reviewed-by: Lorenzo Pieralisi 
Reviewed-by: Christoph Hellwig 
Tested-by: Steven Price 
Signed-off-by: Shameer Kolothum 


Reviewed-by: Hanjun Guo 

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

Re: [PATCH v12 2/9] ACPI/IORT: Make iort_iommu_msi_get_resv_regions() return void

2022-05-04 Thread Hanjun Guo via iommu

On 2022/5/4 0:33, Shameer Kolothum wrote:

At present iort_iommu_msi_get_resv_regions() returns the number of
MSI reserved regions on success and there are no users for this.
The reserved region list will get populated anyway for platforms
that require the HW MSI region reservation. Hence, change the
function to return void instead.

Reviewed-by: Christoph Hellwig 
Tested-by: Steven Price 
Signed-off-by: Shameer Kolothum 


Reviewed-by: Hanjun Guo 

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


Re: [PATCH 2/7] dt-bindings: i2c: renesas,rcar-i2c: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:54PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  I2C on R-Car V3U also supports some extra features (e.g. Slave
> Clock Stretch Select), which are supported by other R-Car Gen4 SoCs, but
> not by any other R-Car Gen3 SoC.
> 
> Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Applied to for-next, thanks!



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

Re: [PATCH 7/7] dt-bindings: watchdog: renesas,wdt: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:59PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Wolfram Sang 



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

Re: [PATCH 6/7] dt-bindings: serial: renesas,scif: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:58PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Wolfram Sang 



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

Re: [PATCH 5/7] dt-bindings: serial: renesas,hscif: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:57PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Wolfram Sang 



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

Re: [PATCH 3/7] dt-bindings: iommu: renesas, ipmmu-vmsa: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:55PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Wolfram Sang 



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

Re: [PATCH 2/7] dt-bindings: i2c: renesas,rcar-i2c: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:54PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  I2C on R-Car V3U also supports some extra features (e.g. Slave
> Clock Stretch Select), which are supported by other R-Car Gen4 SoCs, but
> not by any other R-Car Gen3 SoC.
> 
> Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 

Reviewed-by: Wolfram Sang 



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

Re: [PATCH 1/7] dt-bindings: gpio: renesas,rcar-gpio: R-Car V3U is R-Car Gen4

2022-05-04 Thread Wolfram Sang
On Mon, May 02, 2022 at 03:34:53PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---

Reviewed-by: Wolfram Sang 



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

[PATCH v2] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
Once the group enters 'owned' mode it can never be assigned back to the
default_domain or to a NULL domain. It must always be actively assigned to
a current domain. If the caller hasn't provided a domain then the core
must provide an explicit DMA blocking domain that has no DMA map.

Lazily create a group-global blocking DMA domain when
iommu_group_claim_dma_owner is first called and immediately assign the
group to it. This ensures that DMA is immediately fully isolated on all
IOMMU drivers.

If the user attaches/detaches while owned then detach will set the group
back to the blocking domain.

Slightly reorganize the call chains so that
__iommu_group_attach_core_domain() is the function that removes any caller
configured domain and sets the domains back a core owned domain with an
appropriate lifetime.

__iommu_group_attach_domain() is the worker function that can change the
domain assigned to a group to any target domain, including NULL.

Add comments clarifying how the NULL vs detach_dev vs default_domain works
based on Robin's remarks.

This fixes an oops with VFIO and SMMUv3 because VFIO will call
iommu_detach_group() and then immediately iommu_domain_free(), but
SMMUv3 has no way to know that the domain it is holding a pointer to
has been freed. Now the iommu_detach_group() will assign the blocking
domain and SMMUv3 will no longer hold a stale domain reference.

Fixes: 1ea2a07a532b ("iommu: Add DMA ownership management interfaces")
Reported-by: Qian Cai 
Tested-by: Baolu Lu 
Tested-by: Nicolin Chen 
Co-developed-by: Robin Murphy 
Signed-off-by: Robin Murphy 
Signed-off-by: Jason Gunthorpe 
---
 drivers/iommu/iommu.c | 122 ++
 1 file changed, 87 insertions(+), 35 deletions(-)

Joerg, this should address the issue, Nicolin reproduced the original issue
and verified this fix on ARM SMMUv3.

v2:
 - Remove redundant detach_dev ops check in __iommu_detach_device and
   make the added WARN_ON fail instead
 - Check for blocking_domain in __iommu_attach_group() so VFIO can
   actually attach a new group
 - Update comments and spelling
 - Fix missed change to new_domain in iommu_group_do_detach_device()
v1: 
https://lore.kernel.org/r/0-v1-6e9d2d0a759d+11b-iommu_dma_block_...@nvidia.com

Thanks,
Jason

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0c42ece2585406..c1bdec807ea381 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -44,6 +44,7 @@ struct iommu_group {
char *name;
int id;
struct iommu_domain *default_domain;
+   struct iommu_domain *blocking_domain;
struct iommu_domain *domain;
struct list_head entry;
unsigned int owner_cnt;
@@ -82,8 +83,7 @@ static int __iommu_attach_device(struct iommu_domain *domain,
 struct device *dev);
 static int __iommu_attach_group(struct iommu_domain *domain,
struct iommu_group *group);
-static void __iommu_detach_group(struct iommu_domain *domain,
-struct iommu_group *group);
+static void __iommu_group_attach_core_domain(struct iommu_group *group);
 static int iommu_create_device_direct_mappings(struct iommu_group *group,
   struct device *dev);
 static struct iommu_group *iommu_group_get_for_dev(struct device *dev);
@@ -596,6 +596,8 @@ static void iommu_group_release(struct kobject *kobj)
 
if (group->default_domain)
iommu_domain_free(group->default_domain);
+   if (group->blocking_domain)
+   iommu_domain_free(group->blocking_domain);
 
kfree(group->name);
kfree(group);
@@ -1963,9 +1965,6 @@ static void __iommu_detach_device(struct iommu_domain 
*domain,
if (iommu_is_attach_deferred(dev))
return;
 
-   if (unlikely(domain->ops->detach_dev == NULL))
-   return;
-
domain->ops->detach_dev(domain, dev);
trace_detach_device_from_domain(dev);
 }
@@ -1979,12 +1978,10 @@ void iommu_detach_device(struct iommu_domain *domain, 
struct device *dev)
return;
 
mutex_lock(>mutex);
-   if (iommu_group_device_count(group) != 1) {
-   WARN_ON(1);
+   if (WARN_ON(domain != group->domain) ||
+   WARN_ON(iommu_group_device_count(group) != 1))
goto out_unlock;
-   }
-
-   __iommu_detach_group(domain, group);
+   __iommu_group_attach_core_domain(group);
 
 out_unlock:
mutex_unlock(>mutex);
@@ -2040,7 +2037,8 @@ static int __iommu_attach_group(struct iommu_domain 
*domain,
 {
int ret;
 
-   if (group->domain && group->domain != group->default_domain)
+   if (group->domain && group->domain != group->default_domain &&
+   group->domain != group->blocking_domain)
return -EBUSY;
 
ret = __iommu_group_for_each_dev(group, domain,
@@ -2072,38 +2070,68 @@ static int 

Re: [git pull] IOMMU Fixes for Linux v5.18-rc5

2022-05-04 Thread pr-tracker-bot
The pull request you sent on Wed, 4 May 2022 17:57:47 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
> tags/iomm-fixes-v5.18-rc5

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/a7391ad3572431a354c927cf8896e86e50d7d0bf

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 04:29:24PM +0100, Robin Murphy wrote:
> On 2022-05-04 15:54, Jason Gunthorpe wrote:
> > On Wed, May 04, 2022 at 03:42:09PM +0100, Robin Murphy wrote:
> > 
> > > > This fixes an oops with VFIO and SMMUv3 because VFIO will call
> > > > iommu_detach_group() and then immediately iommu_domain_free(), but
> > > > SMMUv3 has no way to know that the domain it is holding a pointer to
> > > > has been freed. Now the iommu_detach_group() will assign the blocking
> > > > domain and SMMUv3 will no longer hold a stale domain reference.
> > > 
> > > Thanks for taking this on! I do like the overall structure and naming much
> > > more than my initial sketch :)
> > 
> > Thanks, no problem!
> > > > /*
> > > > -* If the group has been claimed already, do not re-attach the 
> > > > default
> > > > -* domain.
> > > > +* A NULL domain means to call the detach_dev() op. New drivers 
> > > > should
> > > > +* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL 
> > > > default_domain
> > > 
> > > Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support, 
> > > passthrough
> > > is more of an optional extra.
> > 
> > Can you elaborate on this a bit more for the comment, I'm not sure I
> > understand all the historical stuff here.
> 
> Well, the comment could effectively just be "New drivers should support
> default domains."

I made it this:

/*
 * New drivers should support default domains and so the detach_dev() op
 * will never be called. Otheriwse the NULL domain indicates the
 * translation for the group should be set so it will work with the
 * platform DMA ops.
 */

It also seems clear to me we should delete a bunch of detach_dev ops
from drivers?

I naively see that these drivers have the word IOMMU_DOMAIN_DMA in
them and also define detach_dev:
 amd iommu
 apple-dart
 qcom_iommu
 exynos-iommu
 intel iommu
 ipmmu-vmsa
 mtk_iommu
 rockchip-iommu
 sprd-iommu
 sun50i-iommu

These ones have IOMMU_DOMAIN_DMA but don't define detach_dev:
 arm-smmuv3
 arm-smmu
 virtio-iommu

And I suppose these are the 'old' drivers:
 fsl_pamu
 omap-iommu
 tegra-gart
 tegra-smmu

I can get a patch for this as well, I think it will be clarifying to
remove this cruft.

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


Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()

2022-05-04 Thread Alex Williamson
On Wed, 4 May 2022 10:42:07 +0200
Joerg Roedel  wrote:

> On Mon, May 02, 2022 at 12:12:04PM -0400, Qian Cai wrote:
> > Reverting this series fixed an user-after-free while doing SR-IOV.
> > 
> >  BUG: KASAN: use-after-free in __lock_acquire  
> 
> Hrm, okay. I am going exclude this series from my next branch for now
> until this has been sorted out.
> 
> Alex, I suggest you do the same.

Done, and thanks for the heads-up.  Please try to cc me when the
vfio-notifier-fix branch is merged back into your next branch.  Thanks,

Alex

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


[git pull] IOMMU Fixes for Linux v5.18-rc5

2022-05-04 Thread Joerg Roedel
Hi Linus,

The following changes since commit af2d861d4cd2a4da5137f795ee3509e6f944a25b:

  Linux 5.18-rc4 (2022-04-24 14:51:22 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
tags/iomm-fixes-v5.18-rc5

for you to fetch changes up to 392bf51946c2463436a1ba237c1ec5865b234825:

  iommu: Make sysfs robust for non-API groups (2022-05-04 15:13:39 +0200)


IOMMU Fixes for Linux v5.18-rc5

Including:

- Fix for a regression in IOMMU core code which could cause NULL-ptr
  dereferences

- Arm SMMU fixes for 5.18
  - Fix off-by-one in SMMUv3 SVA TLB invalidation
  - Disable large mappings to workaround nvidia erratum

- Intel VT-d fixes
  - Handle PCI stop marker messages in IOMMU driver to meet the
requirement of I/O page fault handling framework.
  - Calculate a feasible mask for non-aligned page-selective IOTLB
invalidation.

- Two fixes for Apple DART IOMMU driver
  - Fix potential NULL-ptr dereference
  - Set module owner


Ashish Mhetre (1):
  iommu: arm-smmu: disable large page mappings for Nvidia arm-smmu

David Stevens (1):
  iommu/vt-d: Calculate mask for non-aligned flushes

Hector Martin (1):
  iommu/dart: Add missing module owner to ops structure

Joerg Roedel (1):
  Merge tag 'arm-smmu-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/will/linux into iommu/fixes

Lu Baolu (1):
  iommu/vt-d: Drop stop marker messages

Nicolin Chen (1):
  iommu/arm-smmu-v3: Fix size calculation in arm_smmu_mm_invalidate_range()

Robin Murphy (1):
  iommu: Make sysfs robust for non-API groups

Yang Yingliang (1):
  iommu/dart: check return value after calling platform_get_resource()

 drivers/iommu/apple-dart.c  | 10 -
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c |  9 +++-
 drivers/iommu/arm/arm-smmu/arm-smmu-nvidia.c| 30 +
 drivers/iommu/intel/iommu.c | 27 +++---
 drivers/iommu/intel/svm.c   |  4 
 drivers/iommu/iommu.c   |  9 +++-
 6 files changed, 79 insertions(+), 10 deletions(-)

Please pull.

Thanks,

Joerg


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

Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Robin Murphy

On 2022-05-04 15:54, Jason Gunthorpe wrote:

On Wed, May 04, 2022 at 03:42:09PM +0100, Robin Murphy wrote:


This fixes an oops with VFIO and SMMUv3 because VFIO will call
iommu_detach_group() and then immediately iommu_domain_free(), but
SMMUv3 has no way to know that the domain it is holding a pointer to
has been freed. Now the iommu_detach_group() will assign the blocking
domain and SMMUv3 will no longer hold a stale domain reference.


Thanks for taking this on! I do like the overall structure and naming much
more than my initial sketch :)


Thanks, no problem!
  

/*
-* If the group has been claimed already, do not re-attach the default
-* domain.
+* A NULL domain means to call the detach_dev() op. New drivers should
+* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain


Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support, passthrough
is more of an optional extra.


Can you elaborate on this a bit more for the comment, I'm not sure I
understand all the historical stuff here.


Well, the comment could effectively just be "New drivers should support 
default domains."


What supporting default domains means in practice is two things: that 
.attach_dev handles moving directly between domains without .detach_dev 
being called, and that .domain_alloc supports at least IOMMU_DOMAIN_DMA 
- other unsupported default domain types can fall back to that, but not 
vice versa, see iommu_group_alloc_default_domain().



Here we are looking at a case where group->domain becomes NULL - what
does this mean in the historical world? ie what should the iommu
driver do when detach_dev is called?

I had guessed it was remove all translation - ie IOMMU_DOMAIN_IDENTITY?


Historically, whatever a NULL domain means is mostly between the IOMMU 
driver and the platform DMA ops - I honestly have no idea what the likes 
of s390 and fsl-pamu do, for example. For SMMUv3 it was always configurable.


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


Re: [PATCH 1/2] dt-bindings: arm-smmu: Add compatible for Qualcomm SC8280XP

2022-05-04 Thread Krzysztof Kozlowski
On 03/05/2022 18:34, Bjorn Andersson wrote:
> Add compatible for the Qualcomm SC8280XP platform to the ARM SMMU
> DeviceTree binding.
> 
> Signed-off-by: Bjorn Andersson 

Acked-by: Krzysztof Kozlowski 


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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 10:55:00PM +0800, Baolu Lu wrote:
> On 2022/5/4 22:38, Jason Gunthorpe wrote:
> > > @@ -3180,7 +3181,9 @@ int iommu_group_claim_dma_owner(struct iommu_group
> > > *group, void *owner)
> > >  ret = -EPERM;
> > >  goto unlock_out;
> > >  } else {
> > > -   if (group->domain && group->domain != 
> > > group->default_domain)
> > > {
> > > +   if (group->domain &&
> > > +   group->domain != group->default_domain &&
> > > +   group->domain != group->blocking_domain) {
> > Why do we need this hunk? This is just trying to check if there is
> > some conflict with some other domain attach, group->domain can never
> > be blocking_domain here.
> This is *not* needed. Also verified with real hardware.
> 
> Sorry for the noise.

Okay, great, I'll add your tested-by, thanks

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Baolu Lu

On 2022/5/4 22:38, Jason Gunthorpe wrote:

@@ -3180,7 +3181,9 @@ int iommu_group_claim_dma_owner(struct iommu_group
*group, void *owner)
 ret = -EPERM;
 goto unlock_out;
 } else {
-   if (group->domain && group->domain != group->default_domain)
{
+   if (group->domain &&
+   group->domain != group->default_domain &&
+   group->domain != group->blocking_domain) {

Why do we need this hunk? This is just trying to check if there is
some conflict with some other domain attach, group->domain can never
be blocking_domain here.

This is *not* needed. Also verified with real hardware.

Sorry for the noise.

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 03:42:09PM +0100, Robin Murphy wrote:

> > This fixes an oops with VFIO and SMMUv3 because VFIO will call
> > iommu_detach_group() and then immediately iommu_domain_free(), but
> > SMMUv3 has no way to know that the domain it is holding a pointer to
> > has been freed. Now the iommu_detach_group() will assign the blocking
> > domain and SMMUv3 will no longer hold a stale domain reference.
> 
> Thanks for taking this on! I do like the overall structure and naming much
> more than my initial sketch :)

Thanks, no problem!
 
> > /*
> > -* If the group has been claimed already, do not re-attach the default
> > -* domain.
> > +* A NULL domain means to call the detach_dev() op. New drivers should
> > +* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain
> 
> Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support, passthrough
> is more of an optional extra.

Can you elaborate on this a bit more for the comment, I'm not sure I
understand all the historical stuff here.

Here we are looking at a case where group->domain becomes NULL - what
does this mean in the historical world? ie what should the iommu
driver do when detach_dev is called?

I had guessed it was remove all translation - ie IOMMU_DOMAIN_IDENTITY?

> > +* and detatch_dev().
> 
> Nit: detach_dev
> 
> Otherwise, modulo the other things already pointed out, this looks OK to me.

Done, thanks

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Robin Murphy

On 2022-05-04 01:11, Jason Gunthorpe wrote:

Once the group enters 'owned' mode it can never be assigned back to the
default_domain or to a NULL domain. It must always be actively assigned to
a current domain. If the caller hasn't provided a domain then the core
must provide an explicit DMA blocking domain that has no DMA map.

Lazily create a group-global blocking DMA domain when
iommu_group_claim_dma_owner is first called and immediately assign the
group to it. This ensures that DMA is immediately fully isolated on all
IOMMU drivers.

If the user attaches/detaches while owned then detach will set the group
back to the blocking domain.

Slightly reorganize the call chains so that
__iommu_group_attach_core_domain() is the function that removes any caller
configured domain and sets the domains back a core owned domain with an
appropriate lifetime.

__iommu_group_attach_domain() is the worker function that can change the
domain assigned to a group to any target domain, including NULL.

Add comments clarifying how the NULL vs detach_dev vs default_domain works
based on Robin's remarks.

This fixes an oops with VFIO and SMMUv3 because VFIO will call
iommu_detach_group() and then immediately iommu_domain_free(), but
SMMUv3 has no way to know that the domain it is holding a pointer to
has been freed. Now the iommu_detach_group() will assign the blocking
domain and SMMUv3 will no longer hold a stale domain reference.


Thanks for taking this on! I do like the overall structure and naming 
much more than my initial sketch :)



Fixes: 1ea2a07a532b ("iommu: Add DMA ownership management interfaces")
Reported-by: Qian Cai 


Co-developed-by: Robin Murphy 

(so the sign-off chain makes sense - you deserve overall authorship here)


Signed-off-by: Robin Murphy 
Signed-off-by: Jason Gunthorpe 
---

[...]

@@ -2072,38 +2072,66 @@ static int iommu_group_do_detach_device(struct device 
*dev, void *data)
return 0;
  }
  
-static void __iommu_detach_group(struct iommu_domain *domain,

-struct iommu_group *group)
+static int __iommu_group_attach_domain(struct iommu_group *group,
+  struct iommu_domain *new_domain)
  {
int ret;
  
+	if (group->domain == new_domain)

+   return 0;
+
/*
-* If the group has been claimed already, do not re-attach the default
-* domain.
+* A NULL domain means to call the detach_dev() op. New drivers should
+* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain


Nit: IOMMU_DOMAIN_DMA is the baseline of default domain support, 
passthrough is more of an optional extra.



+* and detatch_dev().


Nit: detach_dev

Otherwise, modulo the other things already pointed out, this looks OK to me.

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 10:35:12PM +0800, Baolu Lu wrote:

> With below additional changes, this patch works on my Intel test
> machine.

Thanks!
 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 513da82f2ed1..7c415e9b6906 100644
> +++ b/drivers/iommu/iommu.c
> @@ -2063,7 +2063,8 @@ static int __iommu_attach_group(struct iommu_domain
> *domain,
>  {
> int ret;
> 
> -   if (group->domain && group->domain != group->default_domain)
> +   if (group->domain && group->domain != group->default_domain &&
> +   group->domain != group->blocking_domain)
> return -EBUSY;
> 
> ret = __iommu_group_for_each_dev(group, domain,
> @@ -2125,7 +2126,7 @@ static int __iommu_group_attach_domain(struct
> iommu_group *group,
>  * Note that this is called in error unwind paths, attaching to a
>  * domain that has already been attached cannot fail.
>  */
> -   ret = __iommu_group_for_each_dev(group, group->default_domain,
> +   ret = __iommu_group_for_each_dev(group, new_domain,
>  iommu_group_do_attach_device);
> if (ret)
> return ret;

Done

> @@ -3180,7 +3181,9 @@ int iommu_group_claim_dma_owner(struct iommu_group
> *group, void *owner)
> ret = -EPERM;
> goto unlock_out;
> } else {
> -   if (group->domain && group->domain != group->default_domain)
> {
> +   if (group->domain &&
> +   group->domain != group->default_domain &&
> +   group->domain != group->blocking_domain) {

Why do we need this hunk? This is just trying to check if there is
some conflict with some other domain attach, group->domain can never
be blocking_domain here.

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


Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries

2022-05-04 Thread Baolu Lu

On 2022/5/4 21:31, Jason Gunthorpe wrote:

On Wed, May 04, 2022 at 03:25:50PM +0800, Baolu Lu wrote:

Hi Jason,

On 2022/5/2 21:05, Jason Gunthorpe wrote:

On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote:

The SNP bit is only valid for second-level PTEs. Setting this bit in the
first-level PTEs has no functional impact because the Intel IOMMU always
ignores the same bit in first-level PTEs. Anyway, let's check the page
table type before setting SNP bit in PTEs to make the code more readable.

Shouldn't this be tested before setting force_snooping and not during
every map?


The check is in the following patch. This just makes sure that SNP is
only set in second-level page table entries.


I think you should add a 2nd flag to indicate 'set SNP bit in PTEs'
and take care of computing that flag in the enforce_no_snoop function


Yours looks better. Will add it in the next version.


Jason


Best regards,
baolu

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Baolu Lu

Hi Jason,

On 2022/5/4 08:11, Jason Gunthorpe wrote:

Once the group enters 'owned' mode it can never be assigned back to the
default_domain or to a NULL domain. It must always be actively assigned to
a current domain. If the caller hasn't provided a domain then the core
must provide an explicit DMA blocking domain that has no DMA map.

Lazily create a group-global blocking DMA domain when
iommu_group_claim_dma_owner is first called and immediately assign the
group to it. This ensures that DMA is immediately fully isolated on all
IOMMU drivers.

If the user attaches/detaches while owned then detach will set the group
back to the blocking domain.

Slightly reorganize the call chains so that
__iommu_group_attach_core_domain() is the function that removes any caller
configured domain and sets the domains back a core owned domain with an
appropriate lifetime.

__iommu_group_attach_domain() is the worker function that can change the
domain assigned to a group to any target domain, including NULL.

Add comments clarifying how the NULL vs detach_dev vs default_domain works
based on Robin's remarks.

This fixes an oops with VFIO and SMMUv3 because VFIO will call
iommu_detach_group() and then immediately iommu_domain_free(), but
SMMUv3 has no way to know that the domain it is holding a pointer to
has been freed. Now the iommu_detach_group() will assign the blocking
domain and SMMUv3 will no longer hold a stale domain reference.

Fixes: 1ea2a07a532b ("iommu: Add DMA ownership management interfaces")
Reported-by: Qian Cai 
Signed-off-by: Robin Murphy 
Signed-off-by: Jason Gunthorpe 
---
  drivers/iommu/iommu.c | 112 +++---
  1 file changed, 82 insertions(+), 30 deletions(-)

This is based on Robins draft here:

https://lore.kernel.org/linux-iommu/18831161-473f-e04f-4a81-1c7062ad1...@arm.com/

With some rework. I re-organized the call chains instead of introducing
iommu_group_user_attached(), fixed a recursive locking for
iommu_group_get_purgatory(), and made a proper commit message.

Still only compile tested, so RFCish.

Nicolin/Lu? What do you think, can you check it?


Thank you for the patch.

With below additional changes, this patch works on my Intel test
machine.

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 513da82f2ed1..7c415e9b6906 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2063,7 +2063,8 @@ static int __iommu_attach_group(struct 
iommu_domain *domain,

 {
int ret;

-   if (group->domain && group->domain != group->default_domain)
+   if (group->domain && group->domain != group->default_domain &&
+   group->domain != group->blocking_domain)
return -EBUSY;

ret = __iommu_group_for_each_dev(group, domain,
@@ -2125,7 +2126,7 @@ static int __iommu_group_attach_domain(struct 
iommu_group *group,

 * Note that this is called in error unwind paths, attaching to a
 * domain that has already been attached cannot fail.
 */
-   ret = __iommu_group_for_each_dev(group, group->default_domain,
+   ret = __iommu_group_for_each_dev(group, new_domain,
 iommu_group_do_attach_device);
if (ret)
return ret;
@@ -3180,7 +3181,9 @@ int iommu_group_claim_dma_owner(struct iommu_group 
*group, void *owner)

ret = -EPERM;
goto unlock_out;
} else {
-   if (group->domain && group->domain != 
group->default_domain) {

+   if (group->domain &&
+   group->domain != group->default_domain &&
+   group->domain != group->blocking_domain) {
ret = -EBUSY;
goto unlock_out;

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


Re: [PATCH v12 0/9] ACPI/IORT: Support for IORT RMR node

2022-05-04 Thread Laurentiu Tudor



On 5/3/2022 7:33 PM, Shameer Kolothum wrote:

Hi

v11 --> v12
   -Minor fix in patch #4 to address the issue reported by the kernel test 
robot.
   -Added R-by tags by Christoph(patch #1) and Lorenzo(patch #4).
   -Added T-by from Steve to all relevant patches. Many thanks!.


Tested on a NXP LX2160A with SMMUv2, so:

Tested-by: Laurentiu Tudor 

---
Thanks & Best Regards, Laurentiu


Please note, this series has a dependency on the ACPICA header patch
here[1].

Please take a look and let me know.

Thanks,
Shameer
[1] 
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F44610361.fMDQidcC6G%40kreacher%2Fdata=05%7C01%7Claurentiu.tudor%40nxp.com%7C8157d32925724ac9bf7908da2d22c1ab%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637871924543316157%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=DdYCpg%2B7NW%2Fb8FBYOEsmlYV88kwN0K75AF9Y7%2Fg2BEo%3Dreserved=0

 From old:
We have faced issues with 3408iMR RAID controller cards which
fail to boot when SMMU is enabled. This is because these
controllers make use of host memory for various caching related
purposes and when SMMU is enabled the iMR firmware fails to
access these memory regions as there is no mapping for them.
IORT RMR provides a way for UEFI to describe and report these
memory regions so that the kernel can make a unity mapping for
these in SMMU.

Change History:

v10 --> v11
  -Addressed Christoph's comments. We now have a  callback to
   struct iommu_resv_region to free all related memory and also dropped
   the FW specific union and now has a container struct iommu_iort_rmr_data.
   See patches #1 & #4
  -Added R-by from Christoph.
  -Dropped R-by from Lorenzo for patches #4 & #5 due to the above changes.
  -Also dropped T-by from Steve and Laurentiu. Many thanks for your test
   efforts. I have done basic sanity testing on my platform but please
   do it again at your end.

v9 --> v10
  - Dropped patch #1 ("Add temporary RMR node flag definitions") since
the ACPICA header updates patch is now in the mailing list
  - Based on the suggestion from Christoph, introduced a
resv_region_free_fw_data() callback in struct iommu_resv_region and
used that to free RMR specific memory allocations.

v8 --> v9
  - Adressed comments from Robin on interfaces.
  - Addressed comments from Lorenzo.

v7 --> v8
   - Patch #1 has temp definitions for RMR related changes till
     the ACPICA header changes are part of kernel.
   - No early parsing of RMR node info and is only parsed at the
     time of use.
   - Changes to the RMR get/put API format compared to the
     previous version.
   - Support for RMR descriptor shared by multiple stream IDs.

v6 --> v7
  -fix pointed out by Steve to the SMMUv2 SMR bypass install in patch #8.

v5 --> v6
- Addressed comments from Robin & Lorenzo.
   : Moved iort_parse_rmr() to acpi_iort_init() from
     iort_init_platform_devices().
   : Removed use of struct iort_rmr_entry during the initial
     parse. Using struct iommu_resv_region instead.
   : Report RMR address alignment and overlap errors, but continue.
   : Reworked arm_smmu_init_bypass_stes() (patch # 6).
- Updated SMMUv2 bypass SMR code. Thanks to Jon N (patch #8).
- Set IOMMU protection flags(IOMMU_CACHE, IOMMU_MMIO) based
   on Type of RMR region. Suggested by Jon N.

v4 --> v5
  -Added a fw_data union to struct iommu_resv_region and removed
   struct iommu_rmr (Based on comments from Joerg/Robin).
  -Added iommu_put_rmrs() to release mem.
  -Thanks to Steve for verifying on SMMUv2, but not added the Tested-by
   yet because of the above changes.

v3 -->v4
-Included the SMMUv2 SMR bypass install changes suggested by
  Steve(patch #7)
-As per Robin's comments, RMR reserve implementation is now
  more generic  (patch #8) and dropped v3 patches 8 and 10.
-Rebase to 5.13-rc1

RFC v2 --> v3
  -Dropped RFC tag as the ACPICA header changes are now ready to be
   part of 5.13[0]. But this series still has a dependency on that patch.
  -Added IORT E.b related changes(node flags, _DSM function 5 checks for
   PCIe).
  -Changed RMR to stream id mapping from M:N to M:1 as per the spec and
   discussion here[1].
  -Last two patches add support for SMMUv2(Thanks to Jon Nettleton!)

Jon Nettleton (1):
   iommu/arm-smmu: Get associated RMR info and install bypass SMR

Shameer Kolothum (8):
   iommu: Introduce a callback to struct iommu_resv_region
   ACPI/IORT: Make iort_iommu_msi_get_resv_regions() return void
   ACPI/IORT: Provide a generic helper to retrieve reserve regions
   ACPI/IORT: Add support to retrieve IORT RMR reserved regions
   ACPI/IORT: Add a helper to retrieve RMR info directly
   iommu/arm-smmu-v3: Introduce strtab init helper
   iommu/arm-smmu-v3: Refactor arm_smmu_init_bypass_stes() to force
 bypass
   iommu/arm-smmu-v3: Get associated RMR info and install bypass STE

  drivers/acpi/arm64/iort.c   | 360 ++--
  

Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 03:25:50PM +0800, Baolu Lu wrote:
> Hi Jason,
> 
> On 2022/5/2 21:05, Jason Gunthorpe wrote:
> > On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote:
> > > The SNP bit is only valid for second-level PTEs. Setting this bit in the
> > > first-level PTEs has no functional impact because the Intel IOMMU always
> > > ignores the same bit in first-level PTEs. Anyway, let's check the page
> > > table type before setting SNP bit in PTEs to make the code more readable.
> > Shouldn't this be tested before setting force_snooping and not during
> > every map?
> 
> The check is in the following patch. This just makes sure that SNP is
> only set in second-level page table entries.

I think you should add a 2nd flag to indicate 'set SNP bit in PTEs'
and take care of computing that flag in the enforce_no_snoop function

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


Re: [PATCH] iommu: Make sysfs robust for non-API groups

2022-05-04 Thread Joerg Roedel
On Wed, May 04, 2022 at 01:39:58PM +0100, Robin Murphy wrote:
> Groups created by VFIO backends outside the core IOMMU API should never
> be passed directly into the API itself, however they still expose their
> standard sysfs attributes, so we can still stumble across them that way.
> Take care to consider those cases before jumping into our normal
> assumptions of a fully-initialised core API group.
> 
> Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
> Reported-by: Jan Stancek 
> Tested-by: Jan Stancek 
> Reviewed-by: Jason Gunthorpe 
> Signed-off-by: Robin Murphy 
> ---
> 
> /me has a vested interest in not going backwards on dev_iommu_ops() :)
> 
>  drivers/iommu/iommu.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

Applied, thanks.

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


[PATCH] iommu: Make sysfs robust for non-API groups

2022-05-04 Thread Robin Murphy
Groups created by VFIO backends outside the core IOMMU API should never
be passed directly into the API itself, however they still expose their
standard sysfs attributes, so we can still stumble across them that way.
Take care to consider those cases before jumping into our normal
assumptions of a fully-initialised core API group.

Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
Reported-by: Jan Stancek 
Tested-by: Jan Stancek 
Reviewed-by: Jason Gunthorpe 
Signed-off-by: Robin Murphy 
---

/me has a vested interest in not going backwards on dev_iommu_ops() :)

 drivers/iommu/iommu.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 29906bc16371..41ea2deaee03 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -510,6 +510,13 @@ int iommu_get_group_resv_regions(struct iommu_group *group,
list_for_each_entry(device, >devices, list) {
struct list_head dev_resv_regions;
 
+   /*
+* Non-API groups still expose reserved_regions in sysfs,
+* so filter out calls that get here that way.
+*/
+   if (!device->dev->iommu)
+   break;
+
INIT_LIST_HEAD(_resv_regions);
iommu_get_resv_regions(device->dev, _resv_regions);
ret = iommu_insert_device_resv_regions(_resv_regions, head);
@@ -2977,7 +2984,7 @@ static ssize_t iommu_group_store_type(struct iommu_group 
*group,
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
 
-   if (WARN_ON(!group))
+   if (WARN_ON(!group) || !group->default_domain)
return -EINVAL;
 
if (sysfs_streq(buf, "identity"))
-- 
2.35.3.dirty

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


Re: [bug] NULL pointer deref after 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 12:14:07PM +0100, Robin Murphy wrote:
> On 2022-05-04 08:53, Jan Stancek wrote:
> [...]
> > Hi,
> > 
> > I'm getting panics after hunk above was applied in this patch
> > on ppc64le KVM guest, dev->iommu is NULL.
> 
> Oof, this can probably be hit with vfio-noiommu too, and by the look of
> things, `echo auto > /sys/kernel/iommu_groups/0/type` would likely blow
> up as well. Does the patch below work for you?
> 
> Thanks,
> Robin.
> 
> ->8-
> From abf0a38563bb2922a849e235d33d342170b5bc90 Mon Sep 17 00:00:00 2001
> Message-Id: 
> 
> From: Robin Murphy 
> Date: Wed, 4 May 2022 11:53:20 +0100
> Subject: [PATCH] iommu: Make sysfs robust for non-API groups
> 
> Groups created by VFIO backends outside the core IOMMU API should never
> be passed directly into the API itself, however they still expose their
> standard sysfs attributes, so we can still stumble across them that way.
> Take care to consider those cases before jumping into our normal
> assumptions of a fully-initialised core API group.
> 
> Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
> Reported-by: Jan Stancek 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/iommu/iommu.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Jason Gunthorpe 

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


Re: [bug] NULL pointer deref after 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")

2022-05-04 Thread Jan Stancek
On Wed, May 4, 2022 at 1:14 PM Robin Murphy  wrote:
>
> On 2022-05-04 08:53, Jan Stancek wrote:
> [...]
> > Hi,
> >
> > I'm getting panics after hunk above was applied in this patch
> > on ppc64le KVM guest, dev->iommu is NULL.
>
> Oof, this can probably be hit with vfio-noiommu too, and by the look of
> things, `echo auto > /sys/kernel/iommu_groups/0/type` would likely blow
> up as well. Does the patch below work for you?

Thanks for quick reply. Yes, it does.

# cat /sys/kernel/iommu_groups/0/reserved_regions
# echo auto > /sys/kernel/iommu_groups/0/type
-bash: echo: write error: Invalid argument

Tested-by: Jan Stancek 

>
> Thanks,
> Robin.
>
> ->8-
>  From abf0a38563bb2922a849e235d33d342170b5bc90 Mon Sep 17 00:00:00 2001
> Message-Id: 
> 
> From: Robin Murphy 
> Date: Wed, 4 May 2022 11:53:20 +0100
> Subject: [PATCH] iommu: Make sysfs robust for non-API groups
>
> Groups created by VFIO backends outside the core IOMMU API should never
> be passed directly into the API itself, however they still expose their
> standard sysfs attributes, so we can still stumble across them that way.
> Take care to consider those cases before jumping into our normal
> assumptions of a fully-initialised core API group.
>
> Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
> Reported-by: Jan Stancek 
> Signed-off-by: Robin Murphy 
> ---
>   drivers/iommu/iommu.c | 9 -
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 29906bc16371..41ea2deaee03 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -510,6 +510,13 @@ int iommu_get_group_resv_regions(struct iommu_group 
> *group,
> list_for_each_entry(device, >devices, list) {
> struct list_head dev_resv_regions;
>
> +   /*
> +* Non-API groups still expose reserved_regions in sysfs,
> +* so filter out calls that get here that way.
> +*/
> +   if (!device->dev->iommu)
> +   break;
> +
> INIT_LIST_HEAD(_resv_regions);
> iommu_get_resv_regions(device->dev, _resv_regions);
> ret = iommu_insert_device_resv_regions(_resv_regions, 
> head);
> @@ -2977,7 +2984,7 @@ static ssize_t iommu_group_store_type(struct 
> iommu_group *group,
> if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> return -EACCES;
>
> -   if (WARN_ON(!group))
> +   if (WARN_ON(!group) || !group->default_domain)
> return -EINVAL;
>
> if (sysfs_streq(buf, "identity"))
> --
> 2.35.3.dirty
>

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 01:22:00AM -0700, Nicolin Chen wrote:

> I am able to repro the issue on ARM64 and give this a quick try.
> But the patch seems to need to include the following change too.
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 94d99768023c..9bb108d01baa 100644
> +++ b/drivers/iommu/iommu.c
> @@ -2040,7 +2040,8 @@ static int __iommu_attach_group(struct iommu_domain 
> *domain,
>  {
>   int ret;
>  
> - if (group->domain && group->domain != group->default_domain)
> + if (group->domain && group->domain != group->default_domain &&
> + group->domain != group->blocking_domain)
>   return -EBUSY;
>  
>   ret = __iommu_group_for_each_dev(group, domain,

Make sense, thanks

> > /*
> > -* If the group has been claimed already, do not re-attach the default
> > -* domain.
> > +* A NULL domain means to call the detach_dev() op. New drivers should
> > +* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain
> 
> an IOMMU_DOMAIN_IDENTITY?

Done

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


Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()

2022-05-04 Thread Joerg Roedel
On Wed, May 04, 2022 at 08:51:35AM -0300, Jason Gunthorpe wrote:
> Nicolin and Eric have been testing with this series on ARM for a long
> time now, it is not like it is completely broken.

Yeah, I am also optimistic this can be fixed soon. But the rule is that
the next branch should only contain patches which I would send to Linus.
And with a known issue in it I wouldn't, so it is excluded at least from
my next branch for now. The topic branch is still alive and I will merge
it again when the fix is in.

Regards,

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


Re: [bug] NULL pointer deref after 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")

2022-05-04 Thread Joerg Roedel
Hi Robin,

On Wed, May 04, 2022 at 12:14:07PM +0100, Robin Murphy wrote:
> Oof, this can probably be hit with vfio-noiommu too, and by the look of
> things, `echo auto > /sys/kernel/iommu_groups/0/type` would likely blow
> up as well. Does the patch below work for you?

Thanks for the quick patch! I am delaying to send iommu-fixes upstream
for now in the hope the fix can make it into the branch asap. Please
send it out as a separate patch as soon as it is successfully tested.

Regards,

Joerg

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 07:48:58PM +0800, Baolu Lu wrote:

> > +   /*
> > +* New drivers do not implement detach_dev, so changing the domain is
> > +* done by calling attach on the new domain. Drivers should implement
> > +* this so that DMA is always translated by either the new, old, or a
> > +* blocking domain. DMA should never become untranslated.
> > +*
> > +* Note that this is called in error unwind paths, attaching to a
> > +* domain that has already been attached cannot fail.
> > +*/
> > ret = __iommu_group_for_each_dev(group, group->default_domain,
>   ^^
> 
> I suppose this should be @new_domain, right?

Yes, thank you

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


Re: [PATCH v2] drm/tegra: Stop using iommu_present()

2022-05-04 Thread Robin Murphy

On 2022-05-04 01:52, Dmitry Osipenko wrote:

On 4/11/22 16:46, Robin Murphy wrote:

@@ -1092,6 +1092,19 @@ static bool host1x_drm_wants_iommu(struct host1x_device 
*dev)
struct host1x *host1x = dev_get_drvdata(dev->dev.parent);
struct iommu_domain *domain;
  
+	/* For starters, this is moot if no IOMMU is available */

+   if (!device_iommu_mapped(>dev))
+   return false;


Unfortunately this returns false on T30 with enabled IOMMU because we
don't use IOMMU for Host1x on T30 [1] to optimize performance. We can't
change it until we will update drivers to support Host1x-dedicated buffers.


Huh, so is dev->dev here not the DRM device? If it is, and 
device_iommu_mapped() returns false, then the later iommu_attach_group() 
call is going to fail anyway, so there's not much point allocating a 
domain. If it's not, then what the heck is host1x_drm_wants_iommu() 
actually testing for?


In the not-too-distant future we'll need to pass an appropriate IOMMU 
client device to iommu_domain_alloc() as well, so the sooner we can get 
this code straight the better.


Thanks,
Robin.



[1]
https://elixir.bootlin.com/linux/latest/source/drivers/gpu/host1x/dev.c#L258


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


Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()

2022-05-04 Thread Jason Gunthorpe via iommu
On Wed, May 04, 2022 at 10:42:07AM +0200, Joerg Roedel wrote:
> On Mon, May 02, 2022 at 12:12:04PM -0400, Qian Cai wrote:
> > Reverting this series fixed an user-after-free while doing SR-IOV.
> > 
> >  BUG: KASAN: use-after-free in __lock_acquire
>
> Hrm, okay. I am going exclude this series from my next branch for now
> until this has been sorted out.

This is going to blow up everything going on in vfio right now, let's
not do something so drastic please.

There is already a patch to fix it, lets wait for it to get sorted
out.

Nicolin and Eric have been testing with this series on ARM for a long
time now, it is not like it is completely broken.

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Baolu Lu

Hi Jason,

On 2022/5/4 08:11, Jason Gunthorpe wrote:

+static int __iommu_group_attach_domain(struct iommu_group *group,
+  struct iommu_domain *new_domain)
  {
int ret;
  
+	if (group->domain == new_domain)

+   return 0;
+
/*
-* If the group has been claimed already, do not re-attach the default
-* domain.
+* A NULL domain means to call the detach_dev() op. New drivers should
+* use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain
+* and detatch_dev().
 */
-   if (!group->default_domain || group->owner) {
-   __iommu_group_for_each_dev(group, domain,
+   if (!new_domain) {
+   WARN_ON(!group->domain->ops->detach_dev);
+   __iommu_group_for_each_dev(group, group->domain,
   iommu_group_do_detach_device);
group->domain = NULL;
-   return;
+   return 0;
}
  
-	if (group->domain == group->default_domain)

-   return;
-
-   /* Detach by re-attaching to the default domain */
+   /*
+* New drivers do not implement detach_dev, so changing the domain is
+* done by calling attach on the new domain. Drivers should implement
+* this so that DMA is always translated by either the new, old, or a
+* blocking domain. DMA should never become untranslated.
+*
+* Note that this is called in error unwind paths, attaching to a
+* domain that has already been attached cannot fail.
+*/
ret = __iommu_group_for_each_dev(group, group->default_domain,

^^

I suppose this should be @new_domain, right?


 iommu_group_do_attach_device);
-   if (ret != 0)
-   WARN_ON(1);
+   if (ret)
+   return ret;
+   group->domain = new_domain;
+   return 0;
+}


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


Re: [bug] NULL pointer deref after 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")

2022-05-04 Thread Robin Murphy

On 2022-05-04 08:53, Jan Stancek wrote:
[...]

Hi,

I'm getting panics after hunk above was applied in this patch
on ppc64le KVM guest, dev->iommu is NULL.


Oof, this can probably be hit with vfio-noiommu too, and by the look of
things, `echo auto > /sys/kernel/iommu_groups/0/type` would likely blow
up as well. Does the patch below work for you?

Thanks,
Robin.

->8-
From abf0a38563bb2922a849e235d33d342170b5bc90 Mon Sep 17 00:00:00 2001
Message-Id: 

From: Robin Murphy 
Date: Wed, 4 May 2022 11:53:20 +0100
Subject: [PATCH] iommu: Make sysfs robust for non-API groups

Groups created by VFIO backends outside the core IOMMU API should never
be passed directly into the API itself, however they still expose their
standard sysfs attributes, so we can still stumble across them that way.
Take care to consider those cases before jumping into our normal
assumptions of a fully-initialised core API group.

Fixes: 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")
Reported-by: Jan Stancek 
Signed-off-by: Robin Murphy 
---
 drivers/iommu/iommu.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 29906bc16371..41ea2deaee03 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -510,6 +510,13 @@ int iommu_get_group_resv_regions(struct iommu_group *group,
list_for_each_entry(device, >devices, list) {
struct list_head dev_resv_regions;
 
+		/*

+* Non-API groups still expose reserved_regions in sysfs,
+* so filter out calls that get here that way.
+*/
+   if (!device->dev->iommu)
+   break;
+
INIT_LIST_HEAD(_resv_regions);
iommu_get_resv_regions(device->dev, _resv_regions);
ret = iommu_insert_device_resv_regions(_resv_regions, head);
@@ -2977,7 +2984,7 @@ static ssize_t iommu_group_store_type(struct iommu_group 
*group,
if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
return -EACCES;
 
-	if (WARN_ON(!group))

+   if (WARN_ON(!group) || !group->default_domain)
return -EINVAL;
 
 	if (sysfs_streq(buf, "identity"))

--
2.35.3.dirty

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


Re: [PATCH 2/4] mmc: host/sdhci-msm: Add compatible string check for sdx65

2022-05-04 Thread Ulf Hansson
On Mon, 2 May 2022 at 10:38, Rohit Agarwal  wrote:
>
> Add sdx65 SoC specific compatible string check inside qcom
> 'sdhci-msm' controller driver.
>
> Signed-off-by: Rohit Agarwal 

Applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/host/sdhci-msm.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index fd8b4a9..65661ad 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -2453,6 +2453,7 @@ static const struct of_device_id sdhci_msm_dt_match[] = 
> {
>  */
> {.compatible = "qcom,qcs404-sdhci", .data = _msm_v5_var},
> {.compatible = "qcom,sdx55-sdhci",  .data = _msm_v5_var},
> +   {.compatible = "qcom,sdx65-sdhci",  .data = _msm_v5_var},
> {.compatible = "qcom,sdm630-sdhci", .data = _msm_v5_var},
> {.compatible = "qcom,sm6125-sdhci", .data = _msm_v5_var},
> {.compatible = "qcom,sm6350-sdhci", .data = _msm_v5_var},
> --
> 2.7.4
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH 1/4] dt-bindings: mmc: sdhci-msm: Document the SDX65 compatible

2022-05-04 Thread Ulf Hansson
On Mon, 2 May 2022 at 10:38, Rohit Agarwal  wrote:
>
> The SDHCI controller on SDX65 is based on MSM SDHCI v5 IP. Hence,
> document the compatible with "qcom,sdhci-msm-v5" as the fallback.
>
> Signed-off-by: Rohit Agarwal 

Applied for next, thanks!

Kind regards
Uffe


> ---
>  Documentation/devicetree/bindings/mmc/sdhci-msm.yaml | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml 
> b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> index da42a88..e423633 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-msm.yaml
> @@ -33,6 +33,7 @@ properties:
>- qcom,sdm630-sdhci
>- qcom,sdm845-sdhci
>- qcom,sdx55-sdhci
> +  - qcom,sdx65-sdhci
>- qcom,sm6125-sdhci
>- qcom,sm6350-sdhci
>- qcom,sm8150-sdhci
> --
> 2.7.4
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2] iommu: fix an incorrect NULL check on list iterator

2022-05-04 Thread Joerg Roedel
On Sun, May 01, 2022 at 09:28:23PM +0800, Xiaomeng Tong wrote:
> The bug is here:
>   if (!iommu || iommu->dev->of_node != spec->np) {
> 
> The list iterator value 'iommu' will *always* be set and non-NULL by
> list_for_each_entry(), so it is incorrect to assume that the iterator
> value will be NULL if the list is empty or no element is found (in fact,
> it will point to a invalid structure object containing HEAD).
> 
> To fix the bug, use a new value 'iter' as the list iterator, while use
> the old value 'iommu' as a dedicated variable to point to the found one,
> and remove the unneeded check for 'iommu->dev->of_node != spec->np'
> outside the loop.
> 
> Cc: sta...@vger.kernel.org
> Fixes: f78ebca8ff3d6 ("iommu/msm: Add support for generic master bindings")
> Signed-off-by: Xiaomeng Tong 
> ---
> changes since v1:
>  - add a new iter variable (suggested by Joerg Roedel)

This is now applied. I had to manually apply it because the patch was
malformed at line 36 and git-am complained.

Regards,

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


Re: [PATCH 5/5] iommu/vt-d: Remove hard coding PGSNP bit in PASID entries

2022-05-04 Thread Baolu Lu

On 2022/5/2 21:19, Jason Gunthorpe wrote:

On Sun, May 01, 2022 at 07:24:34PM +0800, Lu Baolu wrote:

As enforce_cache_coherency has been introduced into the iommu_domain_ops,
the kernel component which owns the iommu domain is able to opt-in its
requirement for force snooping support. The iommu driver has no need to
hard code the page snoop control bit in the PASID table entries anymore.

Signed-off-by: Lu Baolu 
---
  drivers/iommu/intel/pasid.h | 1 -
  drivers/iommu/intel/iommu.c | 3 ---
  drivers/iommu/intel/pasid.c | 6 --
  3 files changed, 10 deletions(-)


It seems fine, but as in the other email where do we do
pasid_set_pgsnp() for a new device attach on an already no-snopp domain?


Yes. I will take care of this in the next version.


Jason


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


Re: [PATCH 4/5] iommu/vt-d: Remove domain_update_iommu_snooping()

2022-05-04 Thread Baolu Lu

On 2022/5/3 05:36, Jacob Pan wrote:

Hi BaoLu,

On Sun, 1 May 2022 19:24:33 +0800, Lu Baolu 
wrote:


The IOMMU force snooping capability is not required to be consistent
among all the IOMMUs anymore. Remove force snooping capability check
in the IOMMU hot-add path and domain_update_iommu_snooping() becomes
a dead code now.

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

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 3c1c228f9031..d5808495eb64 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -533,33 +533,6 @@ static void domain_update_iommu_coherency(struct
dmar_domain *domain) rcu_read_unlock();
  }
  
-static bool domain_update_iommu_snooping(struct intel_iommu *skip)

-{
-   struct dmar_drhd_unit *drhd;
-   struct intel_iommu *iommu;
-   bool ret = true;
-
-   rcu_read_lock();
-   for_each_active_iommu(iommu, drhd) {
-   if (iommu != skip) {
-   /*
-* If the hardware is operating in the scalable
mode,
-* the snooping control is always supported
since we
-* always set PASID-table-entry.PGSNP bit if the
domain
-* is managed outside (UNMANAGED).
-*/
-   if (!sm_supported(iommu) &&
-   !ecap_sc_support(iommu->ecap)) {
-   ret = false;
-   break;
-   }
-   }
-   }
-   rcu_read_unlock();
-
-   return ret;
-}
-
  static int domain_update_iommu_superpage(struct dmar_domain *domain,
 struct intel_iommu *skip)
  {
@@ -3593,12 +3566,7 @@ static int intel_iommu_add(struct dmar_drhd_unit
*dmaru) iommu->name);
return -ENXIO;
}
-   if (!ecap_sc_support(iommu->ecap) &&
-   domain_update_iommu_snooping(iommu)) {
-   pr_warn("%s: Doesn't support snooping.\n",
-   iommu->name);
-   return -ENXIO;
-   }
+

Maybe I missed earlier patches, so this bit can also be deleted?

struct dmar_domain {
u8 iommu_snooping: 1;   /* indicate snooping control
feature */


It has been cleaned up by below commit:

71cfafda9c9b vfio: Move the Intel no-snoop control off of IOMMU_CACHE




sp = domain_update_iommu_superpage(NULL, iommu) - 1;
if (sp >= 0 && !(cap_super_page_val(iommu->cap) & (1 << sp))) {
pr_warn("%s: Doesn't support large page.\n",



Thanks,

Jacob


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


Re: [RESEND PATCH v8 00/11] Fix BUG_ON in vfio_iommu_group_notifier()

2022-05-04 Thread Joerg Roedel
On Mon, May 02, 2022 at 12:12:04PM -0400, Qian Cai wrote:
> Reverting this series fixed an user-after-free while doing SR-IOV.
> 
>  BUG: KASAN: use-after-free in __lock_acquire

Hrm, okay. I am going exclude this series from my next branch for now
until this has been sorted out.

Alex, I suggest you do the same.

Regards,

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


Re: [PATCH v7 00/36] MT8195 and MT8186 IOMMU SUPPORT

2022-05-04 Thread Joerg Roedel
On Tue, May 03, 2022 at 03:13:51PM +0800, Yong Wu wrote:
> Yong Wu (36):
>   dt-bindings: mediatek: mt8195: Add binding for MM IOMMU
>   dt-bindings: mediatek: mt8195: Add binding for infra IOMMU
>   dt-bindings: mediatek: mt8186: Add binding for MM iommu
>   iommu/mediatek: Fix 2 HW sharing pgtable issue
>   iommu/mediatek: Add list_del in mtk_iommu_remove
>   iommu/mediatek: Remove clk_disable in mtk_iommu_remove
>   iommu/mediatek: Add mutex for m4u_group and m4u_dom in data
>   iommu/mediatek: Add mutex for data in the mtk_iommu_domain
>   iommu/mediatek: Adapt sharing and non-sharing pgtable case
>   iommu/mediatek: Add 12G~16G support for multi domains
>   iommu/mediatek: Add a flag DCM_DISABLE
>   iommu/mediatek: Add a flag STD_AXI_MODE
>   iommu/mediatek: Remove the granule in the tlb flush
>   iommu/mediatek: Always enable output PA over 32bits in isr
>   iommu/mediatek: Add SUB_COMMON_3BITS flag
>   iommu/mediatek: Add IOMMU_TYPE flag
>   iommu/mediatek: Contain MM IOMMU flow with the MM TYPE
>   iommu/mediatek: Adjust device link when it is sub-common
>   iommu/mediatek: Allow IOMMU_DOMAIN_UNMANAGED for PCIe VFIO
>   iommu/mediatek: Add a PM_CLK_AO flag for infra iommu
>   iommu/mediatek: Add infra iommu support
>   iommu/mediatek: Add PCIe support
>   iommu/mediatek: Add mt8195 support
>   iommu/mediatek: Only adjust code about register base
>   iommu/mediatek: Just move code position in hw_init
>   iommu/mediatek: Separate mtk_iommu_data for v1 and v2
>   iommu/mediatek: Remove mtk_iommu.h
>   iommu/mediatek-v1: Just rename mtk_iommu to mtk_iommu_v1
>   iommu/mediatek: Add mtk_iommu_bank_data structure
>   iommu/mediatek: Initialise bank HW for each a bank
>   iommu/mediatek: Change the domid to iova_region_id
>   iommu/mediatek: Get the proper bankid for multi banks
>   iommu/mediatek: Initialise/Remove for multi bank dev
>   iommu/mediatek: Backup/restore regsiters for multi banks
>   iommu/mediatek: mt8195: Enable multi banks for infra iommu
>   iommu/mediatek: Add mt8186 iommu support

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


Re: [PATCH 3/7] dt-bindings: iommu: renesas, ipmmu-vmsa: R-Car V3U is R-Car Gen4

2022-05-04 Thread Joerg Roedel
On Mon, May 02, 2022 at 03:34:55PM +0200, Geert Uytterhoeven wrote:
> Despite the name, R-Car V3U is the first member of the R-Car Gen4
> family.  Hence move its compatible value to the R-Car Gen4 section.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  .../devicetree/bindings/iommu/renesas,ipmmu-vmsa.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Joerg Roedel 

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


Re: [PATCH] iommu: dart: Add missing module owner to ops structure

2022-05-04 Thread Joerg Roedel
On Mon, May 02, 2022 at 06:22:38PM +0900, Hector Martin wrote:
> This is required to make loading this as a module work.
> 
> Signed-off-by: Hector Martin 
> ---
>  drivers/iommu/apple-dart.c | 1 +
>  1 file changed, 1 insertion(+)

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


Re: [PATCH] iommu: iommu_group_claim_dma_owner() must always assign a domain

2022-05-04 Thread Nicolin Chen via iommu
On Tue, May 03, 2022 at 09:11:02PM -0300, Jason Gunthorpe wrote:

> This is based on Robins draft here:
> 
> https://lore.kernel.org/linux-iommu/18831161-473f-e04f-4a81-1c7062ad1...@arm.com/
> 
> With some rework. I re-organized the call chains instead of introducing
> iommu_group_user_attached(), fixed a recursive locking for
> iommu_group_get_purgatory(), and made a proper commit message.
> 
> Still only compile tested, so RFCish.
> 
> Nicolin/Lu? What do you think, can you check it?

I am able to repro the issue on ARM64 and give this a quick try.
But the patch seems to need to include the following change too.

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 94d99768023c..9bb108d01baa 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2040,7 +2040,8 @@ static int __iommu_attach_group(struct iommu_domain 
*domain,
 {
int ret;
 
-   if (group->domain && group->domain != group->default_domain)
+   if (group->domain && group->domain != group->default_domain &&
+   group->domain != group->blocking_domain)
return -EBUSY;
 
ret = __iommu_group_for_each_dev(group, domain,

> @@ -2072,38 +2072,66 @@ static int iommu_group_do_detach_device(struct device 
> *dev, void *data)
>   return 0;
>  }
>  
> -static void __iommu_detach_group(struct iommu_domain *domain,
> -  struct iommu_group *group)
> +static int __iommu_group_attach_domain(struct iommu_group *group,
> +struct iommu_domain *new_domain)
>  {
>   int ret;
>  
> + if (group->domain == new_domain)
> + return 0;
> +
>   /*
> -  * If the group has been claimed already, do not re-attach the default
> -  * domain.
> +  * A NULL domain means to call the detach_dev() op. New drivers should
> +  * use a IOMMU_DOMAIN_IDENTITY domain instead of a NULL default_domain

an IOMMU_DOMAIN_IDENTITY?

Just a nit here. I will take a closer look at the change tomorrow.

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


[bug] NULL pointer deref after 3f6634d997db ("iommu: Use right way to retrieve iommu_ops")

2022-05-04 Thread Jan Stancek

On Wed, Feb 16, 2022 at 10:52:47AM +0800, Lu Baolu wrote:

The common iommu_ops is hooked to both device and domain. When a helper
has both device and domain pointer, the way to get the iommu_ops looks
messy in iommu core. This sorts out the way to get iommu_ops. The device
related helpers go through device pointer, while the domain related ones
go through domain pointer.

Signed-off-by: Lu Baolu 
Reviewed-by: Christoph Hellwig 
Reviewed-by: Jason Gunthorpe 
---
include/linux/iommu.h | 11 ++
drivers/iommu/iommu.c | 50 +--
2 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 9ffa2e88f3d0..90f1b5e3809d 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -381,6 +381,17 @@ static inline void iommu_iotlb_gather_init(struct 
iommu_iotlb_gather *gather)
};
}

+static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
+{
+   /*
+* Assume that valid ops must be installed if iommu_probe_device()
+* has succeeded. The device ops are essentially for internal use
+* within the IOMMU subsystem itself, so we should be able to trust
+* ourselves not to misuse the helper.
+*/
+   return dev->iommu->iommu_dev->ops;
+}
+
#define IOMMU_GROUP_NOTIFY_ADD_DEVICE   1 /* Device added */
#define IOMMU_GROUP_NOTIFY_DEL_DEVICE   2 /* Pre Device removed */
#define IOMMU_GROUP_NOTIFY_BIND_DRIVER  3 /* Pre Driver bind */
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 7cf073c56036..7af0ee670deb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -323,13 +323,14 @@ int iommu_probe_device(struct device *dev)

void iommu_release_device(struct device *dev)
{
-   const struct iommu_ops *ops = dev->bus->iommu_ops;
+   const struct iommu_ops *ops;

if (!dev->iommu)
return;

iommu_device_unlink(dev->iommu->iommu_dev, dev);

+   ops = dev_iommu_ops(dev);
ops->release_device(dev);

iommu_group_remove_device(dev);
@@ -833,8 +834,10 @@ static int iommu_create_device_direct_mappings(struct 
iommu_group *group,
static bool iommu_is_attach_deferred(struct iommu_domain *domain,
 struct device *dev)
{
-   if (domain->ops->is_attach_deferred)
-   return domain->ops->is_attach_deferred(domain, dev);
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
+
+   if (ops->is_attach_deferred)
+   return ops->is_attach_deferred(domain, dev);

return false;
}
@@ -1252,10 +1255,10 @@ int iommu_page_response(struct device *dev,
struct iommu_fault_event *evt;
struct iommu_fault_page_request *prm;
struct dev_iommu *param = dev->iommu;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
bool has_pasid = msg->flags & IOMMU_PAGE_RESP_PASID_VALID;
-   struct iommu_domain *domain = iommu_get_domain_for_dev(dev);

-   if (!domain || !domain->ops->page_response)
+   if (!ops->page_response)
return -ENODEV;

if (!param || !param->fault_param)
@@ -1296,7 +1299,7 @@ int iommu_page_response(struct device *dev,
msg->pasid = 0;
}

-   ret = domain->ops->page_response(dev, evt, msg);
+   ret = ops->page_response(dev, evt, msg);
list_del(>list);
kfree(evt);
break;
@@ -1521,7 +1524,7 @@ EXPORT_SYMBOL_GPL(fsl_mc_device_group);

static int iommu_get_def_domain_type(struct device *dev)
{
-   const struct iommu_ops *ops = dev->bus->iommu_ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);

if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
return IOMMU_DOMAIN_DMA;
@@ -1580,7 +1583,7 @@ static int iommu_alloc_default_domain(struct iommu_group 
*group,
 */
static struct iommu_group *iommu_group_get_for_dev(struct device *dev)
{
-   const struct iommu_ops *ops = dev->bus->iommu_ops;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);
struct iommu_group *group;
int ret;

@@ -1588,9 +1591,6 @@ static struct iommu_group *iommu_group_get_for_dev(struct 
device *dev)
if (group)
return group;

-   if (!ops)
-   return ERR_PTR(-EINVAL);
-
group = ops->device_group(dev);
if (WARN_ON_ONCE(group == NULL))
return ERR_PTR(-EINVAL);
@@ -1759,10 +1759,10 @@ static int __iommu_group_dma_attach(struct iommu_group 
*group)

static int iommu_group_do_probe_finalize(struct device *dev, void *data)
{
-   struct iommu_domain *domain = data;
+   const struct iommu_ops *ops = dev_iommu_ops(dev);

-   if (domain->ops->probe_finalize)
-   domain->ops->probe_finalize(dev);
+   if (ops->probe_finalize)
+   ops->probe_finalize(dev);

return 0;
}
@@ -2020,7 +2020,7 @@ 

Re: [PATCH 3/5] iommu/vt-d: Check domain force_snooping against attached devices

2022-05-04 Thread Baolu Lu

On 2022/5/3 05:31, Jacob Pan wrote:

Hi BaoLu,


Hi Jacob,



On Sun, 1 May 2022 19:24:32 +0800, Lu Baolu 
wrote:


As domain->force_snooping only impacts the devices attached with the
domain, there's no need to check against all IOMMU units. At the same
time, for a brand new domain (hasn't been attached to any device), the
force_snooping field could be set, but the attach_dev callback will
return failure if it wants to attach to a device which IOMMU has no
snoop control capability.

Signed-off-by: Lu Baolu 
---
  drivers/iommu/intel/pasid.h |  2 ++
  drivers/iommu/intel/iommu.c | 50 -
  drivers/iommu/intel/pasid.c | 18 +
  3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index ab4408c824a5..583ea67fc783 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -123,4 +123,6 @@ void intel_pasid_tear_down_entry(struct intel_iommu
*iommu, bool fault_ignore);
  int vcmd_alloc_pasid(struct intel_iommu *iommu, u32 *pasid);
  void vcmd_free_pasid(struct intel_iommu *iommu, u32 pasid);
+void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
+ struct device *dev, u32 pasid);
  #endif /* __INTEL_PASID_H */
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 98050943d863..3c1c228f9031 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4554,13 +4554,61 @@ static phys_addr_t
intel_iommu_iova_to_phys(struct iommu_domain *domain, return phys;
  }
  
+static bool domain_support_force_snooping(struct dmar_domain *domain)

+{
+   struct device_domain_info *info;
+   unsigned long flags;
+   bool support = true;
+
+   spin_lock_irqsave(_domain_lock, flags);
+   if (list_empty(>devices))
+   goto out;
+
+   list_for_each_entry(info, >devices, link) {
+   if (!ecap_sc_support(info->iommu->ecap)) {
+   support = false;
+   break;
+   }
+   }

why not just check the flag dmar_domain->force_snooping? devices wouldn't
be able to attach if !ecap_sc, right?


I should check "dmar_domain->force_snooping" first. If this is the first
time that this flag is about to set, then check the capabilities.




+out:
+   spin_unlock_irqrestore(_domain_lock, flags);
+   return support;
+}
+
+static void domain_set_force_snooping(struct dmar_domain *domain)
+{
+   struct device_domain_info *info;
+   unsigned long flags;
+
+   /*
+* Second level page table supports per-PTE snoop control. The
+* iommu_map() interface will handle this by setting SNP bit.
+*/
+   if (!domain_use_first_level(domain))
+   return;
+
+   spin_lock_irqsave(_domain_lock, flags);
+   if (list_empty(>devices))
+   goto out_unlock;
+
+   list_for_each_entry(info, >devices, link)
+   intel_pasid_setup_page_snoop_control(info->iommu,
info->dev,
+PASID_RID2PASID);
+

I guess other DMA API PASIDs need to have sc bit set as well. I will keep
this in mind for my DMA API PASID patch.


Kernel DMA don't need to set the PGSNP bit. The x86 arch is always DMA
coherent. The force snooping is only needed when the device is
controlled by user space, but the VMM is optimized not to support the
virtualization of the wbinv instruction.




+out_unlock:
+   spin_unlock_irqrestore(_domain_lock, flags);
+}
+
  static bool intel_iommu_enforce_cache_coherency(struct iommu_domain
*domain) {
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
  
-	if (!domain_update_iommu_snooping(NULL))

+   if (!domain_support_force_snooping(dmar_domain))
return false;
+
+   domain_set_force_snooping(dmar_domain);
dmar_domain->force_snooping = true;
+

nit: spurious change


I expect a blank line before return in the end.


return true;
  }
  
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c

index f8d215d85695..815c744e6a34 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -762,3 +762,21 @@ int intel_pasid_setup_pass_through(struct
intel_iommu *iommu,
return 0;
  }
+
+/*
+ * Set the page snoop control for a pasid entry which has been set up.
+ */
+void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
+ struct device *dev, u32 pasid)
+{
+   struct pasid_entry *pte;
+   u16 did;
+
+   pte = intel_pasid_get_entry(dev, pasid);
+   if (WARN_ON(!pte || !pasid_pte_is_present(pte)))
+   return;
+
+   pasid_set_pgsnp(pte);
+   did = pasid_get_domain_id(pte);
+   pasid_flush_caches(iommu, pte, pasid, did);
+}



Thanks,

Jacob


Best regards,
baolu
___
iommu mailing list

Re: [PATCH 3/5] iommu/vt-d: Check domain force_snooping against attached devices

2022-05-04 Thread Baolu Lu

On 2022/5/2 21:17, Jason Gunthorpe wrote:

On Sun, May 01, 2022 at 07:24:32PM +0800, Lu Baolu wrote:

+static bool domain_support_force_snooping(struct dmar_domain *domain)
+{
+   struct device_domain_info *info;
+   unsigned long flags;
+   bool support = true;
+
+   spin_lock_irqsave(_domain_lock, flags);
+   if (list_empty(>devices))
+   goto out;


Why? list_for_each_entry will just do nothing..


Yes. I will remove above two lines.




+   list_for_each_entry(info, >devices, link) {
+   if (!ecap_sc_support(info->iommu->ecap)) {
+   support = false;
+   break;
+   }
+   }
+out:
+   spin_unlock_irqrestore(_domain_lock, flags);
+   return support;
+}
+
+static void domain_set_force_snooping(struct dmar_domain *domain)
+{
+   struct device_domain_info *info;
+   unsigned long flags;
+
+   /*
+* Second level page table supports per-PTE snoop control. The
+* iommu_map() interface will handle this by setting SNP bit.
+*/
+   if (!domain_use_first_level(domain))
+   return;
+
+   spin_lock_irqsave(_domain_lock, flags);
+   if (list_empty(>devices))
+   goto out_unlock;
+
+   list_for_each_entry(info, >devices, link)
+   intel_pasid_setup_page_snoop_control(info->iommu, info->dev,
+PASID_RID2PASID);
+
+out_unlock:
+   spin_unlock_irqrestore(_domain_lock, flags);
+}
+
  static bool intel_iommu_enforce_cache_coherency(struct iommu_domain *domain)
  {
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
  
-	if (!domain_update_iommu_snooping(NULL))

+   if (!domain_support_force_snooping(dmar_domain))
return false;


Maybe exit early if force_snooping = true?


Yes, should check "force_snooping = true" and return directly if
force_snooping has already been set. As you pointed below, the new
domain_attach should take care of this flag as well. Thanks!




+   domain_set_force_snooping(dmar_domain);
dmar_domain->force_snooping = true;
+
return true;
  }
  
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c

index f8d215d85695..815c744e6a34 100644
+++ b/drivers/iommu/intel/pasid.c
@@ -762,3 +762,21 @@ int intel_pasid_setup_pass_through(struct intel_iommu 
*iommu,
  
  	return 0;

  }
+
+/*
+ * Set the page snoop control for a pasid entry which has been set up.
+ */


So the 'first level' is only used with pasid?


Yes. A fake pasid (RID2PASID in spec) is used for legacy transactions
(those w/o pasid).




+void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
+ struct device *dev, u32 pasid)
+{
+   struct pasid_entry *pte;
+   u16 did;
+
+   pte = intel_pasid_get_entry(dev, pasid);
+   if (WARN_ON(!pte || !pasid_pte_is_present(pte)))
+   return;
+
+   pasid_set_pgsnp(pte);


Doesn't this need to be done in other places too, like when a new attach
is made? Patch 5 removed it, but should that be made if
domain->force_snooping?


Yes. I missed this. Will take care of this in the next version.



Jason


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


Re: [PATCH 2/5] iommu/vt-d: Set SNP bit only in second-level page table entries

2022-05-04 Thread Baolu Lu

Hi Jason,

On 2022/5/2 21:05, Jason Gunthorpe wrote:

On Sun, May 01, 2022 at 07:24:31PM +0800, Lu Baolu wrote:

The SNP bit is only valid for second-level PTEs. Setting this bit in the
first-level PTEs has no functional impact because the Intel IOMMU always
ignores the same bit in first-level PTEs. Anyway, let's check the page
table type before setting SNP bit in PTEs to make the code more readable.

Shouldn't this be tested before setting force_snooping and not during
every map?


The check is in the following patch. This just makes sure that SNP is
only set in second-level page table entries.

Best regards,
baolu

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