Hi stefano
> -----Original Message-----
> From: Stefano Stabellini <sstabell...@kernel.org>
> Sent: Thursday, September 8, 2022 8:16 AM
> To: Julien Grall <jul...@xen.org>
> Cc: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org;
> Wei Chen <wei.c...@arm.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>
> Subject: Re: [PATCH v7 7/9] xen/arm: create shared memory nodes in guest
> device tree
>
> On Wed, 7 Sep 2022, Julien Grall wrote:
> > On 06/09/2022 09:59, Penny Zheng wrote:
> > > We expose the shared memory to the domU using the "xen,shared-
> memory-v1"
> > > reserved-memory binding. See
> > > Documentation/devicetree/bindings/reserved-memory/xen,shared-
> memory.
> > > txt in Linux for the corresponding device tree binding.
> > >
> > > To save the cost of re-parsing shared memory device tree
> > > configuration when creating shared memory nodes in guest device
> > > tree, this commit adds new field "shm_mem" to store shm-info per
> > > domain.
> > >
> > > For each shared memory region, a range is exposed under the
> > > /reserved-memory node as a child node. Each range sub-node is named
> > > xen-shmem@<address> and has the following properties:
> > > - compatible:
> > > compatible = "xen,shared-memory-v1"
> > > - reg:
> > > the base guest physical address and size of the shared
> > > memory region
> > > - xen,id:
> > > a string that identifies the shared memory region.
> >
> > So technically, there is a property "xen,offset" that should be
> > specified for the borrowers.
> >
> > TBH, I don't quite understand what this property is used for. So it is
> > not quite clear why this is skipped.
> >
> > The Stefano is the author of the binding. So maybe he can explain the
> > purpose of the property and help to document it in the commit message
> > why this is ignored.
>
> It looks like it is something we introduced to handle the case where memory
> from the owner is shared with multiple borrowers. Each borrower would
> have its own offset within the region shared by the owner:
>
> https://marc.info/?l=xen-devel&m=154110446604365&w=2
>
IMHO, "xen,offset" more fits in the xen dts? We configure it in borrower memory
node,
then later we shall only set up foreign memory map starting at the offset?
'''
domU1-shared-mem@10000000 {
compatible = "xen,domain-shared-memory-v1";
role = "borrower";
xen,shm-id = "my-shared-mem-0";
xen,shared-mem = <0x10000000 0x50000000 0x10000000>;
xen,offset = <0x0 0x01000000>;
}
'''
For borrower domain, only [0x11000000, 0x20000000) need to get mapped...
Of course, we could limit the memory map in related Linux driver, but for
safety,
it should be at Xen?
>
> The use-case is a bit of a corner case but it looks valid. If I had to do it
> now, I
> would at least mark "xen,offset" as "optional".
>
> I think we have two options here and I am happy with either one:
>
> 1) we add xen,offset = <0x0>;
>
I will let v8 stay with this configuration, and a TODO for actual
implementation.
> 2) we do *not* add xen,offset and instead send a patch to the LKML to fix
> Documentation/devicetree/bindings/reserved-memory/xen,shared-
> memory.txt
> so that it clearly states that xen,offset is optional. I don't know if Rob
> would
> accept such a patch without changing the version in the compatible string.
>
> Given the release deadline, I would go with 1). We can always remove it once
> it becomes clearly optional.
