Re: [PATCH v1] memory: remove assert to avoid unnecessary coredump

[Yi Sun]

On 20-03-02 22:36:39, Yan Zhao wrote:
> On Tue, Mar 03, 2020 at 11:11:14AM +0800, Yi Sun wrote:
> > It is too strict to use assert to make qemu coredump if
> > the notification does not overlap with registered range.
> > Skip it is fine enough.
> > 
> > During test, we found such a case for vhost net device:
> >     memory_region_notify_one: entry->iova=0xfee00000, entry_end=0xfeffffff, 
> > notifier->start=0xfef00000, notifier->end=0xffffffffffffffff
> >
> so for range from 0xfef00000 to 0xfefffff,  would notification for this
> range get lost?
> 
Yes, that is an issue although there is no any problem found during test
with this fix.

I think we should notify the intersection between entry and notifier. How
do you think?

> Thanks
> Yan
> 
> > Skip this notification but not coredump makes everything
> > work well.
> > 
> > Signed-off-by: Yi Sun <address@hidden>
> > ---
> >  memory.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/memory.c b/memory.c
> > index 06484c2bff..62ad0f3377 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1921,12 +1921,11 @@ void memory_region_notify_one(IOMMUNotifier 
> > *notifier,
> >       * Skip the notification if the notification does not overlap
> >       * with registered range.
> >       */
> > -    if (notifier->start > entry_end || notifier->end < entry->iova) {
> > +    if (notifier->start > entry_end || notifier->end < entry->iova ||
> > +        entry->iova < notifier->start || entry_end > notifier->end) {
> >          return;
> >      }
> >  
> > -    assert(entry->iova >= notifier->start && entry_end <= notifier->end);
> > -
> >      if (entry->perm & IOMMU_RW) {
> >          request_flags = IOMMU_NOTIFIER_MAP;
> >      } else {
> > -- 
> > 2.15.1
> >