| Message ID | 1561354383-21322-1-git-send-email-yan.y.zhao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | memory: warning on out of scope notification | expand |
Hi Yan, On 6/24/19 7:33 AM, Yan Zhao wrote: > if an entry has parts out of scope of notifier's range, print warning > message. > > Out of scope mapping/unmapping would cause problem, as in below case: > > 1. initially there are two notifiers with ranges > 0-0xfedfffff, 0xfef00000-0xffffffffffffffff, > IOVAs from 0x3c000000 - 0x3c1fffff is in shadow page table. > > 2. in vfio, memory_region_register_iommu_notifier() is followed by > memory_region_iommu_replay(), which will first call address space > unmap, > and walk and add back all entries in vtd shadow page table. e.g. > (1) for notifier 0-0xfedfffff, > IOVAs from 0 - 0xffffffff get unmapped, > and IOVAs from 0x3c000000 - 0x3c1fffff get mapped > (2) for notifier 0xfef00000-0xffffffffffffffff > IOVAs from 0 - 0x7fffffffff get unmapped, > but IOVAs from 0x3c000000 - 0x3c1fffff cannot get mapped back. > > Cc: Auger Eric <eric.auger@redhat.com> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> > --- > memory.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/memory.c b/memory.c > index 0a089a7..18927f2 100644 > --- a/memory.c > +++ b/memory.c > @@ -1953,6 +1953,16 @@ void memory_region_notify_one(IOMMUNotifier *notifier, > request_flags = IOMMU_NOTIFIER_UNMAP; > } I think introducing a local variable for entry end would be worth now > > + if (entry->iova < notifier->start || > + entry->iova + entry->addr_mask > notifier->end) { > + warn_report("%s IOMMUTLBEntry %lx-%lx outside of " > + "notifier scope %lx-%lx", Also I think you should use PRIx64 instead. Thanks Eric > + (request_flags == IOMMU_NOTIFIER_MAP) ? > + "Mapping" : "Unmapping", > + entry->iova, entry->iova + entry->addr_mask, > + notifier->start, notifier->end); > + } > + > if (notifier->notifier_flags & request_flags) { > notifier->notify(notifier, entry); > } >
On Mon, Jun 24, 2019 at 03:09:24PM +0800, Auger Eric wrote: > Hi Yan, > > On 6/24/19 7:33 AM, Yan Zhao wrote: > > if an entry has parts out of scope of notifier's range, print warning > > message. > > > > Out of scope mapping/unmapping would cause problem, as in below case: > > > > 1. initially there are two notifiers with ranges > > 0-0xfedfffff, 0xfef00000-0xffffffffffffffff, > > IOVAs from 0x3c000000 - 0x3c1fffff is in shadow page table. > > > > 2. in vfio, memory_region_register_iommu_notifier() is followed by > > memory_region_iommu_replay(), which will first call address space > > unmap, > > and walk and add back all entries in vtd shadow page table. e.g. > > (1) for notifier 0-0xfedfffff, > > IOVAs from 0 - 0xffffffff get unmapped, > > and IOVAs from 0x3c000000 - 0x3c1fffff get mapped > > (2) for notifier 0xfef00000-0xffffffffffffffff > > IOVAs from 0 - 0x7fffffffff get unmapped, > > but IOVAs from 0x3c000000 - 0x3c1fffff cannot get mapped back. > > > > Cc: Auger Eric <eric.auger@redhat.com> > > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> > > --- > > memory.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/memory.c b/memory.c > > index 0a089a7..18927f2 100644 > > --- a/memory.c > > +++ b/memory.c > > @@ -1953,6 +1953,16 @@ void memory_region_notify_one(IOMMUNotifier *notifier, > > request_flags = IOMMU_NOTIFIER_UNMAP; > > } > I think introducing a local variable for entry end would be worth now > > > > + if (entry->iova < notifier->start || > > + entry->iova + entry->addr_mask > notifier->end) { > > + warn_report("%s IOMMUTLBEntry %lx-%lx outside of " > > + "notifier scope %lx-%lx", > Also I think you should use PRIx64 instead. > Got it! Thanks :) > Thanks > > Eric > > + (request_flags == IOMMU_NOTIFIER_MAP) ? > > + "Mapping" : "Unmapping", > > + entry->iova, entry->iova + entry->addr_mask, > > + notifier->start, notifier->end); > > + } > > + > > if (notifier->notifier_flags & request_flags) { > > notifier->notify(notifier, entry); > > } > >
diff --git a/memory.c b/memory.c index 0a089a7..18927f2 100644 --- a/memory.c +++ b/memory.c @@ -1953,6 +1953,16 @@ void memory_region_notify_one(IOMMUNotifier *notifier, request_flags = IOMMU_NOTIFIER_UNMAP; } + if (entry->iova < notifier->start || + entry->iova + entry->addr_mask > notifier->end) { + warn_report("%s IOMMUTLBEntry %lx-%lx outside of " + "notifier scope %lx-%lx", + (request_flags == IOMMU_NOTIFIER_MAP) ? + "Mapping" : "Unmapping", + entry->iova, entry->iova + entry->addr_mask, + notifier->start, notifier->end); + } + if (notifier->notifier_flags & request_flags) { notifier->notify(notifier, entry); }
if an entry has parts out of scope of notifier's range, print warning message. Out of scope mapping/unmapping would cause problem, as in below case: 1. initially there are two notifiers with ranges 0-0xfedfffff, 0xfef00000-0xffffffffffffffff, IOVAs from 0x3c000000 - 0x3c1fffff is in shadow page table. 2. in vfio, memory_region_register_iommu_notifier() is followed by memory_region_iommu_replay(), which will first call address space unmap, and walk and add back all entries in vtd shadow page table. e.g. (1) for notifier 0-0xfedfffff, IOVAs from 0 - 0xffffffff get unmapped, and IOVAs from 0x3c000000 - 0x3c1fffff get mapped (2) for notifier 0xfef00000-0xffffffffffffffff IOVAs from 0 - 0x7fffffffff get unmapped, but IOVAs from 0x3c000000 - 0x3c1fffff cannot get mapped back. Cc: Auger Eric <eric.auger@redhat.com> Signed-off-by: Yan Zhao <yan.y.zhao@intel.com> --- memory.c | 10 ++++++++++ 1 file changed, 10 insertions(+)