On Fri, Mar 01, 2024 at 05:04:54PM +0000, Alex Bennée wrote:
> Vikram Garhwal <vikram.garh...@amd.com> writes:
>
> > From: Juergen Gross <jgr...@suse.com>
> >
> > qemu_map_ram_ptr() and qemu_ram_ptr_length() share quite some code, so
> > modify qemu_ram_ptr_length() a little bit and use it for
> > qemu_map_ram_ptr(), too.
> >
> > Signed-off-by: Juergen Gross <jgr...@suse.com>
> > Signed-off-by: Vikram Garhwal <vikram.garh...@amd.com>
> > Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
> > ---
> > system/physmem.c | 56 ++++++++++++++++++++----------------------------
> > 1 file changed, 23 insertions(+), 33 deletions(-)
> >
> <snip>
> > -
> > -/* Return a host pointer to guest's ram. Similar to qemu_map_ram_ptr
> > - * but takes a size argument.
> > +/*
> > + * Return a host pointer to guest's ram.
> > *
> > * Called within RCU critical section.
> > */
>
> If you end up re-spinning it would be nice to kdoc this function and at
> least call out size as a return by ref and optional.
Will do if re-spinning is needed.
>
> > static void *qemu_ram_ptr_length(RAMBlock *block, ram_addr_t addr,
> > hwaddr *size, bool lock)
> > {
> > - if (*size == 0) {
> > + hwaddr len = 0;
> > +
> > + if (size && *size == 0) {
> > return NULL;
> > }
> >
> > @@ -2207,7 +2181,10 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> > ram_addr_t addr,
> > block = qemu_get_ram_block(addr);
> > addr -= block->offset;
> > }
> > - *size = MIN(*size, block->max_length - addr);
> > + if (size) {
> > + *size = MIN(*size, block->max_length - addr);
> > + len = *size;
> > + }
> >
> > if (xen_enabled() && block->host == NULL) {
> > /* We need to check if the requested address is in the RAM
> > @@ -2215,7 +2192,7 @@ static void *qemu_ram_ptr_length(RAMBlock *block,
> > ram_addr_t addr,
> > * In that case just map the requested area.
> > */
> > if (block->offset == 0) {
> > - return xen_map_cache(addr, *size, lock, lock);
> > + return xen_map_cache(addr, len, lock, lock);
>
> I did wonder if len == 0 will confuse things but it seems xen_map_cache
> will default to XC_PAGE_SIZE in that case.
>
> Anyway:
>
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
>
> --
> Alex Bennée
> Virtualisation Tech Lead @ Linaro
