* [PATCH v1] memory: remove assert to avoid unnecessary coredump
@ 2020-03-03 3:11 Yi Sun
2020-03-03 3:36 ` Yan Zhao
0 siblings, 1 reply; 4+ messages in thread
From: Yi Sun @ 2020-03-03 3:11 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, yan.y.zhao, Yi Sun
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
Skip this notification but not coredump makes everything
work well.
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v1] memory: remove assert to avoid unnecessary coredump
2020-03-03 3:11 [PATCH v1] memory: remove assert to avoid unnecessary coredump Yi Sun
@ 2020-03-03 3:36 ` Yan Zhao
2020-03-03 5:22 ` Yi Sun
0 siblings, 1 reply; 4+ messages in thread
From: Yan Zhao @ 2020-03-03 3:36 UTC (permalink / raw)
To: Yi Sun; +Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
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?
Thanks
Yan
> Skip this notification but not coredump makes everything
> work well.
>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] memory: remove assert to avoid unnecessary coredump
2020-03-03 3:36 ` Yan Zhao
@ 2020-03-03 5:22 ` Yi Sun
2020-03-03 5:30 ` Yan Zhao
0 siblings, 1 reply; 4+ messages in thread
From: Yi Sun @ 2020-03-03 5:22 UTC (permalink / raw)
To: Yan Zhao; +Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
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 <yi.y.sun@linux.intel.com>
> > ---
> > 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
> >
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] memory: remove assert to avoid unnecessary coredump
2020-03-03 5:22 ` Yi Sun
@ 2020-03-03 5:30 ` Yan Zhao
0 siblings, 0 replies; 4+ messages in thread
From: Yan Zhao @ 2020-03-03 5:30 UTC (permalink / raw)
To: Yi Sun; +Cc: pbonzini@redhat.com, qemu-devel@nongnu.org
On Tue, Mar 03, 2020 at 01:22:26PM +0800, Yi Sun wrote:
> 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?
>
no. please refer to the link below.
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04218.html
Thanks
Yan
> > Thanks
> > Yan
> >
> > > Skip this notification but not coredump makes everything
> > > work well.
> > >
> > > Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> > > ---
> > > 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
> > >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-03 5:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-03 3:11 [PATCH v1] memory: remove assert to avoid unnecessary coredump Yi Sun
2020-03-03 3:36 ` Yan Zhao
2020-03-03 5:22 ` Yi Sun
2020-03-03 5:30 ` Yan Zhao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.