[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v8 5/5] memory: Skip bad range assertion if notifier is DEVIOTL
From: |
Eugenio Perez Martin |
Subject: |
Re: [RFC v8 5/5] memory: Skip bad range assertion if notifier is DEVIOTLB type |
Date: |
Thu, 3 Sep 2020 19:05:11 +0200 |
On Thu, Sep 3, 2020 at 2:05 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>
> 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.
>
Hi!
You are right. I think I've made it clearer in the new patch sent (now
as patch instead of RFC).
Please let me know if you think further explanations are needed.
Thanks!
> > > ---
> > > 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