Re: [bug report] iommu/amd: Improve error handling for amd_iommu_init_pci

2022-03-09 Thread Vasant Hegde via iommu
On 3/9/2022 12:41 PM, Dan Carpenter wrote:
> Hello Suravee Suthikulpanit,

Dan,

Thanks for the bug report.

.../...

> 
> 1979 pr_err("IOMMU: Failed to initialize IOMMU-API 
> interface (error=%d)!\n",
> 1980ret);
> 1981 goto out;
> 1982 }
> 1983 
> 1984 init_device_table_dma();
> 1985 
> 1986 for_each_iommu(iommu)
> 1987 iommu_flush_all_caches(iommu);
> 1988 
> --> 1989 if (!ret)
> 
> Where before we just checked for errors here.  This condition is
> impossible now.

We missed to remove this check. We will send fix patch.


-Vasant

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


Re: [PATCH v2 1/4] dt-bindings: arm-smmu: Document nvidia,memory-controller property

2022-03-09 Thread Will Deacon
On Tue, Mar 08, 2022 at 04:59:05PM +0100, Thierry Reding wrote:
> On Wed, Feb 16, 2022 at 02:25:20PM +0100, Thierry Reding wrote:
> > On Thu, Dec 09, 2021 at 05:35:57PM +0100, Thierry Reding wrote:
> > > From: Thierry Reding 
> > > 
> > > On NVIDIA SoC's the ARM SMMU needs to interact with the memory
> > > controller in order to map memory clients to the corresponding stream
> > > IDs. Document how the nvidia,memory-controller property can be used to
> > > achieve this.
> > > 
> > > Note that this is a backwards-incompatible change that is, however,
> > > necessary to ensure correctness. Without the new property, most of the
> > > devices would still work but it is not guaranteed that all will.
> > > 
> > > Signed-off-by: Thierry Reding 
> > > ---
> > > Changes in v2:
> > > - clarify why the new nvidia,memory-controller property is required
> > > 
> > >  .../devicetree/bindings/iommu/arm,smmu.yaml | 17 +
> > >  1 file changed, 17 insertions(+)
> > 
> > Hi Joerg,
> > 
> > can you pick up patches 1-3 of this series? DT bindings have been
> > reviewed by Rob and Will acked the ARM SMMU change. I can take the
> > device tree changes (patch 4) through the Tegra tree.
> 
> Will, Robin, Joerg,
> 
> I haven't seen this show up in linux-next yet but was hoping to see this
> go in for v5.18. Anything I can do to help this move along?

Hmm, I guess I could've taken 1-3, but after your message to Joerg I just
Acked the driver change on the assumption that was a dependency or
something.

In any case, I'm happy for Joerg to pick these three up directly if he has
time this late in the cycle.

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


[bug report] iommu/amd: Improve amd_iommu_v2_exit()

2022-03-09 Thread Dan Carpenter
Hello Suravee Suthikulpanit,

The patch d46c04558fdd: "iommu/amd: Improve amd_iommu_v2_exit()" from
Mar 1, 2022, leads to the following Smatch static checker warning:

drivers/iommu/amd/iommu_v2.c:133 free_device_state()
warn: sleeping in atomic context

drivers/iommu/amd/iommu_v2.c
   955static void __exit amd_iommu_v2_exit(void)
   956{
   957struct device_state *dev_state, *next;
   958unsigned long flags;
   959
   960if (!amd_iommu_v2_supported())
   961return;
   962
   963amd_iommu_unregister_ppr_notifier(&ppr_nb);
   964
   965flush_workqueue(iommu_wq);
   966
   967/*
   968 * The loop below might call flush_workqueue(), so call
   969 * destroy_workqueue() after it
   970 */
   971spin_lock_irqsave(&state_lock, flags);

Holding a spin lock.

   972
   973list_for_each_entry_safe(dev_state, next, &state_list, 
list) {
   974WARN_ON_ONCE(1);
   975
   976put_device_state(dev_state);
   977list_del(&dev_state->list);
   978free_device_state(dev_state);

Calling a sleeping function (wait_event()).

   979}
   980
   981spin_unlock_irqrestore(&state_lock, flags);
   982
   983destroy_workqueue(iommu_wq);
   984}

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


Re: [PATCH 11/12] swiotlb: merge swiotlb-xen initialization into swiotlb

2022-03-09 Thread Boris Ostrovsky



On 3/9/22 1:18 AM, Christoph Hellwig wrote:

On Tue, Mar 08, 2022 at 04:38:21PM -0500, Boris Ostrovsky wrote:

On 3/1/22 5:53 AM, Christoph Hellwig wrote:

Allow to pass a remap argument to the swiotlb initialization functions
to handle the Xen/x86 remap case.  ARM/ARM64 never did any remapping
from xen_swiotlb_fixup, so we don't even need that quirk.


Any chance this patch could be split? Lots of things are happening here and 
it's somewhat hard to review. (Patch 7 too BTW but I think I managed to get 
through it)

What would be your preferred split?



swiotlb_init() rework to be done separately?





diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index e0def4b1c3181..2f2c468acb955 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -71,15 +71,12 @@ static inline void __init pci_swiotlb_detect(void)
   #endif /* CONFIG_SWIOTLB */
 #ifdef CONFIG_SWIOTLB_XEN
-static bool xen_swiotlb;
-
   static void __init pci_xen_swiotlb_init(void)
   {
if (!xen_initial_domain() && !x86_swiotlb_enable)
return;


Now that there is a single call site for this routine I think this check can be 
dropped. We are only called here for xen_initial_domain()==true.

The callsite just checks xen_pv_domain() and itself is called
unconditionally during initialization.



Oh, right, nevermind. *pv* domain.


-boris

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


RE: [PATCH] Documentation: x86: add documenation for AMD IOMMU

2022-03-09 Thread Deucher, Alexander via iommu
[Public]

> -Original Message-
> From: Robin Murphy 
> Sent: Tuesday, March 8, 2022 3:09 PM
> To: Deucher, Alexander ; linux-
> d...@vger.kernel.org; linux-ker...@vger.kernel.org; cor...@lwn.net;
> h...@zytor.com; x...@kernel.org; dave.han...@linux.intel.com;
> b...@alien8.de; mi...@redhat.com; t...@linutronix.de; j...@8bytes.org;
> Suthikulpanit, Suravee ; w...@kernel.org;
> iommu@lists.linux-foundation.org
> Subject: Re: [PATCH] Documentation: x86: add documenation for AMD
> IOMMU
> 
> On 2022-03-08 19:04, Alex Deucher via iommu wrote:
> > Add preliminary documenation for AMD IOMMU.
> >
> > Signed-off-by: Alex Deucher 
> > ---
> >   Documentation/x86/amd-iommu.rst   | 85
> +++
> >   Documentation/x86/index.rst   |  1 +
> >   Documentation/x86/intel-iommu.rst |  2 +-
> >   3 files changed, 87 insertions(+), 1 deletion(-)
> >   create mode 100644 Documentation/x86/amd-iommu.rst
> >
> > diff --git a/Documentation/x86/amd-iommu.rst
> > b/Documentation/x86/amd-iommu.rst new file mode 100644 index
> > ..89820140fefa
> > --- /dev/null
> > +++ b/Documentation/x86/amd-iommu.rst
> > @@ -0,0 +1,85 @@
> > +=
> > +AMD IOMMU Support
> > +=
> > +
> > +The architecture spec can be obtained from the below location.
> > +
> >
> +https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fw
> ww
> >
> +.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F48882_IOMMU.pdf&da
> ta=04%7C
> >
> +01%7Calexander.deucher%40amd.com%7C3adb51f8c3f1435e0deb08da013f
> 8172%7
> >
> +C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637823669974023501
> %7CUnkn
> >
> +own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I
> k1haWw
> >
> +iLCJXVCI6Mn0%3D%7C3000&sdata=9Wq07GM%2BdT9xt%2FCZ3xhue
> %2BrNIe6CnD
> > +cG32kwqosUEZ8%3D&reserved=0
> > +
> > +This guide gives a quick cheat sheet for some basic understanding.
> > +
> > +Some Keywords
> > +
> > +- IVRS - I/O Virtualization Reporting Structure
> > +- IVDB - I/O Virtualization Definition Block
> > +- IVHD - I/O Virtualization Hardware Definition
> > +- IOVA - I/O Virtual Address.
> > +
> > +Basic stuff
> > +---
> > +
> > +ACPI enumerates and lists the different DMA engines in the platform,
> > +and device scope relationships between PCI devices and which DMA
> > +engine controls them.
> 
> "DMA engine" typically means a dedicated device for peripheral-to-memory
> or memory-to-memory transfers, or the responsible block within a general
> DMA-capable endpoint. In the context of the original Intel doc from whence I
> see this is copied, this probably should have said "DMAR unit"
> or similar; here I'd suggest picking your favourite vendor-appropriate term
> for "instance of IOMMU translation hardware". Let's not promote confusion
> more than necessary.
> 
> > +
> > +What is IVRS?
> > +-
> > +
> > +The architecture defines an ACPI-compatible data structure called an
> > +I/O Virtualization Reporting Structure (IVRS) that is used to convey
> > +information related to I/O virtualization to system software.  The
> > +IVRS describes the configuration and capabilities of the IOMMUs
> > +contained in the platform as well as information about the devices that
> each IOMMU virtualizes.
> > +
> > +The IVRS provides information about the following:
> > +- IOMMUs present in the platform including their capabilities and
> > +proper configuration
> > +- System I/O topology relevant to each IOMMU
> > +- Peripheral devices that cannot be otherwise enumerated
> > +- Memory regions used by SMI/SMM, platform firmware, and platform
> > +hardware. These are generally exclusion ranges to be configured by
> system software.
> > +
> > +How is IOVA generated?
> > +--
> > +
> > +Well behaved drivers call pci_map_*() calls before sending command to
> > +device
> 
> Horribly out-of-date drivers call pci_map_*(). Modern well-behaved drivers
> call dma_map_*() ;)
> 
> > +that needs to perform DMA. Once DMA is completed and mapping is no
> > +longer required, device performs a pci_unmap_*() calls to unmap the
> region.
> > +
> > +The AMD IOMMU driver allocates a virtual address per domain. Each
> > +PCIE device has its own domain (hence protection). Devices under p2p
> > +bridges share the virtual address with all devices under the p2p
> > +bridge due to transaction id aliasing for p2p bridges.
> > +
> > +IOVA generation is pretty generic. We used the same technique as
> > +vmalloc() but these are not global address spaces, but separate for each
> domain.
> > +Different DMA engines may support different number of domains.
> 
> I'm not sure about this whole section, really - IOVA management was entirely
> decoupled from drivers some time ago. If there's value in having some
> overview documentation, then it's probably worth consolidating into a
> common "IOMMU API" doc that can be cross-referenced for a summary of
> domains, groups, and iommu_dma_ops.
> 
> > +
> > +
> > +Fault reporting
> > +---

[PATCH v2 1/2] Documentation: x86: Add documenation for AMD IOMMU

2022-03-09 Thread Alex Deucher via iommu
Add preliminary documenation for AMD IOMMU.

Signed-off-by: Alex Deucher 
---

V2: incorporate feedback from Robin to clarify IOMMU vs DMA engine (e.g.,
a device) and document proper DMA API.  Also correct the fact that
the AMD IOMMU is not limited to managing PCI devices.

 Documentation/x86/amd-iommu.rst   | 69 +++
 Documentation/x86/index.rst   |  1 +
 Documentation/x86/intel-iommu.rst |  2 +-
 3 files changed, 71 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/x86/amd-iommu.rst

diff --git a/Documentation/x86/amd-iommu.rst b/Documentation/x86/amd-iommu.rst
new file mode 100644
index ..6ecc4bc8c70d
--- /dev/null
+++ b/Documentation/x86/amd-iommu.rst
@@ -0,0 +1,69 @@
+=
+AMD IOMMU Support
+=
+
+The architecture spec can be obtained from the below location.
+
+https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf
+
+This guide gives a quick cheat sheet for some basic understanding.
+
+Some Keywords
+
+- IVRS - I/O Virtualization Reporting Structure
+- IVDB - I/O Virtualization Definition Block
+- IVHD - I/O Virtualization Hardware Definition
+- IOVA - I/O Virtual Address.
+
+Basic stuff
+---
+
+ACPI enumerates and lists the different IOMMUs on the platform, and
+device scope relationships between devices and which IOMMU controls
+them.
+
+What is IVRS?
+-
+
+The architecture defines an ACPI-compatible data structure called an I/O
+Virtualization Reporting Structure (IVRS) that is used to convey information
+related to I/O virtualization to system software.  The IVRS describes the
+configuration and capabilities of the IOMMUs contained in the platform as
+well as information about the devices that each IOMMU virtualizes.
+
+The IVRS provides information about the following:
+- IOMMUs present in the platform including their capabilities and proper 
configuration
+- System I/O topology relevant to each IOMMU
+- Peripheral devices that cannot be otherwise enumerated
+- Memory regions used by SMI/SMM, platform firmware, and platform hardware. 
These are
+generally exclusion ranges to be configured by system software.
+
+How is IOVA generated?
+--
+
+Well behaved drivers call dma_map_*() calls before sending command to device
+that needs to perform DMA. Once DMA is completed and mapping is no longer
+required, driver performs dma_unmap_*() calls to unmap the region.
+
+Fault reporting
+---
+
+When errors are reported, the IOMMU signals via an interrupt. The fault
+reason and device that caused it with fault reason is printed on console.
+
+Boot Message Sample
+---
+
+Something like this gets printed indicating presence of the IOMMU.
+
+   iommu: Default domain type: Translated
+   iommu: DMA domain TLB invalidation policy: lazy mode
+
+Fault reporting
+^^^
+
+::
+
+   AMD-Vi: Event logged [IO_PAGE_FAULT domain=0x0007 address=0xc02000 
flags=0x]
+   AMD-Vi: Event logged [IO_PAGE_FAULT device=07:00.0 domain=0x0007 
address=0xc02000 flags=0x]
+
diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst
index f498f1d36cd3..15711134eb68 100644
--- a/Documentation/x86/index.rst
+++ b/Documentation/x86/index.rst
@@ -22,6 +22,7 @@ x86-specific Documentation
mtrr
pat
intel-iommu
+   amd-iommu
intel_txt
amd-memory-encryption
pti
diff --git a/Documentation/x86/intel-iommu.rst 
b/Documentation/x86/intel-iommu.rst
index 099f13d51d5f..4d3391c7bd3f 100644
--- a/Documentation/x86/intel-iommu.rst
+++ b/Documentation/x86/intel-iommu.rst
@@ -1,5 +1,5 @@
 ===
-Linux IOMMU Support
+Intel IOMMU Support
 ===
 
 The architecture spec can be obtained from the below location.
-- 
2.35.1

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


[PATCH 2/2] Documentation: x86: Clarify Intel IOMMU documenation

2022-03-09 Thread Alex Deucher via iommu
Based on feedback from Robin on the initial AMD IOMMU
documentation, fix up the Intel documentation to
clarify IOMMU vs device and modern DMA API.

Signed-off-by: Alex Deucher 
---
 Documentation/x86/intel-iommu.rst | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/x86/intel-iommu.rst 
b/Documentation/x86/intel-iommu.rst
index 4d3391c7bd3f..22e1934a1335 100644
--- a/Documentation/x86/intel-iommu.rst
+++ b/Documentation/x86/intel-iommu.rst
@@ -19,8 +19,8 @@ Some Keywords
 Basic stuff
 ---
 
-ACPI enumerates and lists the different DMA engines in the platform, and
-device scope relationships between PCI devices and which DMA engine  controls
+ACPI enumerates and lists the different IOMMUs in the platform, and
+device scope relationships between PCI devices and which IOMMU controls
 them.
 
 What is RMRR?
@@ -36,9 +36,9 @@ unity mappings for these regions for these devices to access 
these regions.
 How is IOVA generated?
 --
 
-Well behaved drivers call pci_map_*() calls before sending command to device
+Well behaved drivers call dma_map_*() calls before sending command to device
 that needs to perform DMA. Once DMA is completed and mapping is no longer
-required, device performs a pci_unmap_*() calls to unmap the region.
+required, device performs a dma_unmap_*() calls to unmap the region.
 
 The Intel IOMMU driver allocates a virtual address per domain. Each PCIE
 device has its own domain (hence protection). Devices under p2p bridges
@@ -68,7 +68,7 @@ address from PCI MMIO ranges so they are not allocated for 
IOVA addresses.
 
 Fault reporting
 ---
-When errors are reported, the DMA engine signals via an interrupt. The fault
+When errors are reported, the IOMMU signals via an interrupt. The fault
 reason and device that caused it with fault reason is printed on console.
 
 See below for sample.
-- 
2.35.1

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


Re: [PATCH 2/2] Documentation: x86: Clarify Intel IOMMU documenation

2022-03-09 Thread Dave Hansen
> -ACPI enumerates and lists the different DMA engines in the platform, and
> -device scope relationships between PCI devices and which DMA engine  controls
> +ACPI enumerates and lists the different IOMMUs in the platform, and
> +device scope relationships between PCI devices and which IOMMU controls
>  them.

Isn't this just a really long-winded way of saying:

ACPI enumerates both the IOMMUs in the platform and which IOMMU
controls a specific PCI device.

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


Re: [PATCH v2 1/2] Documentation: x86: Add documenation for AMD IOMMU

2022-03-09 Thread Vasant Hegde via iommu
On 3/9/2022 11:28 PM, Alex Deucher via iommu wrote:
> Add preliminary documenation for AMD IOMMU.

s/documenation /documentation/

> 
> Signed-off-by: Alex Deucher 
> ---
> 
> V2: incorporate feedback from Robin to clarify IOMMU vs DMA engine (e.g.,
> a device) and document proper DMA API.  Also correct the fact that
> the AMD IOMMU is not limited to managing PCI devices.
> 
>  Documentation/x86/amd-iommu.rst   | 69 +++
>  Documentation/x86/index.rst   |  1 +
>  Documentation/x86/intel-iommu.rst |  2 +-
>  3 files changed, 71 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/x86/amd-iommu.rst
> 
> diff --git a/Documentation/x86/amd-iommu.rst b/Documentation/x86/amd-iommu.rst
> new file mode 100644
> index ..6ecc4bc8c70d
> --- /dev/null
> +++ b/Documentation/x86/amd-iommu.rst
> @@ -0,0 +1,69 @@
> +=
> +AMD IOMMU Support
> +=
> +
> +The architecture spec can be obtained from the below location.
> +
> +https://www.amd.com/system/files/TechDocs/48882_IOMMU.pdf
> +
> +This guide gives a quick cheat sheet for some basic understanding.
> +
> +Some Keywords
> +
> +- IVRS - I/O Virtualization Reporting Structure
> +- IVDB - I/O Virtualization Definition Block
> +- IVHD - I/O Virtualization Hardware Definition
> +- IOVA - I/O Virtual Address.
> +
> +Basic stuff
> +---
> +
> +ACPI enumerates and lists the different IOMMUs on the platform, and
> +device scope relationships between devices and which IOMMU controls
> +them.
> +
> +What is IVRS?
> +-
> +
> +The architecture defines an ACPI-compatible data structure called an I/O
> +Virtualization Reporting Structure (IVRS) that is used to convey information
> +related to I/O virtualization to system software.  The IVRS describes the
> +configuration and capabilities of the IOMMUs contained in the platform as
> +well as information about the devices that each IOMMU virtualizes.
> +
> +The IVRS provides information about the following:
> +- IOMMUs present in the platform including their capabilities and proper 
> configuration
> +- System I/O topology relevant to each IOMMU
> +- Peripheral devices that cannot be otherwise enumerated
> +- Memory regions used by SMI/SMM, platform firmware, and platform hardware. 
> These are
> +generally exclusion ranges to be configured by system software.
> +
> +How is IOVA generated?
> +--
> +
> +Well behaved drivers call dma_map_*() calls before sending command to device
> +that needs to perform DMA. Once DMA is completed and mapping is no longer
> +required, driver performs dma_unmap_*() calls to unmap the region.
> +
> +Fault reporting
> +---
> +
> +When errors are reported, the IOMMU signals via an interrupt. The fault
> +reason and device that caused it with fault reason is printed on console.

May be just say "... and device cause it is printed on console."?

Rest looks good to me.

-Vasant

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


Re: [PATCH v2] dma-mapping: benchmark: Extract a common header file for map_benchmark definition

2022-03-09 Thread Christoph Hellwig
Thanks,

applied to the dma-mapping tree for Linux 5.18.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu