Re: [PATCH v4 14/17] xen: Add xen_mr_is_memory()

[Edgar E. Iglesias]

On Mon, May 6, 2024 at 11:59 AM Philippe Mathieu-Daudé
<phi...@linaro.org> wrote:
>
> On 2/5/24 09:26, David Hildenbrand wrote:
> > On 30.04.24 18:49, Edgar E. Iglesias wrote:
> >> From: "Edgar E. Iglesias" <edgar.igles...@amd.com>
> >>
> >> Add xen_mr_is_memory() to abstract away tests for the
> >> xen_memory MR.
> >>
> >> Signed-off-by: Edgar E. Iglesias <edgar.igles...@amd.com>
> >> ---
> >
> > [...]
> >
> >>   #endif
> >> diff --git a/system/physmem.c b/system/physmem.c
> >> index ad7a8c7d95..1a5ffcba2a 100644
> >> --- a/system/physmem.c
> >> +++ b/system/physmem.c
> >> @@ -2227,7 +2227,7 @@ static void *qemu_ram_ptr_length(RAMBlock
> >> *block, ram_addr_t addr,
> >>            * because we don't want to map the entire memory in QEMU.
> >>            * In that case just map the requested area.
> >>            */
> >> -        if (block->offset == 0) {
> >> +        if (xen_mr_is_memory(block->mr)) {
> >>               return xen_map_cache(block->mr, addr, len, lock, lock,
> >>                                    is_write);
> >>           }
> >
> > I'd have moved that into a separate patch, because this is not a simple
> > abstraction here.
>
> Yes please, maybe using Stefano review comment in the description.
>

Thanks, for v5 I've split out this particular change into a separate patch:

    softmmu: Replace check for RAMBlock offset 0 with xen_mr_is_memory

    For xen, when checking for the first RAM (xen_memory), use
    xen_mr_is_memory() rather than checking for a RAMBlock with
    offset 0.

    All Xen machines create xen_memory first so this has no
    functional change for existing machines.

    Signed-off-by: Edgar E. Iglesias <edgar.igles...@amd.com>

diff --git a/system/physmem.c b/system/physmem.c
index ad7a8c7d95..1a5ffcba2a 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -2227,7 +2227,7 @@ static void *qemu_ram_ptr_length(RAMBlock
*block, ram_addr_t addr,
          * because we don't want to map the entire memory in QEMU.
          * In that case just map the requested area.
          */
-        if (block->offset == 0) {
+        if (xen_mr_is_memory(block->mr)) {
             return xen_map_cache(block->mr, addr, len, lock, lock,
                                  is_write);
         }



> >
> > Acked-by: David Hildenbrand <da...@redhat.com>
> >
>