| Message ID | 1485424391-29914-1-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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
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. >
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
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.
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(+)