[PATCH/RFC] iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address space

[PATCH/RFC] iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address space

[Geert Uytterhoeven]

Currently, the IPMMU/VMSA driver supports 32-bit I/O Virtual Addresses
only, and thus sets io_pgtable_cfg.ias = 32.  However, it doesn't force
a 32-bit IOVA space through the IOMMU Domain Geometry.

Hence if a device (e.g. SYS-DMAC) rightfully configures a 40-bit DMA
mask, it will still be handed out a 40-bit IOVA, outside the 32-bit IOVA
space, leading to out-of-bounds accesses of the PGD when mapping the
IOVA.

Force a 32-bit IOMMU Domain Geometry to fix this.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Should the generic code restrict the geometry based on IAS instead?
---
 drivers/iommu/ipmmu-vmsa.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 798578f1676480c6..eb8b3df8733b15fb 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -424,6 +424,8 @@ static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
 	domain->cfg.ias = 32;
 	domain->cfg.oas = 40;
 	domain->cfg.tlb = &ipmmu_gather_ops;
+	domain->io_domain.geometry.aperture_end = DMA_BIT_MASK(32);
+	domain->io_domain.geometry.force_aperture = true;
 	/*
 	 * TODO: Add support for coherent walk through CCI with DVM and remove
 	 * cache handling. For now, delegate it to the io-pgtable code.
-- 
1.9.1

Re: [PATCH/RFC] iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address space

[Magnus Damm]

Hi Geert,

On Thu, Jan 26, 2017 at 6:53 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Currently, the IPMMU/VMSA driver supports 32-bit I/O Virtual Addresses
> only, and thus sets io_pgtable_cfg.ias = 32.  However, it doesn't force
> a 32-bit IOVA space through the IOMMU Domain Geometry.
>
> Hence if a device (e.g. SYS-DMAC) rightfully configures a 40-bit DMA
> mask, it will still be handed out a 40-bit IOVA, outside the 32-bit IOVA
> space, leading to out-of-bounds accesses of the PGD when mapping the
> IOVA.
>
> Force a 32-bit IOMMU Domain Geometry to fix this.

Nice, thanks!

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Should the generic code restrict the geometry based on IAS instead?

Might make sense. Since this is a software policy limited by hardware
it might be good to use as small IOVA space as possible to improve
performance. So selecting IOVA space based on slave device or domain
policy or something similar might be a good thing to do. Not sure how
to tie that into the IOMMU subsystem though...

Thanks!

/ magnus

Re: [PATCH/RFC] iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address space

[Robin Murphy]

On 26/01/17 09:53, Geert Uytterhoeven wrote:
> Currently, the IPMMU/VMSA driver supports 32-bit I/O Virtual Addresses
> only, and thus sets io_pgtable_cfg.ias = 32.  However, it doesn't force
> a 32-bit IOVA space through the IOMMU Domain Geometry.
> 
> Hence if a device (e.g. SYS-DMAC) rightfully configures a 40-bit DMA
> mask, it will still be handed out a 40-bit IOVA, outside the 32-bit IOVA
> space, leading to out-of-bounds accesses of the PGD when mapping the
> IOVA.
> 
> Force a 32-bit IOMMU Domain Geometry to fix this.

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Should the generic code restrict the geometry based on IAS instead?

Which generic code? IAS is specific to the io-pgtable library (well,
really it's an ARM-SMMU-ism, but "input address size" is a fairly
portable concept), but io-pgtable is just factored-out driver helper
code and doesn't know anything about iommu_domains and the IOMMU API.
Conversely, the IOMMU API core and kernel code beyond also know nothing
about io-pgtable internals - in fact the domain geometry *is* how the
IOMMU driver communicates its configured address space size to the
outside world.

Robin.

> ---
>  drivers/iommu/ipmmu-vmsa.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 798578f1676480c6..eb8b3df8733b15fb 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -424,6 +424,8 @@ static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
>  	domain->cfg.ias = 32;
>  	domain->cfg.oas = 40;
>  	domain->cfg.tlb = &ipmmu_gather_ops;
> +	domain->io_domain.geometry.aperture_end = DMA_BIT_MASK(32);
> +	domain->io_domain.geometry.force_aperture = true;
>  	/*
>  	 * TODO: Add support for coherent walk through CCI with DVM and remove
>  	 * cache handling. For now, delegate it to the io-pgtable code.
> 

Re: [PATCH/RFC] iommu/ipmmu-vmsa: Restrict IOMMU Domain Geometry to 32-bit address space

[Geert Uytterhoeven]

Hi Robin,

On Thu, Jan 26, 2017 at 12:23 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 26/01/17 09:53, Geert Uytterhoeven wrote:
>> Currently, the IPMMU/VMSA driver supports 32-bit I/O Virtual Addresses
>> only, and thus sets io_pgtable_cfg.ias = 32.  However, it doesn't force
>> a 32-bit IOVA space through the IOMMU Domain Geometry.
>>
>> Hence if a device (e.g. SYS-DMAC) rightfully configures a 40-bit DMA
>> mask, it will still be handed out a 40-bit IOVA, outside the 32-bit IOVA
>> space, leading to out-of-bounds accesses of the PGD when mapping the
>> IOVA.
>>
>> Force a 32-bit IOMMU Domain Geometry to fix this.
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Thanks!

>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> Should the generic code restrict the geometry based on IAS instead?
>
> Which generic code? IAS is specific to the io-pgtable library (well,
> really it's an ARM-SMMU-ism, but "input address size" is a fairly
> portable concept), but io-pgtable is just factored-out driver helper
> code and doesn't know anything about iommu_domains and the IOMMU API.
> Conversely, the IOMMU API core and kernel code beyond also know nothing
> about io-pgtable internals - in fact the domain geometry *is* how the
> IOMMU driver communicates its configured address space size to the
> outside world.

OK, so the IPMMU/VMSA does have to take care of it itself.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds