Re: [PATCH v5 1/8] xen/arm: introduce static shared memory

[Julien Grall <julien@xxxxxxx>]

On 29/06/2022 06:38, Penny Zheng wrote:
> Hi Julien
Hi Penny,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Sent: Saturday, June 25, 2022 1:55 AM
> > To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Wei Chen <Wei.Chen@xxxxxxx>; Stefano Stabellini
> > <sstabellini@xxxxxxxxxx>; Bertrand Marquis <Bertrand.Marquis@xxxxxxx>;
> > Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> > Subject: Re: [PATCH v5 1/8] xen/arm: introduce static shared memory
> >
> > Hi Penny,
> >
> > On 20/06/2022 06:11, Penny Zheng wrote:
> > > From: Penny Zheng <penny.zheng@xxxxxxx>
> > >
> > > This patch serie introduces a new feature: setting up static
> > Typo: s/serie/series/
> >
> > > shared memory on a dom0less system, through device tree configuration.
> > >
> > > This commit parses shared memory node at boot-time, and reserve it in
> > > bootinfo.reserved_mem to avoid other use.
> > >
> > > This commits proposes a new Kconfig CONFIG_STATIC_SHM to wrap
> > > static-shm-related codes, and this option depends on static memory(
> > > CONFIG_STATIC_MEMORY). That's because that later we want to reuse a
> > > few helpers, guarded with CONFIG_STATIC_MEMORY, like
> > > acquire_staticmem_pages, etc, on static shared memory.
> > >
> > > Signed-off-by: Penny Zheng <penny.zheng@xxxxxxx>
> > > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > ---
> > > v5 change:
> > > - no change
> > > ---
> > > v4 change:
> > > - nit fix on doc
> > > ---
> > > v3 change:
> > > - make nr_shm_domain unsigned int
> > > ---
> > > v2 change:
> > > - document refinement
> > > - remove bitmap and use the iteration to check
> > > - add a new field nr_shm_domain to keep the number of shared domain
> > > ---
> > >    docs/misc/arm/device-tree/booting.txt | 120
> > ++++++++++++++++++++++++++
> > >    xen/arch/arm/Kconfig                  |   6 ++
> > >    xen/arch/arm/bootfdt.c                |  68 +++++++++++++++
> > >    xen/arch/arm/include/asm/setup.h      |   3 +
> > >    4 files changed, 197 insertions(+)
> > >
> > > diff --git a/docs/misc/arm/device-tree/booting.txt
> > > b/docs/misc/arm/device-tree/booting.txt
> > > index 98253414b8..6467bc5a28 100644
> > > --- a/docs/misc/arm/device-tree/booting.txt
> > > +++ b/docs/misc/arm/device-tree/booting.txt
> > > @@ -378,3 +378,123 @@ device-tree:
> > >
> > >    This will reserve a 512MB region starting at the host physical address
> > >    0x30000000 to be exclusively used by DomU1.
> > > +
> > > +Static Shared Memory
> > > +====================
> > > +
> > > +The static shared memory device tree nodes allow users to statically
> > > +set up shared memory on dom0less system, enabling domains to do
> > > +shm-based communication.
> > > +
> > > +- compatible
> > > +
> > > +    "xen,domain-shared-memory-v1"
> > > +
> > > +- xen,shm-id
> > > +
> > > +    An 8-bit integer that represents the unique identifier of the shared
> > memory
> > > +    region. The maximum identifier shall be "xen,shm-id = <0xff>".
> > > +
> > > +- xen,shared-mem
> > > +
> > > +    An array takes a physical address, which is the base address of the
> > > +    shared memory region in host physical address space, a size, and a
> > guest
> > > +    physical address, as the target address of the mapping. The number of
> > cells
> > > +    for the host address (and size) is the same as the guest pseudo-physical
> > > +    address and they are inherited from the parent node.
> > Sorry for jump in the discussion late. But as this is going to be a stable ABI, 
> > I
> > would to make sure the interface is going to be easily extendable.
> >
> > AFAIU, with your proposal the host physical address is mandatory. I would
> > expect that some user may want to share memory but don't care about the
> > exact location in memory. So I think it would be good to make it optional in
> > the binding.
> >
> > I think this wants to be done now because it would be difficult to change the
> > binding afterwards (the host physical address is the first set of cells).
> >
> > The Xen doesn't need to handle the optional case.
> Sure, I'll make "the host physical address" optional here, and right now, with 
> no actual
> code implementation. I'll make up it later in free time~
>
> The user case you mentioned here is that we let xen to allocate an arbitrary 
> static shared
> memory region, so size and guest physical address are still mandatory, right?
That's correct.

Cheers,

--
Julien Grall