[cc +iommu] On Tue, 11 Jun 2019 20:40:41 +0800 Jiangyiwen <jiangyiwen@xxxxxxxxxx> wrote: > Hi Alex, > > I found this problem is not very easy to solve, for > now, in arm64 platform, the "0" physical address > is a valid system memory address, so in function > arm_smmu_iova_to_phys() I think it should not use > "0" as abnormal return value. > > Do you have any idea? I think you're going to need to redefine iommu_iova_to_phys() and fix all the IOMMU implementations of it to comply. Currently AMD and Intel IOMMU driver return zero if a mapping is not found. You could make the function return 0/errno and return the physical address via a pointer arg. You could also keep the existing definition, but introduce a test for a valid result that might use an architecture specific value (akin to IS_ERR()). You could also just reserve the zero page from userspace allocation. I really don't want #ifdef in the vfio iommu driver trying to discern the correct invalid value though. Thanks, Alex > On 2019/6/11 11:21, jiangyiwen wrote: > > On 2019/5/21 3:28, Alex Williamson wrote: > >> On Mon, 20 May 2019 15:50:11 +0800 > >> jiangyiwen <jiangyiwen@xxxxxxxxxx> wrote: > >> > >>> Hello alex, > >>> > >>> We test a call trace as follows use ARM64 architecture, > >>> it prints a WARN_ON() when find not physical address by > >>> iova in vfio_unmap_unpin(), I can't find the cause of > >>> problem now, do you have any ideas? > >> Is it reproducible? Can you explain how to reproduce it? The stack > >> trace indicates a KVM VM is being shutdown and we're trying to clean > >> out the IOMMU mappings from the domain and find a page that we think > >> should be mapped that the IOMMU doesn't have mapped. What device(s) was > >> assigned to the VM? This could be an IOMMU driver bug or a > >> vfio_iommu_type1 bug. Have you been able to reproduce this on other > >> platforms? > >> > > Hello Alex, > > > > Sorry to reply you so late because of some things, > > this problem's reason is in some platform (like ARM64), > > the "0" physical address is valid and can be used for > > system memory, so in this case it should not print a > > WARN_ON() and continue, we should unmap and unpin this > > "0" physical address in these platform. > > > > So I want to return FFFFFFFFFFFFFFFFL instead of "0" as invalid > > physical address in function iommu_iova_to_phys(). Do you think > > it's appropriate? > > > > Thanks, > > Yiwen. > > > >>> In addition, I want to know why there is a WARN_ON() instead > >>> of BUG_ON()? Does it affect the follow-up process? > >> We're removing an IOMMU page mapping entry and find that it's not > >> present, so ultimately the effect at the IOMMU is the same, there's no > >> mapping at that address, but I can't say without further analysis > >> whether that means a page remains pinned or if that inconsistency was > >> resolved previously elsewhere. We WARN_ON because this is not what we > >> expect, but potentially leaking a page of memory doesn't seem worthy of > >> crashing the host, nor would a crash dump at that point necessarily aid > >> in resolving the missing page as it potentially occurred well in the > >> past. Thanks, > >> > >> Alex > >> > >> . > >> > > > > > > . > > >
