Mailing List Archive: [QEMU][PATCH v3 4/7] xen: let xen_ram_addr_from_mapcache() return -1 in case of not found entry

[QEMU][PATCH v3 4/7] xen: let xen_ram_addr_from_mapcache() return -1 in case of not found entry

Feb 27, 2024, 2:34 PM

Post #1 of 3 (30 views)

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);
- }
- 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);
--
2.17.1

Re: [QEMU][PATCH v3 4/7] xen: let xen_ram_addr_from_mapcache() return -1 in case of not found entry [ In reply to ]

alex.bennee at linaro

Mar 1, 2024, 9:08 AM

Post #2 of 3 (25 views)

Permalink

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>

--
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [QEMU][PATCH v3 4/7] xen: let xen_ram_addr_from_mapcache() return -1 in case of not found entry [ In reply to ]

edgar.iglesias at gmail

Apr 10, 2024, 4:14 AM

Post #3 of 3 (14 views)

Permalink

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>