From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Do a check for already installed leaf entry at the current level before
performing any actions when trying to map.
This check is already present in arm_lpae_init_pte(), i.e. before
installing new leaf entry at the current level if conditions to do so
are met (size == block_size).
But, this might be insufficient in case when we have already
installed block mapping at this level and it is not time to
install new leaf entry (size != block_size).
In that case we continue walking the page table down with wrong pointer
to the next level.
So, move check from arm_lpae_init_pte() to __arm_lpae_map() in order to
avoid all cases.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
I hope that following actions can help to catch it:
1. Call iommu_map for a block mapping (e.g. 2M) at some address
(e.g. iova 0x80000000 pa 0x80000000).
2. Call iommu_map for a page mapping (4k) at some address from
the previous mapped region (e.g. iova 0x80008000 pa 0x90000000).
I understand that after iommu_map should be iommu_unmap, but
different scenarios may occur).
---
---
drivers/iommu/io-pgtable-arm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
@@ -272,11 +272,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,arm_lpae_ioptepte=prot;structio_pgtable_cfg*cfg=&data->iop.cfg;-if(iopte_leaf(*ptep,lvl)){-/* We require an unmap first */-WARN_ON(!selftest_running);-return-EEXIST;-}elseif(iopte_type(*ptep,lvl)==ARM_LPAE_PTE_TYPE_TABLE){+if(iopte_type(*ptep,lvl)==ARM_LPAE_PTE_TYPE_TABLE){/**Weneedtounmapandfreetheoldtablebefore*overwritingitwithablockentry.
@@ -315,6 +311,13 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,/* Find our entry at the current level */ptep+=ARM_LPAE_LVL_IDX(iova,lvl,data);+/* Check for already installed leaf entry */+if(iopte_leaf(*ptep,lvl)){+/* We require an unmap first */+WARN_ON(!selftest_running);+return-EEXIST;+}+/* If we can install a leaf entry at this level, then do so */if(size==block_size&&(size&cfg->pgsize_bitmap))returnarm_lpae_init_pte(data,iova,paddr,prot,lvl,ptep);
Hi, all.
Any comments?
On Thu, Feb 9, 2017 at 3:56 PM, Oleksandr Tyshchenko
[off-list ref] wrote:
quoted hunk
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Do a check for already installed leaf entry at the current level before
performing any actions when trying to map.
This check is already present in arm_lpae_init_pte(), i.e. before
installing new leaf entry at the current level if conditions to do so
are met (size == block_size).
But, this might be insufficient in case when we have already
installed block mapping at this level and it is not time to
install new leaf entry (size != block_size).
In that case we continue walking the page table down with wrong pointer
to the next level.
So, move check from arm_lpae_init_pte() to __arm_lpae_map() in order to
avoid all cases.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
I hope that following actions can help to catch it:
1. Call iommu_map for a block mapping (e.g. 2M) at some address
(e.g. iova 0x80000000 pa 0x80000000).
2. Call iommu_map for a page mapping (4k) at some address from
the previous mapped region (e.g. iova 0x80008000 pa 0x90000000).
I understand that after iommu_map should be iommu_unmap, but
different scenarios may occur).
---
---
drivers/iommu/io-pgtable-arm.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
@@ -272,11 +272,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,arm_lpae_ioptepte=prot;structio_pgtable_cfg*cfg=&data->iop.cfg;-if(iopte_leaf(*ptep,lvl)){-/* We require an unmap first */-WARN_ON(!selftest_running);-return-EEXIST;-}elseif(iopte_type(*ptep,lvl)==ARM_LPAE_PTE_TYPE_TABLE){+if(iopte_type(*ptep,lvl)==ARM_LPAE_PTE_TYPE_TABLE){/**Weneedtounmapandfreetheoldtablebefore*overwritingitwithablockentry.
@@ -315,6 +311,13 @@ static int __arm_lpae_map(struct arm_lpae_io_pgtable *data, unsigned long iova,/* Find our entry at the current level */ptep+=ARM_LPAE_LVL_IDX(iova,lvl,data);+/* Check for already installed leaf entry */+if(iopte_leaf(*ptep,lvl)){+/* We require an unmap first */+WARN_ON(!selftest_running);+return-EEXIST;+}+/* If we can install a leaf entry at this level, then do so */if(size==block_size&&(size&cfg->pgsize_bitmap))returnarm_lpae_init_pte(data,iova,paddr,prot,lvl,ptep);--
From: Will Deacon <hidden> Date: 2017-02-13 11:27:28
On Mon, Feb 13, 2017 at 01:07:02PM +0200, Oleksandr Tyshchenko wrote:
Any comments?
Looks fine to me, but I don't think it's urgent and I already sent my
SMMU pull for 4.11. I'll send this as a fix after the merge window.
I suspect we need something similar for io-pgtable-arm-v7s.c, too.
Will
On Mon, Feb 13, 2017 at 1:27 PM, Will Deacon [off-list ref] wrote:
On Mon, Feb 13, 2017 at 01:07:02PM +0200, Oleksandr Tyshchenko wrote:
quoted
Any comments?
Looks fine to me, but I don't think it's urgent and I already sent my
SMMU pull for 4.11. I'll send this as a fix after the merge window.
OK. Thank you.
I suspect we need something similar for io-pgtable-arm-v7s.c, too.
Agree. On the whole I will be able to make similar patch for arm-v7s,
but I won't be 100% sure
since I don't have any boards where arm-v7s compatible IOMMU installed.
Shall I make patch for arm-v7s too?
From: Will Deacon <hidden> Date: 2017-02-13 12:00:21
On Mon, Feb 13, 2017 at 01:50:29PM +0200, Oleksandr Tyshchenko wrote:
On Mon, Feb 13, 2017 at 1:27 PM, Will Deacon [off-list ref] wrote:
quoted
On Mon, Feb 13, 2017 at 01:07:02PM +0200, Oleksandr Tyshchenko wrote:
quoted
Any comments?
Looks fine to me, but I don't think it's urgent and I already sent my
SMMU pull for 4.11. I'll send this as a fix after the merge window.
OK. Thank you.
quoted
I suspect we need something similar for io-pgtable-arm-v7s.c, too.
Agree. On the whole I will be able to make similar patch for arm-v7s,
but I won't be 100% sure
since I don't have any boards where arm-v7s compatible IOMMU installed.
Shall I make patch for arm-v7s too?
Yes, please. Robin seems to enjoy using short-descriptor, so he might
give it a spin for you if you ask nicely.
Will