Re: [RFC PATCH] component: Add common helpers for compare/release functions

2022-01-28 Thread Daniel Vetter
On Fri, Jan 28, 2022 at 04:11:01PM +0800, Yong Wu wrote:
> The component requires the compare/release functions, there are so many
> copy in current kernel. Just define three common helpers for them.
> No functional change.
> 
> Signed-off-by: Yong Wu 
> ---
> Base on v5.17-rc1
> ---
>  .../gpu/drm/arm/display/komeda/komeda_drv.c|  5 -
>  drivers/gpu/drm/arm/hdlcd_drv.c|  7 +--
>  drivers/gpu/drm/armada/armada_drv.c|  5 -
>  drivers/gpu/drm/drm_of.c   |  8 +---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  7 ---
>  drivers/gpu/drm/exynos/exynos_drm_drv.c|  5 -
>  .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c|  5 -
>  drivers/gpu/drm/imx/imx-drm-core.c |  4 ++--
>  drivers/gpu/drm/ingenic/ingenic-drm-drv.c  |  5 -
>  drivers/gpu/drm/mcde/mcde_drv.c|  7 +--
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c |  5 -
>  drivers/gpu/drm/meson/meson_drv.c  |  8 
>  drivers/gpu/drm/msm/msm_drv.c  |  9 -
>  drivers/gpu/drm/omapdrm/dss/dss.c  |  8 +---
>  drivers/gpu/drm/rockchip/rockchip_drm_drv.c|  5 -
>  drivers/gpu/drm/sti/sti_drv.c  |  5 -
>  drivers/gpu/drm/sun4i/sun4i_drv.c  |  9 -
>  drivers/gpu/drm/vc4/vc4_drv.c  |  5 -
>  drivers/iommu/mtk_iommu.h  | 10 --
>  drivers/power/supply/ab8500_charger.c  |  8 +---
>  drivers/video/fbdev/omap2/omapfb/dss/dss.c |  8 +---
>  include/linux/component.h  | 18 ++
>  sound/soc/codecs/wcd938x.c | 16 ++--

Seems like a neat idea. Please add kerneldoc for the new functions you're
adding (bonus point for an example in there) and make sure it all renders
correctly in

$ make htmldoc

Also please split up the patch series per-driver and add the maintainers
to each patches' Cc: list. With that I think this should be ready for
merging.

Cheers, Daniel

>  23 files changed, 28 insertions(+), 144 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
> b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> index e7933930a657..fe5b97107417 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> @@ -92,11 +92,6 @@ static const struct component_master_ops komeda_master_ops 
> = {
>   .unbind = komeda_unbind,
>  };
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
>  static void komeda_add_slave(struct device *master,
>struct component_match **match,
>struct device_node *np,
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 479c2422a2e0..36d84c439df8 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -372,11 +372,6 @@ static const struct component_master_ops 
> hdlcd_master_ops = {
>   .unbind = hdlcd_drm_unbind,
>  };
>  
> -static int compare_dev(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
>  static int hdlcd_probe(struct platform_device *pdev)
>  {
>   struct device_node *port;
> @@ -387,7 +382,7 @@ static int hdlcd_probe(struct platform_device *pdev)
>   if (!port)
>   return -ENODEV;
>  
> - drm_of_component_match_add(>dev, , compare_dev, port);
> + drm_of_component_match_add(>dev, , compare_of, port);
>   of_node_put(port);
>  
>   return component_master_add_with_match(>dev, _master_ops,
> diff --git a/drivers/gpu/drm/armada/armada_drv.c 
> b/drivers/gpu/drm/armada/armada_drv.c
> index 8e3e98f13db4..9edc4912c1a0 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -177,11 +177,6 @@ static void armada_drm_unbind(struct device *dev)
>   drm_mm_takedown(>linear);
>  }
>  
> -static int compare_of(struct device *dev, void *data)
> -{
> - return dev->of_node == data;
> -}
> -
>  static int compare_dev_name(struct device *dev, void *data)
>  {
>   const char *name = data;
> diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> index 59d368ea006b..f958f48f8ba4 100644
> --- a/drivers/gpu/drm/drm_of.c
> +++ b/drivers/gpu/drm/drm_of.c
> @@ -18,11 +18,6 @@
>   * properties.
>   */
>  
> -static void drm_release_of(struct device *dev, void *data)
> -{
> - of_node_put(data);
> -}
> -
>  /**
>   * drm_of_crtc_port_mask - find the mask of a registered CRTC by port OF node
>   * @dev: DRM device
> @@ -94,8 +89,7 @@ void drm_of_component_match_add(struct device *master,
>   struct device_node *node)
>  {
>   of_node_get(node);
> - component_match_add_release(master, matchptr, drm_release_of,
> - compare, 

[PATCH v3 10/11] tools/objtool: Check for use of the ENQCMD instruction in the kernel

2022-01-28 Thread Fenghua Yu
The ENQCMD implicitly accesses the PASID_MSR to fill in the pasid field
of the descriptor being submitted to an accelerator. But there is no
precise (and stable across kernel changes) point at which the PASID_MSR
is updated from the value for one task to the next.

Kernel code that uses accelerators must always use the ENQCMDS instruction
which does not access the PASID_MSR.

Check for use of the ENQCMD instruction in the kernel and warn on its
usage.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
Acked-by: Josh Poimboeuf 
---
v3:
- Add Acked-by: Josh Poimboeuf 

v2:
- Simplify handling ENQCMD (PeterZ and Josh)

 tools/objtool/arch/x86/decode.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index c10ef78df050..479e769ca324 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -112,7 +112,7 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec
const struct elf *elf = file->elf;
struct insn insn;
int x86_64, ret;
-   unsigned char op1, op2,
+   unsigned char op1, op2, op3,
  rex = 0, rex_b = 0, rex_r = 0, rex_w = 0, rex_x = 0,
  modrm = 0, modrm_mod = 0, modrm_rm = 0, modrm_reg = 0,
  sib = 0, /* sib_scale = 0, */ sib_index = 0, sib_base = 0;
@@ -139,6 +139,7 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec
 
op1 = insn.opcode.bytes[0];
op2 = insn.opcode.bytes[1];
+   op3 = insn.opcode.bytes[2];
 
if (insn.rex_prefix.nbytes) {
rex = insn.rex_prefix.bytes[0];
@@ -491,6 +492,14 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec
/* nopl/nopw */
*type = INSN_NOP;
 
+   } else if (op2 == 0x38 && op3 == 0xf8) {
+   if (insn.prefixes.nbytes == 1 &&
+   insn.prefixes.bytes[0] == 0xf2) {
+   /* ENQCMD cannot be used in the kernel. */
+   WARN("ENQCMD instruction at %s:%lx", sec->name,
+offset);
+   }
+
} else if (op2 == 0xa0 || op2 == 0xa8) {
 
/* push fs/gs */
-- 
2.35.0

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


[PATCH v3 03/11] iommu/ioasid: Introduce a helper to check for valid PASIDs

2022-01-28 Thread Fenghua Yu
pasid_valid() is defined to check if a given PASID is valid.

Suggested-by: Ashok Raj 
Suggested-by: Jacob Pan 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Define a helper pasid_valid() (Ashok, Jacob, Thomas, Tony)

 include/linux/ioasid.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/include/linux/ioasid.h b/include/linux/ioasid.h
index e9dacd4b9f6b..2237f64dbaae 100644
--- a/include/linux/ioasid.h
+++ b/include/linux/ioasid.h
@@ -41,6 +41,10 @@ void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid,
 int ioasid_register_allocator(struct ioasid_allocator_ops *allocator);
 void ioasid_unregister_allocator(struct ioasid_allocator_ops *allocator);
 int ioasid_set_data(ioasid_t ioasid, void *data);
+static inline bool pasid_valid(ioasid_t ioasid)
+{
+   return ioasid != INVALID_IOASID;
+}
 
 #else /* !CONFIG_IOASID */
 static inline ioasid_t ioasid_alloc(struct ioasid_set *set, ioasid_t min,
@@ -78,5 +82,10 @@ static inline int ioasid_set_data(ioasid_t ioasid, void 
*data)
return -ENOTSUPP;
 }
 
+static inline bool pasid_valid(ioasid_t ioasid)
+{
+   return false;
+}
+
 #endif /* CONFIG_IOASID */
 #endif /* __LINUX_IOASID_H */
-- 
2.35.0

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


[PATCH v3 11/11] docs: x86: Change documentation for SVA (Shared Virtual Addressing)

2022-01-28 Thread Fenghua Yu
Since allocating, freeing and fixing up PASID are changed, the
documentation is updated to reflect the changes.

Originally-by: Ashok Raj 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v3:
- Remove PASID refcount description (Thomas).

v2:
- Update life cycle info (Tony and Thomas).
- Update initial PASID value to INVALID_IOASID on fork().

 Documentation/x86/sva.rst | 53 ++-
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/Documentation/x86/sva.rst b/Documentation/x86/sva.rst
index 076efd51ef1f..1a22020735a3 100644
--- a/Documentation/x86/sva.rst
+++ b/Documentation/x86/sva.rst
@@ -104,18 +104,47 @@ The MSR must be configured on each logical CPU before any 
application
 thread can interact with a device. Threads that belong to the same
 process share the same page tables, thus the same MSR value.
 
-PASID is cleared when a process is created. The PASID allocation and MSR
-programming may occur long after a process and its threads have been created.
-One thread must call iommu_sva_bind_device() to allocate the PASID for the
-process. If a thread uses ENQCMD without the MSR first being populated, a #GP
-will be raised. The kernel will update the PASID MSR with the PASID for all
-threads in the process. A single process PASID can be used simultaneously
-with multiple devices since they all share the same address space.
-
-One thread can call iommu_sva_unbind_device() to free the allocated PASID.
-The kernel will clear the PASID MSR for all threads belonging to the process.
-
-New threads inherit the MSR value from the parent.
+PASID Life Cycle Management
+==
+
+PASID is initialized as INVALID_IOASID (-1) when a process is created.
+
+Only processes that access SVA-capable devices need to have a PASID
+allocated. This allocation happens when a process opens/binds an SVA-capable
+device but finds no PASID for this process. Subsequent binds of the same, or
+other devices will share the same PASID.
+
+Although the PASID is allocated to the process by opening a device,
+it is not active in any of the threads of that process. It's loaded to the
+IA32_PASID MSR lazily when a thread tries to submit a work descriptor
+to a device using the ENQCMD.
+
+That first access will trigger a #GP fault because the IA32_PASID MSR
+has not been initialized with the PASID value assigned to the process
+when the device was opened. The Linux #GP handler notes that a PASID has
+been allocated for the process, and so initializes the IA32_PASID MSR
+and returns so that the ENQCMD instruction is re-executed.
+
+On fork(2) or exec(2) the PASID is removed from the process as it no
+longer has the same address space that it had when the device was opened.
+
+On clone(2) the new task shares the same address space, so will be
+able to use the PASID allocated to the process. The IA32_PASID is not
+preemptively initialized as the PASID value might not be allocated yet or
+the kernel does not know whether this thread is going to access the device
+and the cleared IA32_PASID MSR reduces context switch overhead by xstate
+init optimization. Since #GP faults have to be handled on any threads that
+were created before the PASID was assigned to the mm of the process, newly
+created threads might as well be treated in a consistent way.
+
+Due to complexity of freeing the PASID and clearing all IA32_PASID MSRs in
+all threads in unbind, free the PASID lazily only on mm exit.
+
+If a process does a close(2) of the device file descriptor and munmap(2)
+of the device MMIO portal, then the driver will unbind the device. The
+PASID is still marked VALID in the PASID_MSR for any threads in the
+process that accessed the device. But this is harmless as without the
+MMIO portal they cannot submit new work to the device.
 
 Relationships
 =
-- 
2.35.0

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


[PATCH v3 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit

2022-01-28 Thread Fenghua Yu
To avoid complexity of updating each thread's PASID status (e.g. sending
IPI to update IA32_PASID MSR) on allocating and freeing PASID, once
allocated and assigned to an mm, the PASID stays with the mm for the
rest of the mm's lifetime. A reference to the PASID is taken on
allocating the PASID. Binding/unbinding the PASID won't change refcount.
The reference is dropped on mm exit and thus the PASID is freed.

Two helpers mm_pasid_set() and mm_pasid_drop() are defined in mm because
the PASID operations handle the pasid member in mm_struct and should be
part of mm operations. Because IOASID's reference count is not used any
more and removed, unused ioasid_get() and iommu_sva_free_pasid()
are deleted and ioasid_put() is renamed to ioasid_free().

20-bit PASID allows up to 1M processes bound to PASIDs at the same time.
With cgroups and other controls that might limit the number of process
creation, the limited number of PASIDs is not a realistic issue for
lazy PASID free.

Suggested-by: Dave Hansen 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v3:
- Rename mm_pasid_get() to mm_pasid_set() (Thomas).
- Remove ioasid_get() because it's not used any more when the IOASID
  is freed on mm exit (Thomas).
- Remove PASID's refcount exercise in ioasid_put() and rename
  ioasid_put() to ioasid_free() (Thomas).

v2:
- Free PASID on mm exit instead of in exit(2) or unbind() (Thomas, AndyL,
  PeterZ)
- Add mm_pasid_init(), mm_pasid_get(), and mm_pasid_drop() functions in mm.
  So the mm's PASID operations are generic for both X86 and ARM
  (Dave Hansen)

 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  5 +--
 drivers/iommu/intel/iommu.c   |  4 +-
 drivers/iommu/intel/svm.c |  9 -
 drivers/iommu/ioasid.c| 38 ++
 drivers/iommu/iommu-sva-lib.c | 39 ++-
 drivers/iommu/iommu-sva-lib.h |  1 -
 include/linux/ioasid.h| 12 +-
 include/linux/sched/mm.h  | 16 
 kernel/fork.c |  1 +
 9 files changed, 38 insertions(+), 87 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c 
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index a737ba5f727e..22ddd05bbdcd 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -340,14 +340,12 @@ __arm_smmu_sva_bind(struct device *dev, struct mm_struct 
*mm)
bond->smmu_mn = arm_smmu_mmu_notifier_get(smmu_domain, mm);
if (IS_ERR(bond->smmu_mn)) {
ret = PTR_ERR(bond->smmu_mn);
-   goto err_free_pasid;
+   goto err_free_bond;
}
 
list_add(>list, >bonds);
return >sva;
 
-err_free_pasid:
-   iommu_sva_free_pasid(mm);
 err_free_bond:
kfree(bond);
return ERR_PTR(ret);
@@ -377,7 +375,6 @@ void arm_smmu_sva_unbind(struct iommu_sva *handle)
if (refcount_dec_and_test(>refs)) {
list_del(>list);
arm_smmu_mmu_notifier_put(bond->smmu_mn);
-   iommu_sva_free_pasid(bond->mm);
kfree(bond);
}
mutex_unlock(_lock);
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 92fea3fbbb11..ef03b2176bbd 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4781,7 +4781,7 @@ static int aux_domain_add_dev(struct dmar_domain *domain,
 link_failed:
spin_unlock_irqrestore(_domain_lock, flags);
if (list_empty(>subdevices) && domain->default_pasid > 0)
-   ioasid_put(domain->default_pasid);
+   ioasid_free(domain->default_pasid);
 
return ret;
 }
@@ -4811,7 +4811,7 @@ static void aux_domain_remove_dev(struct dmar_domain 
*domain,
spin_unlock_irqrestore(_domain_lock, flags);
 
if (list_empty(>subdevices) && domain->default_pasid > 0)
-   ioasid_put(domain->default_pasid);
+   ioasid_free(domain->default_pasid);
 }
 
 static int prepare_domain_attach_device(struct iommu_domain *domain,
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index 5b5d69b04fcc..51ac2096b3da 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -514,11 +514,6 @@ static int intel_svm_alloc_pasid(struct device *dev, 
struct mm_struct *mm,
return iommu_sva_alloc_pasid(mm, PASID_MIN, max_pasid - 1);
 }
 
-static void intel_svm_free_pasid(struct mm_struct *mm)
-{
-   iommu_sva_free_pasid(mm);
-}
-
 static struct iommu_sva *intel_svm_bind_mm(struct intel_iommu *iommu,
   struct device *dev,
   struct mm_struct *mm,
@@ -662,8 +657,6 @@ static int intel_svm_unbind_mm(struct device *dev, u32 
pasid)
kfree(svm);
}
}
-   /* Drop a PASID reference 

[PATCH v3 08/11] x86/traps: Demand-populate PASID MSR via #GP

2022-01-28 Thread Fenghua Yu
All tasks start with PASID state disabled. This means that the first
time they execute an ENQCMD instruction they will take a #GP fault.

Modify the #GP fault handler to check if the "mm" for the task has
already been allocated a PASID. If so, try to fix the #GP fault by
loading the IA32_PASID MSR.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Directly write IA32_PASID MSR in fixup while local IRQ is still disabled
  (Thomas)
- Move #ifdef over to CONFIG_IOMMU_SVA since it is what
  defines mm->pasid and ->pasid_activated (Dave Hansen).
- Rename try_fixup_pasid() -> try_fixup_enqcmd_gp(). This
  code really is highly specific to ENQCMD, not PASIDs (Dave Hansen).
- Add lockdep assert and comment about context (Dave Hansen).
- Re-flow the if() mess (Dave Hansen).

 arch/x86/kernel/traps.c | 55 +
 1 file changed, 55 insertions(+)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c9d566dcf89a..7ef00dee35be 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -559,6 +560,57 @@ static bool fixup_iopl_exception(struct pt_regs *regs)
return true;
 }
 
+/*
+ * The unprivileged ENQCMD instruction generates #GPs if the
+ * IA32_PASID MSR has not been populated.  If possible, populate
+ * the MSR from a PASID previously allocated to the mm.
+ */
+static bool try_fixup_enqcmd_gp(void)
+{
+#ifdef CONFIG_IOMMU_SVA
+   u32 pasid;
+
+   /*
+* MSR_IA32_PASID is managed using XSAVE.  Directly
+* writing to the MSR is only possible when fpregs
+* are valid and the fpstate is not.  This is
+* guaranteed when handling a userspace exception
+* in *before* interrupts are re-enabled.
+*/
+   lockdep_assert_irqs_disabled();
+
+   /*
+* Hardware without ENQCMD will not generate
+* #GPs that can be fixed up here.
+*/
+   if (!cpu_feature_enabled(X86_FEATURE_ENQCMD))
+   return false;
+
+   pasid = current->mm->pasid;
+
+   /*
+* If the mm has not been allocated a
+* PASID, the #GP can not be fixed up.
+*/
+   if (!pasid_valid(pasid))
+   return false;
+
+   /*
+* Did this thread already have its PASID activated?
+* If so, the #GP must be from something else.
+*/
+   if (current->pasid_activated)
+   return false;
+
+   wrmsrl(MSR_IA32_PASID, pasid | MSR_IA32_PASID_VALID);
+   current->pasid_activated = 1;
+
+   return true;
+#else
+   return false;
+#endif
+}
+
 DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
 {
char desc[sizeof(GPFSTR) + 50 + 2*sizeof(unsigned long) + 1] = GPFSTR;
@@ -567,6 +619,9 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_protection)
unsigned long gp_addr;
int ret;
 
+   if (user_mode(regs) && try_fixup_enqcmd_gp())
+   return;
+
cond_local_irq_enable(regs);
 
if (static_cpu_has(X86_FEATURE_UMIP)) {
-- 
2.35.0

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


[PATCH v3 01/11] iommu/sva: Rename CONFIG_IOMMU_SVA_LIB to CONFIG_IOMMU_SVA

2022-01-28 Thread Fenghua Yu
This CONFIG option originally only referred to the Shared
Virtual Address (SVA) library. But it is now also used for
non-library portions of code.

Drop the "_LIB" suffix so that there is just one configuration
options for all code relating to SVA.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Add this patch for more meaningful name CONFIG_IOMMU_SVA

 drivers/iommu/Kconfig | 6 +++---
 drivers/iommu/Makefile| 2 +-
 drivers/iommu/intel/Kconfig   | 2 +-
 drivers/iommu/iommu-sva-lib.h | 6 +++---
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 3eb68fa1b8cc..c79a0df090c0 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -144,8 +144,8 @@ config IOMMU_DMA
select IRQ_MSI_IOMMU
select NEED_SG_DMA_LENGTH
 
-# Shared Virtual Addressing library
-config IOMMU_SVA_LIB
+# Shared Virtual Addressing
+config IOMMU_SVA
bool
select IOASID
 
@@ -379,7 +379,7 @@ config ARM_SMMU_V3
 config ARM_SMMU_V3_SVA
bool "Shared Virtual Addressing support for the ARM SMMUv3"
depends on ARM_SMMU_V3
-   select IOMMU_SVA_LIB
+   select IOMMU_SVA
select MMU_NOTIFIER
help
  Support for sharing process address spaces with devices using the
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index bc7f730edbb0..44475a9b3eea 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -27,6 +27,6 @@ obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
 obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
 obj-$(CONFIG_HYPERV_IOMMU) += hyperv-iommu.o
 obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
-obj-$(CONFIG_IOMMU_SVA_LIB) += iommu-sva-lib.o io-pgfault.o
+obj-$(CONFIG_IOMMU_SVA) += iommu-sva-lib.o io-pgfault.o
 obj-$(CONFIG_SPRD_IOMMU) += sprd-iommu.o
 obj-$(CONFIG_APPLE_DART) += apple-dart.o
diff --git a/drivers/iommu/intel/Kconfig b/drivers/iommu/intel/Kconfig
index 247d0f2d5fdf..39a06d245f12 100644
--- a/drivers/iommu/intel/Kconfig
+++ b/drivers/iommu/intel/Kconfig
@@ -52,7 +52,7 @@ config INTEL_IOMMU_SVM
select PCI_PRI
select MMU_NOTIFIER
select IOASID
-   select IOMMU_SVA_LIB
+   select IOMMU_SVA
help
  Shared Virtual Memory (SVM) provides a facility for devices
  to access DMA resources through process address space by
diff --git a/drivers/iommu/iommu-sva-lib.h b/drivers/iommu/iommu-sva-lib.h
index 031155010ca8..95dc3ebc1928 100644
--- a/drivers/iommu/iommu-sva-lib.h
+++ b/drivers/iommu/iommu-sva-lib.h
@@ -17,7 +17,7 @@ struct device;
 struct iommu_fault;
 struct iopf_queue;
 
-#ifdef CONFIG_IOMMU_SVA_LIB
+#ifdef CONFIG_IOMMU_SVA
 int iommu_queue_iopf(struct iommu_fault *fault, void *cookie);
 
 int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev);
@@ -28,7 +28,7 @@ struct iopf_queue *iopf_queue_alloc(const char *name);
 void iopf_queue_free(struct iopf_queue *queue);
 int iopf_queue_discard_partial(struct iopf_queue *queue);
 
-#else /* CONFIG_IOMMU_SVA_LIB */
+#else /* CONFIG_IOMMU_SVA */
 static inline int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
 {
return -ENODEV;
@@ -64,5 +64,5 @@ static inline int iopf_queue_discard_partial(struct 
iopf_queue *queue)
 {
return -ENODEV;
 }
-#endif /* CONFIG_IOMMU_SVA_LIB */
+#endif /* CONFIG_IOMMU_SVA */
 #endif /* _IOMMU_SVA_LIB_H */
-- 
2.35.0

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


[PATCH v3 07/11] sched: Define and initialize a flag to identify valid PASID in the task

2022-01-28 Thread Fenghua Yu
From: Peter Zijlstra 

Add a new single bit field to the task structure to track whether this task
has initialized the IA32_PASID MSR to the mm's PASID.

Initialize the field to zero when creating a new task with fork/clone.

Signed-off-by: Peter Zijlstra 
Co-developed-by: Fenghua Yu 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Change condition to more accurate CONFIG_IOMMU_SVA (Jacob)

 include/linux/sched.h | 3 +++
 kernel/fork.c | 4 
 2 files changed, 7 insertions(+)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f5b2be39a78c..812e40c5bde5 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -938,6 +938,9 @@ struct task_struct {
/* Recursion prevention for eventfd_signal() */
unsignedin_eventfd_signal:1;
 #endif
+#ifdef CONFIG_IOMMU_SVA
+   unsignedpasid_activated:1;
+#endif
 
unsigned long   atomic_flags; /* Flags requiring atomic 
access. */
 
diff --git a/kernel/fork.c b/kernel/fork.c
index c03c6682464c..51fd1df994b7 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -968,6 +968,10 @@ static struct task_struct *dup_task_struct(struct 
task_struct *orig, int node)
tsk->use_memdelay = 0;
 #endif
 
+#ifdef CONFIG_IOMMU_SVA
+   tsk->pasid_activated = 0;
+#endif
+
 #ifdef CONFIG_MEMCG
tsk->active_memcg = NULL;
 #endif
-- 
2.35.0

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


[PATCH v3 09/11] x86/cpufeatures: Re-enable ENQCMD

2022-01-28 Thread Fenghua Yu
Since ENQCMD is handled by #GP fix up, it can be re-enabled.

The ENQCMD feature can only be used if CONFIG_INTEL_IOMMU_SVM is set. Add
X86_FEATURE_ENQCMD to the disabled features mask as appropriate so that
cpu_feature_enabled() can be used to check the feature.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Update the commit message (Tony).

 arch/x86/include/asm/disabled-features.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/disabled-features.h 
b/arch/x86/include/asm/disabled-features.h
index 8f28fafa98b3..1231d63f836d 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -56,8 +56,11 @@
 # define DISABLE_PTI   (1 << (X86_FEATURE_PTI & 31))
 #endif
 
-/* Force disable because it's broken beyond repair */
-#define DISABLE_ENQCMD (1 << (X86_FEATURE_ENQCMD & 31))
+#ifdef CONFIG_INTEL_IOMMU_SVM
+# define DISABLE_ENQCMD0
+#else
+# define DISABLE_ENQCMD(1 << (X86_FEATURE_ENQCMD & 31))
+#endif
 
 #ifdef CONFIG_X86_SGX
 # define DISABLE_SGX   0
-- 
2.35.0

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


[PATCH v3 04/11] kernel/fork: Initialize mm's PASID

2022-01-28 Thread Fenghua Yu
A new mm doesn't have a PASID yet when it's created. Initialize
the mm's PASID on fork() or for init_mm to INVALID_IOASID (-1).

Suggested-by: Dave Hansen 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Change condition to more accurate CONFIG_IOMMU_SVA (Jacob)

 include/linux/sched/mm.h | 10 ++
 kernel/fork.c| 10 ++
 mm/init-mm.c |  4 
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index aa5f09ca5bcf..c74d1edbac2f 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * Routines for handling mm_structs
@@ -433,4 +434,13 @@ static inline void membarrier_update_current_mm(struct 
mm_struct *next_mm)
 }
 #endif
 
+#ifdef CONFIG_IOMMU_SVA
+static inline void mm_pasid_init(struct mm_struct *mm)
+{
+   mm->pasid = INVALID_IOASID;
+}
+#else
+static inline void mm_pasid_init(struct mm_struct *mm) {}
+#endif
+
 #endif /* _LINUX_SCHED_MM_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index 6ee7551d3bd2..deacd2c17a7f 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -97,6 +97,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1019,13 +1020,6 @@ static void mm_init_owner(struct mm_struct *mm, struct 
task_struct *p)
 #endif
 }
 
-static void mm_init_pasid(struct mm_struct *mm)
-{
-#ifdef CONFIG_IOMMU_SVA
-   mm->pasid = INIT_PASID;
-#endif
-}
-
 static void mm_init_uprobes_state(struct mm_struct *mm)
 {
 #ifdef CONFIG_UPROBES
@@ -1054,7 +1048,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, 
struct task_struct *p,
mm_init_cpumask(mm);
mm_init_aio(mm);
mm_init_owner(mm, p);
-   mm_init_pasid(mm);
+   mm_pasid_init(mm);
RCU_INIT_POINTER(mm->exe_file, NULL);
mmu_notifier_subscriptions_init(mm);
init_tlb_flush_pending(mm);
diff --git a/mm/init-mm.c b/mm/init-mm.c
index b4a6f38fb51d..fbe7844d0912 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -10,6 +10,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef INIT_MM_CONTEXT
@@ -38,6 +39,9 @@ struct mm_struct init_mm = {
.mmlist = LIST_HEAD_INIT(init_mm.mmlist),
.user_ns= _user_ns,
.cpu_bitmap = CPU_BITS_NONE,
+#ifdef CONFIG_IOMMU_SVA
+   .pasid  = INVALID_IOASID,
+#endif
INIT_MM_CONTEXT(init_mm)
 };
 
-- 
2.35.0

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


[PATCH v3 06/11] x86/fpu: Clear PASID when copying fpstate

2022-01-28 Thread Fenghua Yu
The kernel must allocate a Process Address Space ID (PASID) on behalf of
each process which will use ENQCMD and program it into the new MSR to
communicate the process identity to platform hardware. ENQCMD uses the
PASID stored in this MSR to tag requests from this process.

The PASID state must be cleared on fork() since fork creates a
new address space.

For clone(), it would be functionally OK to copy the PASID. However,
clearing it is _also_ functionally OK since any PASID use will trigger
the #GP handler to populate the MSR.

Copying the PASID state has two main downsides:
 * It requires differentiating fork() and clone() in the code,
   both in the FPU code and keeping tsk->pasid_activated consistent.
 * It guarantees that the PASID is out of its init state, which
   incurs small but non-zero cost on every XSAVE/XRSTOR.

The main downside of clearing the PASID at fpstate copy is the future,
one-time #GP for the thread.

Use the simplest approach: clear the PASID state both on clone() and
fork().  Rely on the #GP handler for MSR population in children.

Also, just clear the PASID bit from xfeatures if XSAVE is supported.
This will have no effect on systems that do not have PASID support.  It
is virtually zero overhead because 'dst_fpu' was just written and
the whole thing is cache hot.

Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Rewrite changelog (Dave Hansen).
- Move xfeature tweaking into fpu_clone() and make it unconditional
  if XSAVE is supported (Dave Hansen).

 arch/x86/kernel/fpu/core.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 8dea01ffc5c1..19821f027cb3 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -612,6 +612,13 @@ int fpu_clone(struct task_struct *dst, unsigned long 
clone_flags)
fpu_inherit_perms(dst_fpu);
fpregs_unlock();
 
+   /*
+* Children never inherit PASID state.
+* Force it to have its init value:
+*/
+   if (use_xsave())
+   dst_fpu->fpstate->regs.xsave.header.xfeatures &= 
~XFEATURE_MASK_PASID;
+
trace_x86_fpu_copy_src(src_fpu);
trace_x86_fpu_copy_dst(dst_fpu);
 
-- 
2.35.0

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


[PATCH v3 00/11] Re-enable ENQCMD and PASID MSR

2022-01-28 Thread Fenghua Yu
Problems in the old code to manage SVM (Shared Virtual Memory) devices
and the PASID (Process Address Space ID) led to that code being
disabled.

Subsequent discussions resulted in a far simpler approach:

1) PASID life cycle is from first allocation by a process until that
   process exits.
2) All tasks begin with PASID disabled
3) The #GP fault handler tries to fix faulting ENQCMD instructions very
   early (thus avoiding complexities of the XSAVE infrastructure)

Change Log:
v3:
- Rename mm_pasid_get() to mm_pasid_set() in patch #5 (Thomas).
- Remove ioasid_get() because it's not used any more when the IOASID
  is freed on mm exit in patch #5 (Thomas).
- Remove PASID's refcount exercise in ioasid_put() and rename
  ioasid_put() to ioasid_free() in patch #5 and #11 (Thomas).
- Add Acked-by: Josh Poimboeuf  in patch #10.

v2 can be found at 
https://lore.kernel.org/lkml/20211217220136.2762116-1-fenghua...@intel.com/

v2:
- Free PASID on mm exit instead of in exit(2) or unbind() (Thomas, AndyL,
  PeterZ)
- Directly write IA32_PASID MSR in fixup while local IRQ is still disabled
  (Thomas)
- Simplify handling ENQCMD in objtool (PeterZ and Josh)
- Define mm_pasid_get(), mm_pasid_drop(), and mm_pasid_init() in mm and
  call the functions from IOMMU (Dave Hansen).
- A few changes in the #GP fixup function (Dave Hansen, Tony Luck).
- Initial PASID value is changed to INVALID_PASID (Ashok Raj and
  Jacob Pan).
- Add mm_pasid_init(), mm_pasid_get(), and mm_pasid_drop() functions in mm.
  So the mm's PASID operations are generic for both X86 and ARM
  (Dave Hansen).
- Rename CONFIG_IOMMU_SVA_LIB to more useful and accurate
  CONFIG_IOMMU_SVA
- Use CONFIG_IOMMU_SVA for PASID processing condition (Jacob)
- The patch that cleans up old update_pasid() function is in upstream
  now (commit: 00ecd5401349 "iommu/vt-d: Clean up unused PASID updating
  functions") and therefore it's removed from this version.

v1 can be found at 
https://lore.kernel.org/lkml/20210920192349.2602141-1-fenghua...@intel.com/T/#md6d542091da1d1159eda0a44a16e57d0c0dfb209

Fenghua Yu (10):
  iommu/sva: Rename CONFIG_IOMMU_SVA_LIB to CONFIG_IOMMU_SVA
  mm: Change CONFIG option for mm->pasid field
  iommu/ioasid: Introduce a helper to check for valid PASIDs
  kernel/fork: Initialize mm's PASID
  iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm
exit
  x86/fpu: Clear PASID when copying fpstate
  x86/traps: Demand-populate PASID MSR via #GP
  x86/cpufeatures: Re-enable ENQCMD
  tools/objtool: Check for use of the ENQCMD instruction in the kernel
  docs: x86: Change documentation for SVA (Shared Virtual Addressing)

Peter Zijlstra (1):
  sched: Define and initialize a flag to identify valid PASID in the
task

 Documentation/x86/sva.rst | 53 ++
 arch/x86/include/asm/disabled-features.h  |  7 ++-
 arch/x86/kernel/fpu/core.c|  7 +++
 arch/x86/kernel/traps.c   | 55 +++
 drivers/iommu/Kconfig |  6 +-
 drivers/iommu/Makefile|  2 +-
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  5 +-
 drivers/iommu/intel/Kconfig   |  2 +-
 drivers/iommu/intel/iommu.c   |  4 +-
 drivers/iommu/intel/svm.c |  9 ---
 drivers/iommu/ioasid.c| 38 ++---
 drivers/iommu/iommu-sva-lib.c | 39 -
 drivers/iommu/iommu-sva-lib.h |  7 +--
 include/linux/ioasid.h| 21 +++
 include/linux/mm_types.h  |  2 +-
 include/linux/sched.h |  3 +
 include/linux/sched/mm.h  | 26 +
 kernel/fork.c | 15 +++--
 mm/init-mm.c  |  4 ++
 tools/objtool/arch/x86/decode.c   | 11 +++-
 20 files changed, 197 insertions(+), 119 deletions(-)

-- 
2.35.0

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


[PATCH v3 02/11] mm: Change CONFIG option for mm->pasid field

2022-01-28 Thread Fenghua Yu
This currently depends on CONFIG_IOMMU_SUPPORT. But it is only
needed when CONFIG_IOMMU_SVA option is enabled.

Change the CONFIG guards around definition and initialization
of mm->pasid field.

Suggested-by: Jacob Pan 
Signed-off-by: Fenghua Yu 
Reviewed-by: Tony Luck 
---
v2:
- Change condition to more accurate CONFIG_IOMMU_SVA (Jacob and Tony)

 include/linux/mm_types.h | 2 +-
 kernel/fork.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 9db36dc5d4cf..c959116abd95 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -630,7 +630,7 @@ struct mm_struct {
 #endif
struct work_struct async_put_work;
 
-#ifdef CONFIG_IOMMU_SUPPORT
+#ifdef CONFIG_IOMMU_SVA
u32 pasid;
 #endif
} __randomize_layout;
diff --git a/kernel/fork.c b/kernel/fork.c
index d75a528f7b21..6ee7551d3bd2 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1021,7 +1021,7 @@ static void mm_init_owner(struct mm_struct *mm, struct 
task_struct *p)
 
 static void mm_init_pasid(struct mm_struct *mm)
 {
-#ifdef CONFIG_IOMMU_SUPPORT
+#ifdef CONFIG_IOMMU_SVA
mm->pasid = INIT_PASID;
 #endif
 }
-- 
2.35.0

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


RE: [PATCH 2/2] x86/hyperv: Set swiotlb_alloc_from_low_pages to false

2022-01-28 Thread Michael Kelley (LINUX) via iommu
From: Tianyu Lan  Sent: Wednesday, January 26, 2022 8:11 AM
> 
> In Hyper-V Isolation VM, swiotlb bnounce buffer size maybe 1G at most
> and there maybe no enough memory from 0 to 4G according to memory layout.
> Devices in Isolation VM can use memory above 4G as DMA memory. Set swiotlb_
> alloc_from_low_pages to false in Isolation VM.
> 
> Signed-off-by: Tianyu Lan 
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 5a99f993e639..80a0423ac75d 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -343,6 +343,7 @@ static void __init ms_hyperv_init_platform(void)
>* use swiotlb bounce buffer for dma transaction.
>*/
>   swiotlb_force = SWIOTLB_FORCE;
> + swiotlb_alloc_from_low_pages = false;
>  #endif
>   }
> 
> --
> 2.25.1

Reviewed-by: Michael Kelley 

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


RE: [PATCH 1/2] Swiotlb: Add swiotlb_alloc_from_low_pages switch

2022-01-28 Thread Michael Kelley (LINUX) via iommu
From: Tianyu Lan  Sent: Wednesday, January 26, 2022 8:11 AM
> 
> Hyper-V Isolation VM and AMD SEV VM uses swiotlb bounce buffer to
> share memory with hypervisor. Current swiotlb bounce buffer is only
> allocated from 0 to ARCH_LOW_ADDRESS_LIMIT which is default to
> 0xUL. Isolation VM and AMD SEV VM needs 1G bounce buffer at most.
> This will fail when there is not enough contiguous memory from 0 to 4G
> address space and devices also may use memory above 4G address space as
> DMA memory. Expose swiotlb_alloc_from_low_pages and platform mey set it
> to false when it's not necessary to limit bounce buffer from 0 to 4G memory.
> 
> Signed-off-by: Tianyu Lan 
> ---
>  include/linux/swiotlb.h |  1 +
>  kernel/dma/swiotlb.c| 13 +++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index f6c3638255d5..55c178e8eee0 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -191,5 +191,6 @@ static inline bool is_swiotlb_for_alloc(struct device 
> *dev)
>  #endif /* CONFIG_DMA_RESTRICTED_POOL */
> 
>  extern phys_addr_t swiotlb_unencrypted_base;
> +extern bool swiotlb_alloc_from_low_pages;
> 
>  #endif /* __LINUX_SWIOTLB_H */
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index f1e7ea160b43..159fef80f3db 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -73,6 +73,12 @@ enum swiotlb_force swiotlb_force;
> 
>  struct io_tlb_mem io_tlb_default_mem;
> 
> +/*
> + * Get IO TLB memory from the low pages if swiotlb_alloc_from_low_pages
> + * is set.
> + */
> +bool swiotlb_alloc_from_low_pages = true;
> +
>  phys_addr_t swiotlb_unencrypted_base;
> 
>  /*
> @@ -284,8 +290,11 @@ swiotlb_init(int verbose)
>   if (swiotlb_force == SWIOTLB_NO_FORCE)
>   return;
> 
> - /* Get IO TLB memory from the low pages */
> - tlb = memblock_alloc_low(bytes, PAGE_SIZE);
> + if (swiotlb_alloc_from_low_pages)
> + tlb = memblock_alloc_low(bytes, PAGE_SIZE);
> + else
> + tlb = memblock_alloc(bytes, PAGE_SIZE);
> +
>   if (!tlb)
>   goto fail;
>   if (swiotlb_init_with_tbl(tlb, default_nslabs, verbose))
> --
> 2.25.1

Reviewed-by: Michael Kelley 

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


Re: [PATCH v5 02/24] mm: remove extra ZONE_DEVICE struct page refcount

2022-01-28 Thread Ralph Campbell via iommu



On 1/28/22 06:21, Jason Gunthorpe wrote:

On Thu, Jan 27, 2022 at 05:25:52PM -0700, Logan Gunthorpe wrote:

From: Ralph Campbell 

ZONE_DEVICE struct pages have an extra reference count that complicates the
code for put_page() and several places in the kernel that need to check the
reference count to see that a page is not being used (gup, compaction,
migration, etc.). Clean up the code so the reference count doesn't need to
be treated specially for ZONE_DEVICE.

[logang: dropped no longer used section from mm.h including
  page_is_devmap_managed, rebased on v5.17-rc1 (possibly poorly)]
Signed-off-by: Ralph Campbell 
Signed-off-by: Alex Sierra 
Signed-off-by: Logan Gunthorpe 
Reviewed-by: Christoph Hellwig 
---
  arch/powerpc/kvm/book3s_hv_uvmem.c |  2 +-
  drivers/gpu/drm/nouveau/nouveau_dmem.c |  2 +-
  fs/dax.c   |  4 +-
  include/linux/dax.h|  2 +-
  include/linux/memremap.h   |  7 +--
  include/linux/mm.h | 44 
  lib/test_hmm.c |  2 +-
  mm/internal.h  |  8 +++
  mm/memcontrol.c|  6 +--
  mm/memremap.c  | 70 +++---
  mm/migrate.c   |  5 --
  mm/page_alloc.c|  3 ++
  mm/swap.c  | 45 ++---
  13 files changed, 46 insertions(+), 154 deletions(-)

This patch still can't be applied until the FSDAX issues are solved,
right? See my remarks the last time it was posted..

Jason


That is my understanding too.

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


Re: [PATCH] iommu/iova: Separate out rcache init

2022-01-28 Thread Robin Murphy

On 2022-01-28 11:32, John Garry wrote:

On 26/01/2022 17:00, Robin Murphy wrote:

As above, I vote for just forward-declaring the free routine in iova.c
and keeping it entirely private.


BTW, speaking of forward declarations, it's possible to remove all the 
forward declarations in iova.c now that the FQ code is gone - but with a 
good bit of rearranging. However I am not sure how much people care 
about that or whether the code layout is sane...


Indeed, I was very tempted to raise the question there of whether there 
was any more cleanup or refactoring that could be done to justify 
collecting all the rcache code together at the top of iova.c. But in the 
end I didn't, so my opinion still remains a secret...


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


Re: [PATCH v5 22/24] mm: use custom page_free for P2PDMA pages

2022-01-28 Thread Logan Gunthorpe



On 2022-01-28 7:22 a.m., Jason Gunthorpe wrote:
> On Thu, Jan 27, 2022 at 05:26:12PM -0700, Logan Gunthorpe wrote:
>> When P2PDMA pages are passed to userspace, they will need to be
>> reference counted properly and returned to their genalloc after their
>> reference count returns to 1. This is accomplished with the existing
> 
> It is reference count returns to 0 now, right?

Right, yes.

Thanks,

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


Re: [PATCH v5 02/24] mm: remove extra ZONE_DEVICE struct page refcount

2022-01-28 Thread Logan Gunthorpe



On 2022-01-28 7:21 a.m., Jason Gunthorpe wrote:
> On Thu, Jan 27, 2022 at 05:25:52PM -0700, Logan Gunthorpe wrote:
>> From: Ralph Campbell 
>>
>> ZONE_DEVICE struct pages have an extra reference count that complicates the
>> code for put_page() and several places in the kernel that need to check the
>> reference count to see that a page is not being used (gup, compaction,
>> migration, etc.). Clean up the code so the reference count doesn't need to
>> be treated specially for ZONE_DEVICE.
>>
>> [logang: dropped no longer used section from mm.h including
>>  page_is_devmap_managed, rebased on v5.17-rc1 (possibly poorly)]
>> Signed-off-by: Ralph Campbell 
>> Signed-off-by: Alex Sierra 
>> Signed-off-by: Logan Gunthorpe 
>> Reviewed-by: Christoph Hellwig 
>> ---
>>  arch/powerpc/kvm/book3s_hv_uvmem.c |  2 +-
>>  drivers/gpu/drm/nouveau/nouveau_dmem.c |  2 +-
>>  fs/dax.c   |  4 +-
>>  include/linux/dax.h|  2 +-
>>  include/linux/memremap.h   |  7 +--
>>  include/linux/mm.h | 44 
>>  lib/test_hmm.c |  2 +-
>>  mm/internal.h  |  8 +++
>>  mm/memcontrol.c|  6 +--
>>  mm/memremap.c  | 70 +++---
>>  mm/migrate.c   |  5 --
>>  mm/page_alloc.c|  3 ++
>>  mm/swap.c  | 45 ++---
>>  13 files changed, 46 insertions(+), 154 deletions(-)
> 
> This patch still can't be applied until the FSDAX issues are solved,
> right? See my remarks the last time it was posted..

Yes. As I mentioned in the cover, this is just to show that this
patchset is compatible with the direction this patch goes.

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


Re: [PATCH] iommu: Fix some W=1 warnings

2022-01-28 Thread Robin Murphy

On 2022-01-28 10:44, John Garry wrote:

The code is mostly free of W=1 warning, so fix the following:

drivers/iommu/iommu.c:996: warning: expecting prototype for 
iommu_group_for_each_dev(). Prototype was for __iommu_group_for_each_dev() 
instead
drivers/iommu/iommu.c:3048: warning: Function parameter or member 'drvdata' not 
described in 'iommu_sva_bind_device'
drivers/iommu/ioasid.c:354: warning: Function parameter or member 'ioasid' not 
described in 'ioasid_get'
drivers/iommu/omap-iommu.c:1098: warning: expecting prototype for 
omap_iommu_suspend_prepare(). Prototype was for omap_iommu_prepare() instead


Certainly no harm in keeping the documentation up to date!

Reviewed-by: Robin Murphy 


Signed-off-by: John Garry 

diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
index 50ee27bbd04e..06fee7416816 100644
--- a/drivers/iommu/ioasid.c
+++ b/drivers/iommu/ioasid.c
@@ -349,6 +349,7 @@ EXPORT_SYMBOL_GPL(ioasid_alloc);
  
  /**

   * ioasid_get - obtain a reference to the IOASID
+ * @ioasid: the ID to get
   */
  void ioasid_get(ioasid_t ioasid)
  {
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b86406b7162..75741ce748d5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -980,17 +980,6 @@ static int iommu_group_device_count(struct iommu_group 
*group)
return ret;
  }
  
-/**

- * iommu_group_for_each_dev - iterate over each device in the group
- * @group: the group
- * @data: caller opaque data to be passed to callback function
- * @fn: caller supplied callback function
- *
- * This function is called by group users to iterate over group devices.
- * Callers should hold a reference count to the group during callback.
- * The group->mutex is held across callbacks, which will block calls to
- * iommu_group_add/remove_device.
- */
  static int __iommu_group_for_each_dev(struct iommu_group *group, void *data,
  int (*fn)(struct device *, void *))
  {
@@ -1005,7 +994,17 @@ static int __iommu_group_for_each_dev(struct iommu_group 
*group, void *data,
return ret;
  }
  
-

+/**
+ * iommu_group_for_each_dev - iterate over each device in the group
+ * @group: the group
+ * @data: caller opaque data to be passed to callback function
+ * @fn: caller supplied callback function
+ *
+ * This function is called by group users to iterate over group devices.
+ * Callers should hold a reference count to the group during callback.
+ * The group->mutex is held across callbacks, which will block calls to
+ * iommu_group_add/remove_device.
+ */
  int iommu_group_for_each_dev(struct iommu_group *group, void *data,
 int (*fn)(struct device *, void *))
  {
@@ -3032,6 +3031,7 @@ EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
   * iommu_sva_bind_device() - Bind a process address space to a device
   * @dev: the device
   * @mm: the mm to bind, caller must hold a reference to it
+ * @drvdata: opaque data pointer to pass to bind callback
   *
   * Create a bond between device and address space, allowing the device to 
access
   * the mm using the returned PASID. If a bond already exists between @device 
and
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 91749654fd49..980e4af3f06b 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1085,7 +1085,7 @@ static __maybe_unused int 
omap_iommu_runtime_resume(struct device *dev)
  }
  
  /**

- * omap_iommu_suspend_prepare - prepare() dev_pm_ops implementation
+ * omap_iommu_prepare - prepare() dev_pm_ops implementation
   * @dev:  iommu device
   *
   * This function performs the necessary checks to determine if the IOMMU

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


Re: [PATCH v2 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit

2022-01-28 Thread Thomas Gleixner
On Thu, Jan 27 2022 at 18:42, Fenghua Yu wrote:
> On Wed, Jan 26, 2022 at 10:38:04PM +0100, Thomas Gleixner wrote:
>> Against Linus tree please so that the bugfix applies.
>> 
>> > I will fold the following patch into patch #5. The patch #11 (the doc 
>> > patch)
>> > also needs to remove one paragraph talking about refcount.
>> >
>> > So I will send the whole patch set with the following changes:
>> > 1. One new bug fix patch (the above patch)
>
> When I study your above aux_domain bug fix path, I find more aux_domain bugs.
> But then I find aux_domain will be completely removed because all aux_domain
> related callbacks are not called and are dead code (no wonder there are
> so many bugs in aux_domain). Please see this series: 
> https://lore.kernel.org/linux-iommu/20220124071103.2097118-4-baolu...@linux.intel.com/
> For the series, Baolu confirms that he is "pretty sure that should be part
> of v5.18". And I don't find the series calls any IOASID function after
> removing the aux_domain code.
>
> So that means we don't need to fix those issues in the dead aux_domain
> code any more because it will be completely removed in 5.18, right?
>
> If you agree, I will not include the aux_domain fix patch or any other
> aux_domain fix patches in the up-coming v3. Is that OK?

Fair enough.

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


Re: [PATCH v5 22/24] mm: use custom page_free for P2PDMA pages

2022-01-28 Thread Jason Gunthorpe
On Thu, Jan 27, 2022 at 05:26:12PM -0700, Logan Gunthorpe wrote:
> When P2PDMA pages are passed to userspace, they will need to be
> reference counted properly and returned to their genalloc after their
> reference count returns to 1. This is accomplished with the existing

It is reference count returns to 0 now, right?

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


Re: [PATCH v5 02/24] mm: remove extra ZONE_DEVICE struct page refcount

2022-01-28 Thread Jason Gunthorpe
On Thu, Jan 27, 2022 at 05:25:52PM -0700, Logan Gunthorpe wrote:
> From: Ralph Campbell 
> 
> ZONE_DEVICE struct pages have an extra reference count that complicates the
> code for put_page() and several places in the kernel that need to check the
> reference count to see that a page is not being used (gup, compaction,
> migration, etc.). Clean up the code so the reference count doesn't need to
> be treated specially for ZONE_DEVICE.
> 
> [logang: dropped no longer used section from mm.h including
>  page_is_devmap_managed, rebased on v5.17-rc1 (possibly poorly)]
> Signed-off-by: Ralph Campbell 
> Signed-off-by: Alex Sierra 
> Signed-off-by: Logan Gunthorpe 
> Reviewed-by: Christoph Hellwig 
> ---
>  arch/powerpc/kvm/book3s_hv_uvmem.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_dmem.c |  2 +-
>  fs/dax.c   |  4 +-
>  include/linux/dax.h|  2 +-
>  include/linux/memremap.h   |  7 +--
>  include/linux/mm.h | 44 
>  lib/test_hmm.c |  2 +-
>  mm/internal.h  |  8 +++
>  mm/memcontrol.c|  6 +--
>  mm/memremap.c  | 70 +++---
>  mm/migrate.c   |  5 --
>  mm/page_alloc.c|  3 ++
>  mm/swap.c  | 45 ++---
>  13 files changed, 46 insertions(+), 154 deletions(-)

This patch still can't be applied until the FSDAX issues are solved,
right? See my remarks the last time it was posted..

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


Re: [RFC PATCH] component: Add common helpers for compare/release functions

2022-01-28 Thread Robin Murphy

On 2022-01-28 08:11, Yong Wu wrote:
[...]

diff --git a/include/linux/component.h b/include/linux/component.h
index 16de18f473d7..5a7468ea827c 100644
--- a/include/linux/component.h
+++ b/include/linux/component.h
@@ -2,6 +2,8 @@
  #ifndef COMPONENT_H
  #define COMPONENT_H
  
+#include 

+#include 
  #include 
  
  
@@ -82,6 +84,22 @@ struct component_master_ops {

void (*unbind)(struct device *master);
  };
  
+/* A set common helpers for compare/release functions */

+static inline int compare_of(struct device *dev, void *data)
+{
+   return dev->of_node == data;
+}


Note that this is effectively just device_match_of_node(), although I 
guess there is an argument that having a nice consistent set of 
component_match API helpers might be worth more than a tiny code saving 
by borrowing one from a different API.


Either way, however, I don't think there's any good argument for 
instantiating separate copies of these functions in every driver that 
uses them. If they're used as callbacks then they can't actually be 
inlined anyway, so they may as well be exported from component.c as 
normal so that the code really is shared (plus then there's nice 
symmetry with the aforementioned device_match API helpers too).


Thanks,
Robin.


+static inline void release_of(struct device *dev, void *data)
+{
+   of_node_put(data);
+}
+
+static inline int compare_dev(struct device *dev, void *data)
+{
+   return dev == data;
+}
+
  void component_master_del(struct device *,
const struct component_master_ops *);
  
diff --git a/sound/soc/codecs/wcd938x.c b/sound/soc/codecs/wcd938x.c

index eff200a07d9f..992132cbfb9f 100644
--- a/sound/soc/codecs/wcd938x.c
+++ b/sound/soc/codecs/wcd938x.c
@@ -4417,16 +4417,6 @@ static const struct component_master_ops 
wcd938x_comp_ops = {
.unbind = wcd938x_unbind,
  };
  
-static int wcd938x_compare_of(struct device *dev, void *data)

-{
-   return dev->of_node == data;
-}
-
-static void wcd938x_release_of(struct device *dev, void *data)
-{
-   of_node_put(data);
-}
-
  static int wcd938x_add_slave_components(struct wcd938x_priv *wcd938x,
struct device *dev,
struct component_match **matchptr)
@@ -4442,8 +4432,7 @@ static int wcd938x_add_slave_components(struct 
wcd938x_priv *wcd938x,
}
  
  	of_node_get(wcd938x->rxnode);

-   component_match_add_release(dev, matchptr, wcd938x_release_of,
-   wcd938x_compare_of, wcd938x->rxnode);
+   component_match_add_release(dev, matchptr, release_of, compare_of, 
wcd938x->rxnode);
  
  	wcd938x->txnode = of_parse_phandle(np, "qcom,tx-device", 0);

if (!wcd938x->txnode) {
@@ -4451,8 +4440,7 @@ static int wcd938x_add_slave_components(struct 
wcd938x_priv *wcd938x,
return -ENODEV;
}
of_node_get(wcd938x->txnode);
-   component_match_add_release(dev, matchptr, wcd938x_release_of,
-   wcd938x_compare_of, wcd938x->txnode);
+   component_match_add_release(dev, matchptr, release_of, compare_of, 
wcd938x->txnode);
return 0;
  }
  

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


Re: [PATCH v10 02/13] iommu/mediatek-v1: Free the existed fwspec if the master dev already has

2022-01-28 Thread Mauro Carvalho Chehab
Hi Matthias/Yong,

Are you ok if this patch gets merged via the media tree together with the
remaining series, or do you prefer to apply it via SoC tree instead?

Regards,
Mauro


Em Mon, 17 Jan 2022 15:04:59 +0800
Yong Wu  escreveu:

> When the iommu master device enters of_iommu_xlate, the ops may be
> NULL(iommu dev is defered), then it will initialize the fwspec here:
> 
> [] (dev_iommu_fwspec_set) from []
> (iommu_fwspec_init+0xbc/0xd4)
> [] (iommu_fwspec_init) from []
> (of_iommu_xlate+0x7c/0x12c)
> [] (of_iommu_xlate) from []
> (of_iommu_configure+0x144/0x1e8)
> 
> BUT the mtk_iommu_v1.c only supports arm32, the probing flow still is a bit
> weird. We always expect create the fwspec internally. otherwise it will
> enter here and return fail.
> 
> static int mtk_iommu_create_mapping(struct device *dev,
>   struct of_phandle_args *args)
> {
> ...
>   if (!fwspec) {
>   
>   } else if (dev_iommu_fwspec_get(dev)->ops != _iommu_ops) {
> >>Enter here. return fail.  
>   return -EINVAL;
>   }
>   ...
> }
> 
> Thus, Free the existed fwspec if the master device already has fwspec.
> 
> This issue is reported at:
> https://lore.kernel.org/linux-mediatek/trinity-7d9ebdc9-4849-4d93-bfb5-429dcb4ee449-1626253158870@3c-app-gmx-bs01/
> 
> Reported-by: Frank Wunderlich 
> Tested-by: Frank Wunderlich  # BPI-R2/MT7623
> Signed-off-by: Yong Wu 
> Acked-by: Joerg Roedel 
> Acked-by: AngeloGioacchino Del Regno 
> ---
>  drivers/iommu/mtk_iommu_v1.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> index be22fcf988ce..1467ba1e4417 100644
> --- a/drivers/iommu/mtk_iommu_v1.c
> +++ b/drivers/iommu/mtk_iommu_v1.c
> @@ -425,6 +425,15 @@ static struct iommu_device 
> *mtk_iommu_probe_device(struct device *dev)
>   struct mtk_iommu_data *data;
>   int err, idx = 0;
>  
> + /*
> +  * In the deferred case, free the existed fwspec.
> +  * Always initialize the fwspec internally.
> +  */
> + if (fwspec) {
> + iommu_fwspec_free(dev);
> + fwspec = dev_iommu_fwspec_get(dev);
> + }
> +
>   while (!of_parse_phandle_with_args(dev->of_node, "iommus",
>  "#iommu-cells",
>  idx, _spec)) {



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


Re: [PATCH v10 02/13] iommu/mediatek-v1: Free the existed fwspec if the master dev already has

2022-01-28 Thread Mauro Carvalho Chehab
Em Fri, 28 Jan 2022 13:40:55 +0100
Mauro Carvalho Chehab  escreveu:

> Hi Matthias/Yong,
> 
> Are you ok if this patch gets merged via the media tree together with the
> remaining series, or do you prefer to apply it via SoC tree instead?

Same questions for other patches touching files outside drivers/media
on this pull request:


https://patchwork.kernel.org/project/linux-mediatek/patch/7af52d61-47c7-581d-62ed-76a7f8315...@xs4all.nl/

Like those:
0004-0013-iommu-mediatek-v1-Free-the-existed-fwspec-if-the-mas.patch
0005-0013-iommu-mediatek-Return-ENODEV-if-the-device-is-NULL.patch
0006-0013-iommu-mediatek-Add-probe_defer-for-smi-larb.patch
0007-0013-iommu-mediatek-Add-device_link-between-the-consumer-.patch

Regards,
Mauro

> 
> Regards,
> Mauro
> 
> 
> Em Mon, 17 Jan 2022 15:04:59 +0800
> Yong Wu  escreveu:
> 
> > When the iommu master device enters of_iommu_xlate, the ops may be
> > NULL(iommu dev is defered), then it will initialize the fwspec here:
> > 
> > [] (dev_iommu_fwspec_set) from []
> > (iommu_fwspec_init+0xbc/0xd4)
> > [] (iommu_fwspec_init) from []
> > (of_iommu_xlate+0x7c/0x12c)
> > [] (of_iommu_xlate) from []
> > (of_iommu_configure+0x144/0x1e8)
> > 
> > BUT the mtk_iommu_v1.c only supports arm32, the probing flow still is a bit
> > weird. We always expect create the fwspec internally. otherwise it will
> > enter here and return fail.
> > 
> > static int mtk_iommu_create_mapping(struct device *dev,
> > struct of_phandle_args *args)
> > {
> > ...
> > if (!fwspec) {
> > 
> > } else if (dev_iommu_fwspec_get(dev)->ops != _iommu_ops) {  
> > >>Enter here. return fail.
> > return -EINVAL;
> > }
> > ...
> > }
> > 
> > Thus, Free the existed fwspec if the master device already has fwspec.
> > 
> > This issue is reported at:
> > https://lore.kernel.org/linux-mediatek/trinity-7d9ebdc9-4849-4d93-bfb5-429dcb4ee449-1626253158870@3c-app-gmx-bs01/
> > 
> > Reported-by: Frank Wunderlich 
> > Tested-by: Frank Wunderlich  # BPI-R2/MT7623
> > Signed-off-by: Yong Wu 
> > Acked-by: Joerg Roedel 
> > Acked-by: AngeloGioacchino Del Regno 
> > 
> > ---
> >  drivers/iommu/mtk_iommu_v1.c | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c
> > index be22fcf988ce..1467ba1e4417 100644
> > --- a/drivers/iommu/mtk_iommu_v1.c
> > +++ b/drivers/iommu/mtk_iommu_v1.c
> > @@ -425,6 +425,15 @@ static struct iommu_device 
> > *mtk_iommu_probe_device(struct device *dev)
> > struct mtk_iommu_data *data;
> > int err, idx = 0;
> >  
> > +   /*
> > +* In the deferred case, free the existed fwspec.
> > +* Always initialize the fwspec internally.
> > +*/
> > +   if (fwspec) {
> > +   iommu_fwspec_free(dev);
> > +   fwspec = dev_iommu_fwspec_get(dev);
> > +   }
> > +
> > while (!of_parse_phandle_with_args(dev->of_node, "iommus",
> >"#iommu-cells",
> >idx, _spec)) {  
> 
> 
> 
> Thanks,
> Mauro



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


Re: [PATCH] iommu/iova: Separate out rcache init

2022-01-28 Thread John Garry via iommu

On 26/01/2022 17:00, Robin Murphy wrote:

As above, I vote for just forward-declaring the free routine in iova.c
and keeping it entirely private.


BTW, speaking of forward declarations, it's possible to remove all the 
forward declarations in iova.c now that the FQ code is gone - but with a 
good bit of rearranging. However I am not sure how much people care 
about that or whether the code layout is sane...

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


[PATCH] iommu: Fix some W=1 warnings

2022-01-28 Thread John Garry via iommu
The code is mostly free of W=1 warning, so fix the following:

drivers/iommu/iommu.c:996: warning: expecting prototype for 
iommu_group_for_each_dev(). Prototype was for __iommu_group_for_each_dev() 
instead
drivers/iommu/iommu.c:3048: warning: Function parameter or member 'drvdata' not 
described in 'iommu_sva_bind_device'
drivers/iommu/ioasid.c:354: warning: Function parameter or member 'ioasid' not 
described in 'ioasid_get'
drivers/iommu/omap-iommu.c:1098: warning: expecting prototype for 
omap_iommu_suspend_prepare(). Prototype was for omap_iommu_prepare() instead

Signed-off-by: John Garry 

diff --git a/drivers/iommu/ioasid.c b/drivers/iommu/ioasid.c
index 50ee27bbd04e..06fee7416816 100644
--- a/drivers/iommu/ioasid.c
+++ b/drivers/iommu/ioasid.c
@@ -349,6 +349,7 @@ EXPORT_SYMBOL_GPL(ioasid_alloc);
 
 /**
  * ioasid_get - obtain a reference to the IOASID
+ * @ioasid: the ID to get
  */
 void ioasid_get(ioasid_t ioasid)
 {
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 8b86406b7162..75741ce748d5 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -980,17 +980,6 @@ static int iommu_group_device_count(struct iommu_group 
*group)
return ret;
 }
 
-/**
- * iommu_group_for_each_dev - iterate over each device in the group
- * @group: the group
- * @data: caller opaque data to be passed to callback function
- * @fn: caller supplied callback function
- *
- * This function is called by group users to iterate over group devices.
- * Callers should hold a reference count to the group during callback.
- * The group->mutex is held across callbacks, which will block calls to
- * iommu_group_add/remove_device.
- */
 static int __iommu_group_for_each_dev(struct iommu_group *group, void *data,
  int (*fn)(struct device *, void *))
 {
@@ -1005,7 +994,17 @@ static int __iommu_group_for_each_dev(struct iommu_group 
*group, void *data,
return ret;
 }
 
-
+/**
+ * iommu_group_for_each_dev - iterate over each device in the group
+ * @group: the group
+ * @data: caller opaque data to be passed to callback function
+ * @fn: caller supplied callback function
+ *
+ * This function is called by group users to iterate over group devices.
+ * Callers should hold a reference count to the group during callback.
+ * The group->mutex is held across callbacks, which will block calls to
+ * iommu_group_add/remove_device.
+ */
 int iommu_group_for_each_dev(struct iommu_group *group, void *data,
 int (*fn)(struct device *, void *))
 {
@@ -3032,6 +3031,7 @@ EXPORT_SYMBOL_GPL(iommu_aux_get_pasid);
  * iommu_sva_bind_device() - Bind a process address space to a device
  * @dev: the device
  * @mm: the mm to bind, caller must hold a reference to it
+ * @drvdata: opaque data pointer to pass to bind callback
  *
  * Create a bond between device and address space, allowing the device to 
access
  * the mm using the returned PASID. If a bond already exists between @device 
and
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 91749654fd49..980e4af3f06b 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1085,7 +1085,7 @@ static __maybe_unused int 
omap_iommu_runtime_resume(struct device *dev)
 }
 
 /**
- * omap_iommu_suspend_prepare - prepare() dev_pm_ops implementation
+ * omap_iommu_prepare - prepare() dev_pm_ops implementation
  * @dev:   iommu device
  *
  * This function performs the necessary checks to determine if the IOMMU
-- 
2.26.2

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


Re: [PATCH 2/2] iommu/mediatek: Add mt8186 iommu support

2022-01-28 Thread Yong Wu
On Thu, 2022-01-27 at 12:28 +0100, AngeloGioacchino Del Regno wrote:
> Il 25/01/22 10:32, Yong Wu ha scritto:
> > Add mt8186 iommu supports.
> > 
> > Signed-off-by: Anan Sun 
> > Signed-off-by: Yong Wu 
> > ---
> >   drivers/iommu/mtk_iommu.c | 17 +
> >   1 file changed, 17 insertions(+)

[snip]

> >   static const struct mtk_iommu_plat_data mt8192_data = {
> > .m4u_plat   = M4U_MT8192,
> > .flags  = HAS_BCLK | HAS_SUB_COMM_2BITS |
> > OUT_ORDER_WR_EN |
> > @@ -1470,6 +1486,7 @@ static const struct of_device_id
> > mtk_iommu_of_ids[] = {
> > { .compatible = "mediatek,mt8167-m4u", .data = _data},
> > { .compatible = "mediatek,mt8173-m4u", .data = _data},
> > { .compatible = "mediatek,mt8183-m4u", .data = _data},
> > +   { .compatible = "mediatek,mt8186-iommu-mm", .data =
> > _data_mm},
> 
> Hello!
> 
> Is there any particular reason why this compatible is not
> "mediatek,mt8186-m4u"?

There is no special reason. In the previous SoC, We only support MM
IOMMU, it was called by "m4u". In the lastest SoC, We have the other
types IOMMU, like for INFRA masters and APU, thus they are called "mm
iommu", "infra iommu" and "apu iommu". Of course, "m4u" means "mm
iommu".

> 
> Thanks,
> Angelo
> 
> > { .compatible = "mediatek,mt8192-m4u", .data = _data},
> > { .compatible = "mediatek,mt8195-iommu-infra", .data =
> > _data_infra},
> > { .compatible = "mediatek,mt8195-iommu-vdo",   .data =
> > _data_vdo},
> 
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek

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


Re: [PATCH v5 01/24] ext4/xfs: add page refcount helper

2022-01-28 Thread Chaitanya Kulkarni via iommu
On 1/27/22 4:25 PM, Logan Gunthorpe wrote:
> External email: Use caution opening links or attachments
> 
> 
> From: Ralph Campbell 
> 
> There are several places where ZONE_DEVICE struct pages assume a reference
> count == 1 means the page is idle and free. Instead of open coding this,
> add a helper function to hide this detail.
> 
> Signed-off-by: Ralph Campbell 
> Signed-off-by: Alex Sierra 
> Reviewed-by: Christoph Hellwig 
> Acked-by: Theodore Ts'o 
> Acked-by: Darrick J. Wong 
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni 

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


Re: [PATCH v5 12/24] nvme-pci: convert to using dma_map_sgtable()

2022-01-28 Thread Chaitanya Kulkarni via iommu
On 1/27/22 4:26 PM, Logan Gunthorpe wrote:
> External email: Use caution opening links or attachments
> 
> 
> The dma_map operations now support P2PDMA pages directly. So remove
> the calls to pci_p2pdma_[un]map_sg_attrs() and replace them with calls
> to dma_map_sgtable().
> 
> dma_map_sgtable() returns more complete error codes than dma_map_sg()
> and allows differentiating EREMOTEIO errors in case an unsupported
> P2PDMA transfer is requested. When this happens, return BLK_STS_TARGET
> so the request isn't retried.
> 
> Signed-off-by: Logan Gunthorpe 
> Reviewed-by: Max Gurtovoy 
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni 

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


[RFC PATCH] component: Add common helpers for compare/release functions

2022-01-28 Thread Yong Wu
The component requires the compare/release functions, there are so many
copy in current kernel. Just define three common helpers for them.
No functional change.

Signed-off-by: Yong Wu 
---
Base on v5.17-rc1
---
 .../gpu/drm/arm/display/komeda/komeda_drv.c|  5 -
 drivers/gpu/drm/arm/hdlcd_drv.c|  7 +--
 drivers/gpu/drm/armada/armada_drv.c|  5 -
 drivers/gpu/drm/drm_of.c   |  8 +---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  7 ---
 drivers/gpu/drm/exynos/exynos_drm_drv.c|  5 -
 .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c|  5 -
 drivers/gpu/drm/imx/imx-drm-core.c |  4 ++--
 drivers/gpu/drm/ingenic/ingenic-drm-drv.c  |  5 -
 drivers/gpu/drm/mcde/mcde_drv.c|  7 +--
 drivers/gpu/drm/mediatek/mtk_drm_drv.c |  5 -
 drivers/gpu/drm/meson/meson_drv.c  |  8 
 drivers/gpu/drm/msm/msm_drv.c  |  9 -
 drivers/gpu/drm/omapdrm/dss/dss.c  |  8 +---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c|  5 -
 drivers/gpu/drm/sti/sti_drv.c  |  5 -
 drivers/gpu/drm/sun4i/sun4i_drv.c  |  9 -
 drivers/gpu/drm/vc4/vc4_drv.c  |  5 -
 drivers/iommu/mtk_iommu.h  | 10 --
 drivers/power/supply/ab8500_charger.c  |  8 +---
 drivers/video/fbdev/omap2/omapfb/dss/dss.c |  8 +---
 include/linux/component.h  | 18 ++
 sound/soc/codecs/wcd938x.c | 16 ++--
 23 files changed, 28 insertions(+), 144 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index e7933930a657..fe5b97107417 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -92,11 +92,6 @@ static const struct component_master_ops komeda_master_ops = 
{
.unbind = komeda_unbind,
 };
 
-static int compare_of(struct device *dev, void *data)
-{
-   return dev->of_node == data;
-}
-
 static void komeda_add_slave(struct device *master,
 struct component_match **match,
 struct device_node *np,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 479c2422a2e0..36d84c439df8 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -372,11 +372,6 @@ static const struct component_master_ops hdlcd_master_ops 
= {
.unbind = hdlcd_drm_unbind,
 };
 
-static int compare_dev(struct device *dev, void *data)
-{
-   return dev->of_node == data;
-}
-
 static int hdlcd_probe(struct platform_device *pdev)
 {
struct device_node *port;
@@ -387,7 +382,7 @@ static int hdlcd_probe(struct platform_device *pdev)
if (!port)
return -ENODEV;
 
-   drm_of_component_match_add(>dev, , compare_dev, port);
+   drm_of_component_match_add(>dev, , compare_of, port);
of_node_put(port);
 
return component_master_add_with_match(>dev, _master_ops,
diff --git a/drivers/gpu/drm/armada/armada_drv.c 
b/drivers/gpu/drm/armada/armada_drv.c
index 8e3e98f13db4..9edc4912c1a0 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -177,11 +177,6 @@ static void armada_drm_unbind(struct device *dev)
drm_mm_takedown(>linear);
 }
 
-static int compare_of(struct device *dev, void *data)
-{
-   return dev->of_node == data;
-}
-
 static int compare_dev_name(struct device *dev, void *data)
 {
const char *name = data;
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 59d368ea006b..f958f48f8ba4 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -18,11 +18,6 @@
  * properties.
  */
 
-static void drm_release_of(struct device *dev, void *data)
-{
-   of_node_put(data);
-}
-
 /**
  * drm_of_crtc_port_mask - find the mask of a registered CRTC by port OF node
  * @dev: DRM device
@@ -94,8 +89,7 @@ void drm_of_component_match_add(struct device *master,
struct device_node *node)
 {
of_node_get(node);
-   component_match_add_release(master, matchptr, drm_release_of,
-   compare, node);
+   component_match_add_release(master, matchptr, release_of, compare, 
node);
 }
 EXPORT_SYMBOL_GPL(drm_of_component_match_add);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c 
b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 0b756ecb1bc2..15351e26ab00 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -574,13 +574,6 @@ static const struct component_master_ops 
etnaviv_master_ops = {
.unbind = etnaviv_unbind,
 };
 
-static int compare_of(struct device *dev, void *data)
-{
-   struct device_node *np = data;
-
-