Re: [RFC v8 5/5] memory: Skip bad range assertion if notifier is DEVIOTLB type

[David Gibson]

On Wed, Sep 02, 2020 at 04:24:50PM +0200, Auger Eric wrote:
> Hi Eugenio,
> 
> On 9/1/20 4:26 PM, Eugenio Pérez wrote:
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> Please could you explain in the commit message why you need to remove
> the assert()? I know you described the assert() in the cover letter but
> the commit msg is the one that remains.

Right... neither in the cover letter nor the individual patches,
AFAICT, does anything actually explain why that assert() could be
hit.  Nor does it connect the dots from an assert() hitting to adding
infrastructure for a new event type.

> > ---
> >  softmmu/memory.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/softmmu/memory.c b/softmmu/memory.c
> > index 09b3443eac..3ee99b4dc0 100644
> > --- a/softmmu/memory.c
> > +++ b/softmmu/memory.c
> > @@ -1895,6 +1895,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier 
> > *notifier,
> >  {
> >      IOMMUTLBEntry *entry = &event->entry;
> >      hwaddr entry_end = entry->iova + entry->addr_mask;
> > +    IOMMUTLBEntry tmp = *entry;
> >  
> >      /*
> >       * Skip the notification if the notification does not overlap
> > @@ -1904,10 +1905,18 @@ void memory_region_notify_iommu_one(IOMMUNotifier 
> > *notifier,
> >          return;
> >      }
> >  
> > -    assert(entry->iova >= notifier->start && entry_end <= notifier->end);
> > +    if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB) {
> > +        /* Crop (iova, addr_mask) to range */
> > +        tmp.iova = MAX(tmp.iova, notifier->start);
> > +        tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova;
> > +        /* Confirm no underflow */
> > +        assert(MIN(entry_end, notifier->end) >= tmp.iova);
> > +    } else {
> > +        assert(entry->iova >= notifier->start && entry_end <= 
> > notifier->end);
> > +    }
> >  
> >      if (event->type & notifier->notifier_flags) {
> > -        notifier->notify(notifier, entry);
> > +        notifier->notify(notifier, &tmp);
> >      }
> >  }
> >  
> > 
> Thanks
> 
> Eric
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature