Hi Will, On 2020/12/7 18:59, Will Deacon wrote: > On Sat, Dec 05, 2020 at 04:29:57PM +0800, Keqian Zhu wrote: >> ... then we have more chance to detect wrong code logic. > > This could do with being a bit more explicit. Something like: > > Although handling a mapping request with no permissions is a > trivial no-op, defer the early return until after the size/range > checks so that we are consistent with other mapping requests. This looks well, thanks.
> >> Signed-off-by: Keqian Zhu <zhukeqi...@huawei.com> >> --- >> drivers/iommu/io-pgtable-arm.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c >> index a7a9bc08dcd1..8ade72adab31 100644 >> --- a/drivers/iommu/io-pgtable-arm.c >> +++ b/drivers/iommu/io-pgtable-arm.c >> @@ -444,10 +444,6 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, >> unsigned long iova, >> arm_lpae_iopte prot; >> long iaext = (s64)iova >> cfg->ias; >> >> - /* If no access, then nothing to do */ >> - if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE))) >> - return 0; >> - >> if (WARN_ON(!size || (size & cfg->pgsize_bitmap) != size)) >> return -EINVAL; >> >> @@ -456,6 +452,10 @@ static int arm_lpae_map(struct io_pgtable_ops *ops, >> unsigned long iova, >> if (WARN_ON(iaext || paddr >> cfg->oas)) >> return -ERANGE; >> >> + /* If no access, then nothing to do */ >> + if (!(iommu_prot & (IOMMU_READ | IOMMU_WRITE))) >> + return 0; > > This looks sensible to me, but please can you make the same change for > io-pgtable-arm-v7s.c so that the behaviour is consistent across the two > formats? > OK. I can do it right now. Thanks, Keqian > Thanks, > > Will > . >
