Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
On 27/04/2022 13:25, Andre Przywara wrote: > The Page Request Interface (PRI) is an optional PCIe feature. As such, a > SMMU would not need to handle it if the PCIe host bridge or the SMMU > itself do not implement it. Also an SMMU could be connected to a platform > device, without any PRI functionality whatsoever. > In all cases there would be no SMMU PRI queue interrupt to be wired up > to an interrupt controller. > > Relax the binding to allow specifying three interrupts, omitting the PRI > IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we > would need to sacrifice the command queue sync interrupt as well, which > might not be desired. > The Linux driver does not care about any order at all, just picks IRQs > based on their names, and treats all (wired) IRQs as optional. The last sentence is not a good explanation for the bindings. They are not about Linux and are used in other projects as well. > > Signed-off-by: Andre Przywara > --- > .../bindings/iommu/arm,smmu-v3.yaml | 21 ++- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > index e87bfbcc69135..6b3111f1f06ce 100644 > --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml > @@ -37,12 +37,23 @@ properties: >hardware supports just a single, combined interrupt line. >If provided, then the combined interrupt will be used in > preference to >any others. > - - minItems: 2 > + - minItems: 1 > items: > - - const: eventq # Event Queue not empty > - - const: gerror # Global Error activated > - - const: priq # PRI Queue not empty > - - const: cmdq-sync # CMD_SYNC complete > + - enum: > + - eventq # Event Queue not empty > + - gerror # Global Error activated > + - cmdq-sync # CMD_SYNC complete > + - priq # PRI Queue not empty > + - enum: > + - gerror > + - cmdq-sync > + - priq > + - enum: > + - cmdq-sync > + - priq > + - enum: > + - cmdq-sync > + - priq The order should be strict, so if you want the first interrupt optional, then: oneOf: - items: ... 4 items list - items ... 3 items list Best regards, Krzysztof ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RFC 08/12] iommufd: IOCTLs for the io_pagetable
On Thu, Mar 31, 2022 at 09:58:41AM -0300, Jason Gunthorpe wrote:
> On Thu, Mar 31, 2022 at 03:36:29PM +1100, David Gibson wrote:
>
> > > +/**
> > > + * struct iommu_ioas_iova_ranges - ioctl(IOMMU_IOAS_IOVA_RANGES)
> > > + * @size: sizeof(struct iommu_ioas_iova_ranges)
> > > + * @ioas_id: IOAS ID to read ranges from
> > > + * @out_num_iovas: Output total number of ranges in the IOAS
> > > + * @__reserved: Must be 0
> > > + * @out_valid_iovas: Array of valid IOVA ranges. The array length is the
> > > smaller
> > > + * of out_num_iovas or the length implied by size.
> > > + * @out_valid_iovas.start: First IOVA in the allowed range
> > > + * @out_valid_iovas.last: Inclusive last IOVA in the allowed range
> > > + *
> > > + * Query an IOAS for ranges of allowed IOVAs. Operation outside these
> > > ranges is
> > > + * not allowed. out_num_iovas will be set to the total number of iovas
> > > + * and the out_valid_iovas[] will be filled in as space permits.
> > > + * size should include the allocated flex array.
> > > + */
> > > +struct iommu_ioas_iova_ranges {
> > > + __u32 size;
> > > + __u32 ioas_id;
> > > + __u32 out_num_iovas;
> > > + __u32 __reserved;
> > > + struct iommu_valid_iovas {
> > > + __aligned_u64 start;
> > > + __aligned_u64 last;
> > > + } out_valid_iovas[];
> > > +};
> > > +#define IOMMU_IOAS_IOVA_RANGES _IO(IOMMUFD_TYPE,
> > > IOMMUFD_CMD_IOAS_IOVA_RANGES)
> >
> > Is the information returned by this valid for the lifeime of the IOAS,
> > or can it change? If it can change, what events can change it?
> >
> > If it *can't* change, then how do we have enough information to
> > determine this at ALLOC time, since we don't necessarily know which
> > (if any) hardware IOMMU will be attached to it.
>
> It is a good point worth documenting. It can change. Particularly
> after any device attachment.
Right.. this is vital and needs to be front and centre in the
comments/docs here. Really, I think an interface that *doesn't* have
magically changing status would be better (which is why I was
advocating that the user set the constraints, and the kernel supplied
or failed outright). Still I recognize that has its own problems.
> I added this:
>
> * Query an IOAS for ranges of allowed IOVAs. Mapping IOVA outside these
> ranges
> * is not allowed. out_num_iovas will be set to the total number of iovas and
> * the out_valid_iovas[] will be filled in as space permits. size should
> include
> * the allocated flex array.
> *
> * The allowed ranges are dependent on the HW path the DMA operation takes,
> and
> * can change during the lifetime of the IOAS. A fresh empty IOAS will have a
> * full range, and each attached device will narrow the ranges based on that
> * devices HW restrictions.
I think you need to be even more explicit about this: which exact
operations on the fd can invalidate exactly which items in the
information from this call? Can it only ever be narrowed, or can it
be broadened with any operations?
> > > +#define IOMMU_IOAS_COPY _IO(IOMMUFD_TYPE, IOMMUFD_CMD_IOAS_COPY)
> >
> > Since it can only copy a single mapping, what's the benefit of this
> > over just repeating an IOAS_MAP in the new IOAS?
>
> It causes the underlying pin accounting to be shared and can avoid
> calling GUP entirely.
If that's the only purpose, then that needs to be right here in the
comments too. So is expected best practice to IOAS_MAP everything you
might want to map into a sort of "scratch" IOAS, then IOAS_COPY the
mappings you actually end up wanting into the "real" IOASes for use?
Seems like it would be nicer for the interface to just figure it out
for you: I can see there being sufficient complications with that to
have this slightly awkward interface, but I think it needs a rationale
to accompany it.
--
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5] iommu/sun50i: Allwinner D1 support
Hi Samuel!
Dne četrtek, 28. april 2022 ob 03:03:55 CEST je Samuel Holland napisal(a):
> D1 is a RISC-V SoC from Allwinner's sunxi family. This series adds IOMMU
> binding and driver support.
>
> One piece is still missing to use the IOMMU for DMA allocations: a call
> to iommu_setup_dma_ops(). On ARM64 this is handled by the architecture's
> code. RISC-V does not currently select ARCH_HAS_SETUP_DMA_OPS, but it
> will once Zicbom support[1] is merged.
>
> [1]: https://lore.kernel.org/lkml/20220307224620.1933061-2-he...@sntech.de/
>
> So I cannot follow virtio-iommu.c and call iommu_setup_dma_ops() when
> ARCH_HAS_SETUP_DMA_OPS=n. However, if I apply the following patch on top
> of Heiko's non-coherent DMA series, the display engine successfully uses
> the IOMMU to allocate its framebuffer:
Did you test this on any other device than display pipeline? It should be
supported by Cedrus too, right? I think there are still some corner cases to
fix on Cedrus before IOMMU fully works.
Best regards,
Jernej
>
> --- a/arch/riscv/mm/dma-noncoherent.c
> +++ b/arch/riscv/mm/dma-noncoherent.c
> @@ -6,6 +6,7 @@
> */
>
> #include
> +#include
> #include
> #include
>
> @@ -53,4 +54,7 @@
> {
> /* If a specific device is dma-coherent, set it here */
> dev->dma_coherent = coherent;
> +
> + if (iommu)
> + iommu_setup_dma_ops(dev, dma_base, dma_base + size - 1);
> }
>
>
> Samuel Holland (5):
> dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
> iommu/sun50i: Support variants without an external reset
> iommu/sun50i: Ensure bypass is disabled
> iommu/sun50i: Add support for the D1 variant
> iommu/sun50i: Ensure the IOMMU can be used for DMA
>
> .../iommu/allwinner,sun50i-h6-iommu.yaml | 16 +++--
> drivers/iommu/Kconfig | 1 +
> drivers/iommu/sun50i-iommu.c | 24 +--
> 3 files changed, 37 insertions(+), 4 deletions(-)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/5] iommu/sun50i: Ensure bypass is disabled
Dne četrtek, 28. april 2022 ob 03:03:58 CEST je Samuel Holland napisal(a): > The H6 variant of the hardware disables bypass by default. The D1 > variant of the hardware enables bypass for all masters by default. > > Since the driver expects bypass to be disabled, ensure that is the case. > > Signed-off-by: Samuel Holland Actually, it would be better to set bypass to 0xff and in sun50i_iommu_attach_device() clear bypass bit for that particular device. As you might notice, index in phandle is currently not used. This would also help expose bugs, like missing second iommu channel for Cedrus on H6, but that's easy to fix. Best regards, Jernej > --- > > drivers/iommu/sun50i-iommu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c > index ec07b60016d3..b9e644b93637 100644 > --- a/drivers/iommu/sun50i-iommu.c > +++ b/drivers/iommu/sun50i-iommu.c > @@ -374,6 +374,8 @@ static int sun50i_iommu_enable(struct sun50i_iommu > *iommu) > > spin_lock_irqsave(&iommu->iommu_lock, flags); > > + iommu_write(iommu, IOMMU_BYPASS_REG, 0); > + > iommu_write(iommu, IOMMU_TTB_REG, sun50i_domain->dt_dma); > iommu_write(iommu, IOMMU_TLB_PREFETCH_REG, > IOMMU_TLB_PREFETCH_MASTER_ENABLE(0) | ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/5] iommu/sun50i: Ensure the IOMMU can be used for DMA
Dne četrtek, 28. april 2022 ob 03:04:00 CEST je Samuel Holland napisal(a): > So far, the driver has relied on arch/arm64/Kconfig to select IOMMU_DMA. > Unsurprisingly, this does not work on RISC-V, so the driver must select > IOMMU_DMA itself. > > Signed-off-by: Samuel Holland Reviewed-by: Jernej Skrabec Best regards, Jernej > --- > > drivers/iommu/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index c79a0df090c0..70a0bfa6d907 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -223,6 +223,7 @@ config SUN50I_IOMMU > depends on ARCH_SUNXI || COMPILE_TEST > select ARM_DMA_USE_IOMMU > select IOMMU_API > + select IOMMU_DMA > help > Support for the IOMMU introduced in the Allwinner H6 SoCs. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] iommu/sun50i: Add support for the D1 variant
Dne četrtek, 28. april 2022 ob 03:03:59 CEST je Samuel Holland napisal(a):
> D1 contains an IOMMU similar to the one in the H6 SoC, but the D1
> variant has no external reset signal. It also has some register
> definition changes, but none that affect the current driver.
>
> Signed-off-by: Samuel Holland
Reviewed-by: Jernej Skrabec
Best regards,
Jernej
> ---
>
> drivers/iommu/sun50i-iommu.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> index b9e644b93637..1fb707e37fb3 100644
> --- a/drivers/iommu/sun50i-iommu.c
> +++ b/drivers/iommu/sun50i-iommu.c
> @@ -999,11 +999,15 @@ static int sun50i_iommu_probe(struct platform_device
> *pdev) return ret;
> }
>
> +static const struct sun50i_iommu_variant sun20i_d1_iommu = {
> +};
> +
> static const struct sun50i_iommu_variant sun50i_h6_iommu = {
> .has_reset = true,
> };
>
> static const struct of_device_id sun50i_iommu_dt[] = {
> + { .compatible = "allwinner,sun20i-d1-iommu", .data =
&sun20i_d1_iommu },
> { .compatible = "allwinner,sun50i-h6-iommu", .data =
&sun50i_h6_iommu },
> { /* sentinel */ },
> };
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/5] iommu/sun50i: Support variants without an external reset
Dne četrtek, 28. april 2022 ob 03:03:57 CEST je Samuel Holland napisal(a):
> The IOMMU in the Allwinner D1 SoC does not have an external reset line.
>
> Only attempt to get the reset on hardware variants which should have one
> according to the binding. And switch from the deprecated function to the
> explicit "exclusive" variant.
>
> Signed-off-by: Samuel Holland
Reviewed-by: Jernej Skrabec
Best regards,
Jernej
> ---
>
> drivers/iommu/sun50i-iommu.c | 18 --
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
> index c54ab477b8fd..ec07b60016d3 100644
> --- a/drivers/iommu/sun50i-iommu.c
> +++ b/drivers/iommu/sun50i-iommu.c
> @@ -92,6 +92,10 @@
> #define NUM_PT_ENTRIES 256
> #define PT_SIZE (NUM_PT_ENTRIES *
PT_ENTRY_SIZE)
>
> +struct sun50i_iommu_variant {
> + bool has_reset;
> +};
> +
> struct sun50i_iommu {
> struct iommu_device iommu;
>
> @@ -905,9 +909,14 @@ static irqreturn_t sun50i_iommu_irq(int irq, void
> *dev_id)
>
> static int sun50i_iommu_probe(struct platform_device *pdev)
> {
> + const struct sun50i_iommu_variant *variant;
> struct sun50i_iommu *iommu;
> int ret, irq;
>
> + variant = of_device_get_match_data(&pdev->dev);
> + if (!variant)
> + return -EINVAL;
> +
> iommu = devm_kzalloc(&pdev->dev, sizeof(*iommu), GFP_KERNEL);
> if (!iommu)
> return -ENOMEM;
> @@ -947,7 +956,8 @@ static int sun50i_iommu_probe(struct platform_device
> *pdev) goto err_free_group;
> }
>
> - iommu->reset = devm_reset_control_get(&pdev->dev, NULL);
> + if (variant->has_reset)
> + iommu->reset = devm_reset_control_get_exclusive(&pdev-
>dev, NULL);
> if (IS_ERR(iommu->reset)) {
> dev_err(&pdev->dev, "Couldn't get our reset line.\n");
> ret = PTR_ERR(iommu->reset);
> @@ -987,8 +997,12 @@ static int sun50i_iommu_probe(struct platform_device
> *pdev) return ret;
> }
>
> +static const struct sun50i_iommu_variant sun50i_h6_iommu = {
> + .has_reset = true,
> +};
> +
> static const struct of_device_id sun50i_iommu_dt[] = {
> - { .compatible = "allwinner,sun50i-h6-iommu", },
> + { .compatible = "allwinner,sun50i-h6-iommu", .data =
&sun50i_h6_iommu },
> { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, sun50i_iommu_dt);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/5] iommu/sun50i: Add support for the D1 variant
D1 contains an IOMMU similar to the one in the H6 SoC, but the D1
variant has no external reset signal. It also has some register
definition changes, but none that affect the current driver.
Signed-off-by: Samuel Holland
---
drivers/iommu/sun50i-iommu.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index b9e644b93637..1fb707e37fb3 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -999,11 +999,15 @@ static int sun50i_iommu_probe(struct platform_device
*pdev)
return ret;
}
+static const struct sun50i_iommu_variant sun20i_d1_iommu = {
+};
+
static const struct sun50i_iommu_variant sun50i_h6_iommu = {
.has_reset = true,
};
static const struct of_device_id sun50i_iommu_dt[] = {
+ { .compatible = "allwinner,sun20i-d1-iommu", .data = &sun20i_d1_iommu },
{ .compatible = "allwinner,sun50i-h6-iommu", .data = &sun50i_h6_iommu },
{ /* sentinel */ },
};
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/5] iommu/sun50i: Allwinner D1 support
D1 is a RISC-V SoC from Allwinner's sunxi family. This series adds IOMMU
binding and driver support.
One piece is still missing to use the IOMMU for DMA allocations: a call
to iommu_setup_dma_ops(). On ARM64 this is handled by the architecture's
code. RISC-V does not currently select ARCH_HAS_SETUP_DMA_OPS, but it
will once Zicbom support[1] is merged.
[1]: https://lore.kernel.org/lkml/20220307224620.1933061-2-he...@sntech.de/
So I cannot follow virtio-iommu.c and call iommu_setup_dma_ops() when
ARCH_HAS_SETUP_DMA_OPS=n. However, if I apply the following patch on top
of Heiko's non-coherent DMA series, the display engine successfully uses
the IOMMU to allocate its framebuffer:
--- a/arch/riscv/mm/dma-noncoherent.c
+++ b/arch/riscv/mm/dma-noncoherent.c
@@ -6,6 +6,7 @@
*/
#include
+#include
#include
#include
@@ -53,4 +54,7 @@
{
/* If a specific device is dma-coherent, set it here */
dev->dma_coherent = coherent;
+
+ if (iommu)
+ iommu_setup_dma_ops(dev, dma_base, dma_base + size - 1);
}
Samuel Holland (5):
dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
iommu/sun50i: Support variants without an external reset
iommu/sun50i: Ensure bypass is disabled
iommu/sun50i: Add support for the D1 variant
iommu/sun50i: Ensure the IOMMU can be used for DMA
.../iommu/allwinner,sun50i-h6-iommu.yaml | 16 +++--
drivers/iommu/Kconfig | 1 +
drivers/iommu/sun50i-iommu.c | 24 +--
3 files changed, 37 insertions(+), 4 deletions(-)
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/5] iommu/sun50i: Ensure bypass is disabled
The H6 variant of the hardware disables bypass by default. The D1 variant of the hardware enables bypass for all masters by default. Since the driver expects bypass to be disabled, ensure that is the case. Signed-off-by: Samuel Holland --- drivers/iommu/sun50i-iommu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index ec07b60016d3..b9e644b93637 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -374,6 +374,8 @@ static int sun50i_iommu_enable(struct sun50i_iommu *iommu) spin_lock_irqsave(&iommu->iommu_lock, flags); + iommu_write(iommu, IOMMU_BYPASS_REG, 0); + iommu_write(iommu, IOMMU_TTB_REG, sun50i_domain->dt_dma); iommu_write(iommu, IOMMU_TLB_PREFETCH_REG, IOMMU_TLB_PREFETCH_MASTER_ENABLE(0) | -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/5] dt-bindings: iommu: sun50i: Add compatible for Allwinner D1
D1 contains an IOMMU similar to the one in the H6 SoC, but the D1 variant has no external reset signal. Signed-off-by: Samuel Holland --- .../iommu/allwinner,sun50i-h6-iommu.yaml | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml index 5e125cf2a88b..18d3451d4dd5 100644 --- a/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml +++ b/Documentation/devicetree/bindings/iommu/allwinner,sun50i-h6-iommu.yaml @@ -17,7 +17,9 @@ properties: The content of the cell is the master ID. compatible: -const: allwinner,sun50i-h6-iommu +enum: + - allwinner,sun20i-d1-iommu + - allwinner,sun50i-h6-iommu reg: maxItems: 1 @@ -37,7 +39,17 @@ required: - reg - interrupts - clocks - - resets + +if: + properties: +compatible: + contains: +enum: + - allwinner,sun50i-h6-iommu + +then: + required: +- resets additionalProperties: false -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/5] iommu/sun50i: Support variants without an external reset
The IOMMU in the Allwinner D1 SoC does not have an external reset line.
Only attempt to get the reset on hardware variants which should have one
according to the binding. And switch from the deprecated function to the
explicit "exclusive" variant.
Signed-off-by: Samuel Holland
---
drivers/iommu/sun50i-iommu.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c
index c54ab477b8fd..ec07b60016d3 100644
--- a/drivers/iommu/sun50i-iommu.c
+++ b/drivers/iommu/sun50i-iommu.c
@@ -92,6 +92,10 @@
#define NUM_PT_ENTRIES 256
#define PT_SIZE(NUM_PT_ENTRIES * PT_ENTRY_SIZE)
+struct sun50i_iommu_variant {
+ bool has_reset;
+};
+
struct sun50i_iommu {
struct iommu_device iommu;
@@ -905,9 +909,14 @@ static irqreturn_t sun50i_iommu_irq(int irq, void *dev_id)
static int sun50i_iommu_probe(struct platform_device *pdev)
{
+ const struct sun50i_iommu_variant *variant;
struct sun50i_iommu *iommu;
int ret, irq;
+ variant = of_device_get_match_data(&pdev->dev);
+ if (!variant)
+ return -EINVAL;
+
iommu = devm_kzalloc(&pdev->dev, sizeof(*iommu), GFP_KERNEL);
if (!iommu)
return -ENOMEM;
@@ -947,7 +956,8 @@ static int sun50i_iommu_probe(struct platform_device *pdev)
goto err_free_group;
}
- iommu->reset = devm_reset_control_get(&pdev->dev, NULL);
+ if (variant->has_reset)
+ iommu->reset = devm_reset_control_get_exclusive(&pdev->dev,
NULL);
if (IS_ERR(iommu->reset)) {
dev_err(&pdev->dev, "Couldn't get our reset line.\n");
ret = PTR_ERR(iommu->reset);
@@ -987,8 +997,12 @@ static int sun50i_iommu_probe(struct platform_device *pdev)
return ret;
}
+static const struct sun50i_iommu_variant sun50i_h6_iommu = {
+ .has_reset = true,
+};
+
static const struct of_device_id sun50i_iommu_dt[] = {
- { .compatible = "allwinner,sun50i-h6-iommu", },
+ { .compatible = "allwinner,sun50i-h6-iommu", .data = &sun50i_h6_iommu },
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, sun50i_iommu_dt);
--
2.35.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/5] iommu/sun50i: Ensure the IOMMU can be used for DMA
So far, the driver has relied on arch/arm64/Kconfig to select IOMMU_DMA. Unsurprisingly, this does not work on RISC-V, so the driver must select IOMMU_DMA itself. Signed-off-by: Samuel Holland --- drivers/iommu/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index c79a0df090c0..70a0bfa6d907 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -223,6 +223,7 @@ config SUN50I_IOMMU depends on ARCH_SUNXI || COMPILE_TEST select ARM_DMA_USE_IOMMU select IOMMU_API + select IOMMU_DMA help Support for the IOMMU introduced in the Allwinner H6 SoCs. -- 2.35.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 05/11] iommu/sva: Assign a PASID to mm on PASID allocation and free it on mm exit
Hi, Dave and Jean,
On Tue, Apr 26, 2022 at 01:04:45PM +0800, Zhangfei Gao wrote:
>
>
> On 2022/4/26 下午12:20, Fenghua Yu wrote:
> > Hi, Jean and Zhangfei,
> >
> > On Mon, Apr 25, 2022 at 05:13:02PM +0100, Jean-Philippe Brucker wrote:
> > > Could we move mm_pasid_drop() to __mmdrop() instead of __mmput()? For Arm
> > > we do need to hold the mm_count until unbind(), and mmgrab()/mmdrop() is
> > > also part of Lu's rework [1].
> > Is this a right fix for the issue? Could you please test it on ARM?
> > I don't have an ARM machine.
> >
> > Thanks.
> >
> > -Fenghua
> >
> > From 84aa68f6174439d863c40cdc2db0e1b89d620dd0 Mon Sep 17 00:00:00 2001
> > From: Fenghua Yu
> > Date: Fri, 15 Apr 2022 00:51:33 -0700
> > Subject: [PATCH] iommu/sva: Fix PASID use-after-free issue
> >
> > A PASID might be still used on ARM after it is freed in __mmput().
> >
> > process:
> > open()->sva_bind()->ioasid_alloc() = N; // Get PASID N for the mm
> > exit();
> > exit_mm()->__mmput()->mm_pasid_drop()->mm->pasid = -1; // PASID -1
> > exit_files()->release(dev)->sva_unbind()->use mm->pasid; // Failure
> >
> > To avoid the use-after-free issue, free the PASID after no device uses it,
> > i.e. after all devices are unbound from the mm.
> >
> > sva_bind()/sva_unbind() call mmgrab()/mmdrop() to track mm->mm_count.
> > __mmdrop() is called only after mm->mm_count is zero. So freeing the PASID
> > in __mmdrop() guarantees the PASID is safely freed only after no device
> > is bound to the mm.
> >
> > Fixes: 701fac40384f ("iommu/sva: Assign a PASID to mm on PASID allocation
> > and free it on mm exit")
> >
> > Reported-by: Zhangfei Gao
> > Suggested-by: Jean-Philippe Brucker
> > Suggested-by: Jacob Pan
> > Signed-off-by: Fenghua Yu
> Thanks for the fix.
>
> Tested-by: Zhangfei Gao
>
>
> > ---
> > kernel/fork.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index 9796897560ab..35a3beff140b 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -792,6 +792,7 @@ void __mmdrop(struct mm_struct *mm)
> > mmu_notifier_subscriptions_destroy(mm);
> > check_mm(mm);
> > put_user_ns(mm->user_ns);
> > + mm_pasid_drop(mm);
> > free_mm(mm);
> > }
> > EXPORT_SYMBOL_GPL(__mmdrop);
> > @@ -1190,7 +1191,6 @@ static inline void __mmput(struct mm_struct *mm)
> > }
> > if (mm->binfmt)
> > module_put(mm->binfmt->module);
> > - mm_pasid_drop(mm);
> > mmdrop(mm);
> > }
>
Is this patch a good fix? Will you help push the fix into upstream?
Thank you very much!
-Fenghua
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] Documentation: x86: rework IOMMU documentation
[Public] > -Original Message- > From: Jacob Pan > Sent: Tuesday, April 26, 2022 12:45 PM > To: Deucher, Alexander > Cc: linux-...@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; robin.mur...@arm.com; Hegde, Vasant > ; jacob.jun@intel.com; Lu, Baolu > > Subject: Re: [PATCH] Documentation: x86: rework IOMMU documentation > > Hi Alex, > > Thanks for doing this, really helps to catch up the current state. Please see > my > comments inline. > > On Fri, 22 Apr 2022 16:06:07 -0400, Alex Deucher > wrote: > > > Add preliminary documentation for AMD IOMMU and combine with the > > existing Intel IOMMU documentation and clean up and modernize some of > > the existing documentation to align with the current state of the > > kernel. > > > > 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. > > v3: Fix spelling and rework text as suggested by Vasant > > v4: Combine Intel and AMD documents into a single document as suggested > > by Dave Hansen > > v5: Clarify that keywords are related to ACPI, grammatical fixes > > v6: Make more stuff common based on feedback from Robin > > > > Documentation/x86/index.rst | 2 +- > > Documentation/x86/intel-iommu.rst | 115 > > Documentation/x86/iommu.rst | 143 > ++ > > 3 files changed, 144 insertions(+), 116 deletions(-) delete mode > > 100644 Documentation/x86/intel-iommu.rst create mode 100644 > > Documentation/x86/iommu.rst > > > > diff --git a/Documentation/x86/index.rst b/Documentation/x86/index.rst > > index f498f1d36cd3..6f8409fe0674 100644 > > --- a/Documentation/x86/index.rst > > +++ b/Documentation/x86/index.rst > > @@ -21,7 +21,7 @@ x86-specific Documentation > > tlb > > mtrr > > pat > > - intel-iommu > > + iommu > > intel_txt > > amd-memory-encryption > > pti > > diff --git a/Documentation/x86/intel-iommu.rst > > b/Documentation/x86/intel-iommu.rst deleted file mode 100644 index > > 099f13d51d5f.. > > --- a/Documentation/x86/intel-iommu.rst > > +++ /dev/null > > @@ -1,115 +0,0 @@ > > -=== > > -Linux IOMMU Support > > -=== > > - > > -The architecture spec can be obtained from the below location. > > - > > - > https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. > > > intel.com%2Fcontent%2Fdam%2Fwww%2Fpublic%2Fus%2Fen%2Fdocuments > %2Fprodu > > ct-specifications%2Fvt-directed-io- > spec.pdf&data=05%7C01%7Calexand > > > er.deucher%40amd.com%7C929847a4b2524432d1a608da27a3c9b0%7C3dd > 8961fe488 > > > 4e608e11a82d994e183d%7C0%7C0%7C637865881851295857%7CUnknow > n%7CTWFpbGZs > > > b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0 > %3D > > > %7C3000%7C%7C%7C&sdata=KaPkvBSHWbn1cKBRzyk9H%2BuNDll%2Bq > 3wPfR3SFVA > > LwkU%3D&reserved=0 > > - > > -This guide gives a quick cheat sheet for some basic understanding. > > - > > -Some Keywords > > - > > -- DMAR - DMA remapping > > -- DRHD - DMA Remapping Hardware Unit Definition > > -- RMRR - Reserved memory Region Reporting Structure > > -- ZLR - Zero length reads from PCI devices > > -- IOVA - IO Virtual address. > > - > I feel this combined document only focus on IOVA and DMA APIs, it is > considered as legacy DMA after scalable mode is introduced by Intel to > support DMA with PASID, shared virtual addressing (SVA). > Perhaps, we can also combine ./Documentation/x86/sva.rst I think it would make sense to take that up in a separate patch set. > > With scalable mode, it affects boot messages, fault reporting, etc. I am not > saying no to this document, just suggesting. I don't know where AMD is at in > terms of PASID support but there are lots of things in common between VT-d > and ARM's SMMU in terms of PASID/SVA. Should we broaden the purpose of > this document even further? I think that would make sense for a future clean up. I'd like to land the current clean up first. AMD's IOMMU driver has supported PASID for probably 8-10 years. When we originally added it no other vendors were interested in supporting it so we made it a private API which was used by other AMD drivers that needed it at the time. Suravee can probably comment on the status of our support for the cross vendor API. Alex > > > -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. > > - > > -What is RMRR? > > -- > > - > > -There are some devices the BIOS contr
Re: [PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
On 2022-04-27 12:25, Andre Przywara wrote: The Page Request Interface (PRI) is an optional PCIe feature. As such, a SMMU would not need to handle it if the PCIe host bridge or the SMMU itself do not implement it. Also an SMMU could be connected to a platform device, without any PRI functionality whatsoever. In all cases there would be no SMMU PRI queue interrupt to be wired up to an interrupt controller. Relax the binding to allow specifying three interrupts, omitting the PRI IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we would need to sacrifice the command queue sync interrupt as well, which might not be desired. The Linux driver does not care about any order at all, just picks IRQs based on their names, and treats all (wired) IRQs as optional. Signed-off-by: Andre Przywara --- .../bindings/iommu/arm,smmu-v3.yaml | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml index e87bfbcc69135..6b3111f1f06ce 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml @@ -37,12 +37,23 @@ properties: hardware supports just a single, combined interrupt line. If provided, then the combined interrupt will be used in preference to any others. - - minItems: 2 + - minItems: 1 items: - - const: eventq # Event Queue not empty - - const: gerror # Global Error activated - - const: priq # PRI Queue not empty - - const: cmdq-sync # CMD_SYNC complete + - enum: + - eventq # Event Queue not empty + - gerror # Global Error activated + - cmdq-sync # CMD_SYNC complete + - priq # PRI Queue not empty + - enum: + - gerror + - cmdq-sync + - priq + - enum: + - cmdq-sync + - priq + - enum: + - cmdq-sync + - priq Nit: the commit message doesn't mention this effective relaxation to allow "eventq, gerror, cmdq-sync, priq" as well - if there's a good reason for that it probably wants calling out, otherwise AFAICS the last item should stay as "- const: cmdq-sync" to preserve the existing canonical order for the 4-item case. Otherwise, Reviewed-by: Robin Murphy Cheers, Robin. '#iommu-cells': const: 1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 01/11] dt-bindings: iommu: arm,smmu-v3: make PRI IRQ optional
The Page Request Interface (PRI) is an optional PCIe feature. As such, a SMMU would not need to handle it if the PCIe host bridge or the SMMU itself do not implement it. Also an SMMU could be connected to a platform device, without any PRI functionality whatsoever. In all cases there would be no SMMU PRI queue interrupt to be wired up to an interrupt controller. Relax the binding to allow specifying three interrupts, omitting the PRI IRQ. At the moment, with the "eventq,gerror,priq,cmdq-sync" order, we would need to sacrifice the command queue sync interrupt as well, which might not be desired. The Linux driver does not care about any order at all, just picks IRQs based on their names, and treats all (wired) IRQs as optional. Signed-off-by: Andre Przywara --- .../bindings/iommu/arm,smmu-v3.yaml | 21 ++- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml index e87bfbcc69135..6b3111f1f06ce 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml @@ -37,12 +37,23 @@ properties: hardware supports just a single, combined interrupt line. If provided, then the combined interrupt will be used in preference to any others. - - minItems: 2 + - minItems: 1 items: - - const: eventq # Event Queue not empty - - const: gerror # Global Error activated - - const: priq # PRI Queue not empty - - const: cmdq-sync # CMD_SYNC complete + - enum: + - eventq # Event Queue not empty + - gerror # Global Error activated + - cmdq-sync # CMD_SYNC complete + - priq # PRI Queue not empty + - enum: + - gerror + - cmdq-sync + - priq + - enum: + - cmdq-sync + - priq + - enum: + - cmdq-sync + - priq '#iommu-cells': const: 1 -- 2.25.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Personal email outage
Hi, last week I had an outage of the mail server which handles my 8bytes.org emails, and I probably lost some of the incoming stuff that was sent to me. The machine has been repaired now and I can receive emails again on that account, but it is likely that I still lost a couple of emails (at least I have seen several replies to emails I didn't get). I plan to go through all IOMMU patches tomorrow, if I havn't replied to your patches by Friday, please just re-send them to me. Sorry for the inconveniences and delays! Thanks, Joerg signature.asc Description: Digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
