| Message ID | 20200911170738.82818-3-leobras.c@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | DDW Indirect Mapping | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (4b552a4cbf286ff9dcdab19153f3c1c7d1680fab) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 24 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On 12/09/2020 03:07, Leonardo Bras wrote: > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, > > Having System pagesize < IOMMU pagesize may cause a page owned by another > process/VM to be written by a buggy driver / device. > > As it's intended to use DDW for indirect mapping, it's possible to get > a configuration where (PAGE_SIZE = 4k) < (IOMMU_PAGE_SIZE() = 64k). Have you tried 4K VM kernel under phyp with 64K iommu pages? phyp may assume it is still correct as it knows that it allocates memory in 256MB contiguous chunks and for DMA purposes phyp may assume that there just a VM kernel which owns all the VM physical memory and can align size in iommu_range_alloc() to the IOMMU page size. Not sure. > > To avoid this, make sure create_ddw() can only use pagesize <= PAGE_SIZE. > > Signed-off-by: Leonardo Bras <leobras.c@gmail.com> > --- > arch/powerpc/platforms/pseries/iommu.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > index 9db3927607a4..4eff3a6a441e 100644 > --- a/arch/powerpc/platforms/pseries/iommu.c > +++ b/arch/powerpc/platforms/pseries/iommu.c > @@ -1206,17 +1206,20 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) > goto out_failed; > } > } > - if (query.page_size & 4) { > - page_shift = 24; /* 16MB */ > - } else if (query.page_size & 2) { > + > + /* Choose the biggest pagesize available <= PAGE_SHIFT */ > + if ((query.page_size & 4)) { This just doubles braces. Also I believe for the indirect case you should not even try 16MB. > + page_shift = 24; /* 16MB - Hugepages */ > + } else if ((query.page_size & 2) && PAGE_SHIFT >= 16) { PAGE_SHIFT == 16 > page_shift = 16; /* 64kB */ > - } else if (query.page_size & 1) { > + } else if ((query.page_size & 1) && PAGE_SHIFT >= 12) { Kind of useless check, if it is not 64K, it is 4K. I understand you are trying to be safe here but this give a new reader an idea of more flexibility that there is :) BUILD_BUG_ON() would do here imho. > page_shift = 12; /* 4kB */ > } else { > dev_dbg(&dev->dev, "no supported direct page size in mask %x", > query.page_size); > goto out_failed; > } > + Unrelated. > /* verify the window * number of ptes will map the partition */ > /* check largest block * page size > max memory hotplug addr */ > max_addr = ddw_memory_hotplug_max(); >
diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c index 9db3927607a4..4eff3a6a441e 100644 --- a/arch/powerpc/platforms/pseries/iommu.c +++ b/arch/powerpc/platforms/pseries/iommu.c @@ -1206,17 +1206,20 @@ static u64 enable_ddw(struct pci_dev *dev, struct device_node *pdn) goto out_failed; } } - if (query.page_size & 4) { - page_shift = 24; /* 16MB */ - } else if (query.page_size & 2) { + + /* Choose the biggest pagesize available <= PAGE_SHIFT */ + if ((query.page_size & 4)) { + page_shift = 24; /* 16MB - Hugepages */ + } else if ((query.page_size & 2) && PAGE_SHIFT >= 16) { page_shift = 16; /* 64kB */ - } else if (query.page_size & 1) { + } else if ((query.page_size & 1) && PAGE_SHIFT >= 12) { page_shift = 12; /* 4kB */ } else { dev_dbg(&dev->dev, "no supported direct page size in mask %x", query.page_size); goto out_failed; } + /* verify the window * number of ptes will map the partition */ /* check largest block * page size > max memory hotplug addr */ max_addr = ddw_memory_hotplug_max();
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, Having System pagesize < IOMMU pagesize may cause a page owned by another process/VM to be written by a buggy driver / device. As it's intended to use DDW for indirect mapping, it's possible to get a configuration where (PAGE_SIZE = 4k) < (IOMMU_PAGE_SIZE() = 64k). To avoid this, make sure create_ddw() can only use pagesize <= PAGE_SIZE. Signed-off-by: Leonardo Bras <leobras.c@gmail.com> --- arch/powerpc/platforms/pseries/iommu.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)