Re: [PATCH v8 7/9] xen/arm: create shared memory nodes in guest device tree

[Julien Grall <julien@xxxxxxx>]

Hi,

On 09/09/2022 08:45, Bertrand Marquis wrote:
> > It should be:
> >
> > /*
> > * TODO:
> > *
> >
> > I think this is good to go. The two minor style issues could be fixed on
> > commit. I haven't committed to give Julien & Bertrand another chance to
> > have a look.
> I think that it is ok to fix those on commit and I am ok with the rest so:
> Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
This series doesn't build without !CONFIG_STATIC_SHM:

  UPD     include/xen/compile.h
 Xen 4.17-unstable
make[1]: Nothing to be done for `include'.
make[1]: `arch/arm/include/asm/asm-offsets.h' is up to date.
  CC      common/version.o
  LD      common/built_in.o
  CC      arch/arm/domain_build.o
arch/arm/domain_build.c: In function ‘make_shm_memory_node’:
arch/arm/domain_build.c:1445:1: error: no return statement in function 
returning non-void [-Werror=return-type]
 }
 ^
cc1: all warnings being treated as errors
make[2]: *** [arch/arm/domain_build.o] Error 1
make[1]: *** [arch/arm] Error 2
make: *** [xen] Error 2

This is because...

> > > +         * - xen,offset: (borrower VMs only)
> > > +         *   64 bit integer offset within the owner virtual machine's shared
> > > +         *   memory region used for the mapping in the borrower VM
> > > +         */
> > > +        res = fdt_property_u64(fdt, "xen,offset", 0);
> > > +        if ( res )
> > > +            return res;
> > > +
> > > +        res = fdt_end_node(fdt);
> > > +        if ( res )
> > > +            return res;
> > > +    }
> > > +
> > > +    return res;
> > > +}
> > > +#else
> > > +static int __init make_shm_memory_node(const struct domain *d,
> > > +                                       void *fdt,
> > > +                                       int addrcells, int sizecells,
> > > +                                       const struct meminfo *mem)
> > > +{
> > > +    ASSERT_UNREACHABLE();
... there is a missing 'return -ENOTSUPP' here. While this is simple 
enough to fix, this indicates to me that this version was not tested 
with !CONFIG_STATIC_SHM.
As this is the default option, I will not commit until I get 
confirmation that some smoke was done.
Cheers,

--
Julien Grall