| Message ID | 20240227223501.28475-5-vikram.garhwal@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
Vikram Garhwal <vikram.garhwal@amd.com> writes: > From: Juergen Gross <jgross@suse.com> > > Today xen_ram_addr_from_mapcache() will either abort() or return 0 in > case it can't find a matching entry for a pointer value. Both cases > are bad, so change that to return an invalid address instead. > > Signed-off-by: Juergen Gross <jgross@suse.com> > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > hw/xen/xen-mapcache.c | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c > index dfc412d138..179b7e95b2 100644 > --- a/hw/xen/xen-mapcache.c > +++ b/hw/xen/xen-mapcache.c > @@ -396,13 +396,8 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) > } > } > if (!found) { > - trace_xen_ram_addr_from_mapcache_not_found(ptr); > - QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { > - trace_xen_ram_addr_from_mapcache_found(reventry->paddr_index, > - reventry->vaddr_req); > - } If these tracepoints aren't useful they need removing from trace-events. However I suspect it would be better to keep them in as they are fairly cheap. Otherwise: Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
On Fri, Mar 1, 2024 at 6:08 PM Alex Bennée <alex.bennee@linaro.org> wrote: > Vikram Garhwal <vikram.garhwal@amd.com> writes: > > > From: Juergen Gross <jgross@suse.com> > > > > Today xen_ram_addr_from_mapcache() will either abort() or return 0 in > > case it can't find a matching entry for a pointer value. Both cases > > are bad, so change that to return an invalid address instead. > > > > Signed-off-by: Juergen Gross <jgross@suse.com> > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > > --- > > hw/xen/xen-mapcache.c | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c > > index dfc412d138..179b7e95b2 100644 > > --- a/hw/xen/xen-mapcache.c > > +++ b/hw/xen/xen-mapcache.c > > @@ -396,13 +396,8 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) > > } > > } > > if (!found) { > > - trace_xen_ram_addr_from_mapcache_not_found(ptr); > > - QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { > > - > trace_xen_ram_addr_from_mapcache_found(reventry->paddr_index, > > - reventry->vaddr_req); > > - } > > If these tracepoints aren't useful they need removing from trace-events. > However I suspect it would be better to keep them in as they are fairly > cheap. > > Otherwise: > > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c index dfc412d138..179b7e95b2 100644 --- a/hw/xen/xen-mapcache.c +++ b/hw/xen/xen-mapcache.c @@ -396,13 +396,8 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) } } if (!found) { - trace_xen_ram_addr_from_mapcache_not_found(ptr); - QTAILQ_FOREACH(reventry, &mapcache->locked_entries, next) { - trace_xen_ram_addr_from_mapcache_found(reventry->paddr_index, - reventry->vaddr_req); - } - abort(); - return 0; + mapcache_unlock(); + return RAM_ADDR_INVALID; } entry = &mapcache->entry[paddr_index % mapcache->nr_buckets]; @@ -411,7 +406,7 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr) } if (!entry) { trace_xen_ram_addr_from_mapcache_not_in_cache(ptr); - raddr = 0; + raddr = RAM_ADDR_INVALID; } else { raddr = (reventry->paddr_index << MCACHE_BUCKET_SHIFT) + ((unsigned long) ptr - (unsigned long) entry->vaddr_base);