[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 1/2] physmem: Bail out qemu_ram_block_from_host() for inva
From: |
Alex Bennée |
Subject: |
Re: [PATCH v1 1/2] physmem: Bail out qemu_ram_block_from_host() for invalid ram addrs |
Date: |
Thu, 04 Jul 2024 13:33:05 +0100 |
"Edgar E. Iglesias" <edgar.iglesias@gmail.com> writes:
> On Thu, Jul 4, 2024 at 1:26 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> "Edgar E. Iglesias" <edgar.iglesias@gmail.com> writes:
>
> > From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
> >
> > Bail out in qemu_ram_block_from_host() when
> > xen_ram_addr_from_mapcache() does not find an existing
> > mapping.
> >
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> > system/physmem.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/system/physmem.c b/system/physmem.c
> > index 33d09f7571..59d1576c2b 100644
> > --- a/system/physmem.c
> > +++ b/system/physmem.c
> > @@ -2277,6 +2277,10 @@ RAMBlock *qemu_ram_block_from_host(void *ptr, bool
> round_offset,
> > ram_addr_t ram_addr;
> > RCU_READ_LOCK_GUARD();
> > ram_addr = xen_ram_addr_from_mapcache(ptr);
> > + if (ram_addr == RAM_ADDR_INVALID) {
> > + return NULL;
> > + }
> > +
>
> Isn't this indicative of a failure? Should there at least be a trace
> point for failed mappings?
>
> Yes but there are already trace points for the failure cases inside
> xen_ram_addr_from_mapcache().
> Do those address your concerns or do you think we need additional
> trace points?
Ahh that will do.
I am curious for the reasons why we might not have an entry in the
mapcache. I guess the trace_xen_map_cache() covers all insertions into
the cache although you need to check trace_xen_map_cache_return() to see
if anything failed.
Anyway:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v1 2/2] xen: mapcache: Fix unmapping of first entries in buckets, Stefano Stabellini, 2024/07/08