| Message ID | 20220906085941.944592-8-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | static shared memory on dom0less system | expand |
On Tue, 6 Sep 2022, 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. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > --- > v7 changes: > - allocate reg for worst case addrcells + sizecells > - replace assert() with BUG_ON() since it is init code > --- > v6 change: > - change "struct meminfo *mem" to "const struct meminfo *mem" > - change "unsigned long i" to "unsigned int i" to match the type of nr_banks. > - accroding to the Linux binding, "xen,id" is meant to be a string, not > an integer > --- > v5 change: > - no change > --- > v4 change: > - no change > --- > v3 change: > - move field "shm_mem" to kernel_info > --- > v2 change: > - using xzalloc > - shm_id should be uint8_t > - make reg a local variable > - add #address-cells and #size-cells properties > - fix alignment > --- > xen/arch/arm/domain_build.c | 147 +++++++++++++++++++++++++++++- > xen/arch/arm/include/asm/kernel.h | 1 + > 2 files changed, 146 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d0ff487cc6..3b7436030e 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -914,7 +914,22 @@ static int __init assign_shared_memory(struct domain *d, > return ret; > } > > -static int __init process_shm(struct domain *d, > +static int __init append_shm_bank_to_domain(struct kernel_info *kinfo, > + paddr_t start, paddr_t size, > + const char *shm_id) > +{ > + if ( (kinfo->shm_mem.nr_banks + 1) > NR_MEM_BANKS ) > + return -ENOMEM; > + > + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].start = start; > + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].size = size; > + safe_strcpy(kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].shm_id, shm_id); > + kinfo->shm_mem.nr_banks++; > + > + return 0; > +} > + > +static int __init process_shm(struct domain *d, struct kernel_info *kinfo, > const struct dt_device_node *node) > { > struct dt_device_node *shm_node; > @@ -928,6 +943,7 @@ static int __init process_shm(struct domain *d, > int ret = 0; > unsigned int i; > const char *role_str; > + const char *shm_id; > bool owner_dom_io = true; > > if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-v1") ) > @@ -972,6 +988,9 @@ static int __init process_shm(struct domain *d, > if ( dt_property_read_string(shm_node, "role", &role_str) == 0 ) > owner_dom_io = false; > > + dt_property_read_string(shm_node, "xen,shm-id", &shm_id); > + BUG_ON((strlen(shm_id) <= 0) || (strlen(shm_id) >= MAX_SHM_ID_LENGTH)); > + > /* > * DOMID_IO is a fake domain and is not described in the Device-Tree. > * Therefore when the owner of the shared region is DOMID_IO, we will > @@ -999,6 +1018,14 @@ static int __init process_shm(struct domain *d, > if ( ret ) > return ret; > } > + > + /* > + * Record static shared memory region info for later setting > + * up shm-node in guest device tree. > + */ > + ret = append_shm_bank_to_domain(kinfo, gbase, psize, shm_id); > + if ( ret ) > + return ret; > } > > return 0; > @@ -1329,6 +1356,117 @@ static int __init make_memory_node(const struct domain *d, > return res; > } > > +#ifdef CONFIG_STATIC_SHM > +static int __init make_shm_memory_node(const struct domain *d, > + void *fdt, > + int addrcells, int sizecells, > + const struct meminfo *mem) > +{ > + unsigned int i = 0; > + int res = 0; > + > + if ( mem->nr_banks == 0 ) > + return -ENOENT; > + > + /* > + * 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>. > + */ > + dt_dprintk("Create xen-shmem node\n"); > + > + for ( ; i < mem->nr_banks; i++ ) > + { > + uint64_t start = mem->bank[i].start; > + uint64_t size = mem->bank[i].size; > + /* Placeholder for xen-shmem@ + a 64-bit number + \0 */ > + char buf[27]; > + const char compat[] = "xen,shared-memory-v1"; > + /* Worst case addrcells + sizecells */ > + __be32 reg[4]; I think it should be: __be32 reg[GUEST_ROOT_ADDRESS_CELLS + GUEST_ROOT_SIZE_CELLS] aside from that (it could be fixed on commit): Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > + __be32 *cells; > + unsigned int len = (addrcells + sizecells) * sizeof(__be32); > + > + snprintf(buf, sizeof(buf), "xen-shmem@%"PRIx64, mem->bank[i].start); > + res = fdt_begin_node(fdt, buf); > + if ( res ) > + return res; > + > + res = fdt_property(fdt, "compatible", compat, sizeof(compat)); > + if ( res ) > + return res; > + > + cells = reg; > + dt_child_set_range(&cells, addrcells, sizecells, start, size); > + > + res = fdt_property(fdt, "reg", reg, len); > + if ( res ) > + return res; > + > + dt_dprintk("Shared memory bank %u: %#"PRIx64"->%#"PRIx64"\n", > + i, start, start + size); > + > + res = fdt_property_string(fdt, "xen,id", mem->bank[i].shm_id); > + 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, > + struct meminfo *mem) > +{ > + ASSERT_UNREACHABLE(); > +} > +#endif > + > +static int __init make_resv_memory_node(const struct domain *d, > + void *fdt, > + int addrcells, int sizecells, > + struct meminfo *mem) > +{ > + int res = 0; > + /* Placeholder for reserved-memory\0 */ > + char resvbuf[16] = "reserved-memory"; > + > + if ( mem->nr_banks == 0 ) > + /* No shared memory provided. */ > + return 0; > + > + dt_dprintk("Create reserved-memory node\n"); > + > + res = fdt_begin_node(fdt, resvbuf); > + if ( res ) > + return res; > + > + res = fdt_property(fdt, "ranges", NULL, 0); > + if ( res ) > + return res; > + > + res = fdt_property_cell(fdt, "#address-cells", addrcells); > + if ( res ) > + return res; > + > + res = fdt_property_cell(fdt, "#size-cells", sizecells); > + if ( res ) > + return res; > + > + res = make_shm_memory_node(d, fdt, addrcells, sizecells, mem); > + if ( res ) > + return res; > + > + res = fdt_end_node(fdt); > + > + return res; > +} > + > static int __init add_ext_regions(unsigned long s, unsigned long e, void *data) > { > struct meminfo *ext_regions = data; > @@ -3106,6 +3244,11 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo) > if ( ret ) > goto err; > > + ret = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, > + &kinfo->shm_mem); > + if ( ret ) > + goto err; > + > /* > * domain_handle_dtb_bootmodule has to be called before the rest of > * the device tree is generated because it depends on the value of > @@ -3482,7 +3625,7 @@ static int __init construct_domU(struct domain *d, > assign_static_memory_11(d, &kinfo, node); > > #ifdef CONFIG_STATIC_SHM > - rc = process_shm(d, node); > + rc = process_shm(d, &kinfo, node); > if ( rc < 0 ) > return rc; > #endif > diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h > index c4dc039b54..2cc506b100 100644 > --- a/xen/arch/arm/include/asm/kernel.h > +++ b/xen/arch/arm/include/asm/kernel.h > @@ -19,6 +19,7 @@ struct kernel_info { > void *fdt; /* flat device tree */ > paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */ > struct meminfo mem; > + struct meminfo shm_mem; > > /* kernel entry point */ > paddr_t entry; > -- > 2.25.1 >
Hi Penny, 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. > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c > index d0ff487cc6..3b7436030e 100644 > --- a/xen/arch/arm/domain_build.c > +++ b/xen/arch/arm/domain_build.c > @@ -914,7 +914,22 @@ static int __init assign_shared_memory(struct domain *d, > return ret; > } > > -static int __init process_shm(struct domain *d, > +static int __init append_shm_bank_to_domain(struct kernel_info *kinfo, > + paddr_t start, paddr_t size, > + const char *shm_id) > +{ > + if ( (kinfo->shm_mem.nr_banks + 1) > NR_MEM_BANKS ) NIT: The +1 could be avoided if you use >=. This would also avoid to think about the overflow case for nr_banks :). > + return -ENOMEM; > + > + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].start = start; > + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].size = size; > + safe_strcpy(kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].shm_id, shm_id); > + kinfo->shm_mem.nr_banks++; > + > + return 0; > +} > + > +static int __init process_shm(struct domain *d, struct kernel_info *kinfo, > const struct dt_device_node *node) > { > struct dt_device_node *shm_node; > @@ -928,6 +943,7 @@ static int __init process_shm(struct domain *d, > int ret = 0; > unsigned int i; > const char *role_str; > + const char *shm_id; > bool owner_dom_io = true; > > if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-v1") ) > @@ -972,6 +988,9 @@ static int __init process_shm(struct domain *d, > if ( dt_property_read_string(shm_node, "role", &role_str) == 0 ) > owner_dom_io = false; > > + dt_property_read_string(shm_node, "xen,shm-id", &shm_id); shm_id will only be set if dt_property_read_string() returns 0. Otherwise it will be unknown. Given... > + BUG_ON((strlen(shm_id) <= 0) || (strlen(shm_id) >= MAX_SHM_ID_LENGTH)); ... this BUG_ON(), I assuming you want to double check that the property is correct. So you want to also check the return value. Otherwise, I suspect a static analyzer will complain that you may use unitialized value. [...] > +static int __init make_resv_memory_node(const struct domain *d, > + void *fdt, > + int addrcells, int sizecells, > + struct meminfo *mem) AFAICT 'mem' could be const. > +{ > + int res = 0; > + /* Placeholder for reserved-memory\0 */ > + char resvbuf[16] = "reserved-memory"; Same here. Cheers,
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 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>; 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.
Hi stefano > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: Thursday, September 8, 2022 8:16 AM > To: Julien Grall <julien@xen.org> > Cc: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org; > Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini > <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>; > Volodymyr Babchuk <Volodymyr_Babchuk@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.
Hi, Replying to Penny and Stefano answer in the same e-mail. On 08/09/2022 04:21, Penny Zheng wrote: > >> -----Original Message----- >> From: Stefano Stabellini <sstabellini@kernel.org> >> Sent: Thursday, September 8, 2022 8:16 AM >> To: Julien Grall <julien@xen.org> >> Cc: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org; >> Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini >> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>; >> Volodymyr Babchuk <Volodymyr_Babchuk@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 Thanks for the pointer :). Now, I remember what this was meant for. >> > > 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? Yes and Yes. > ''' > 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? Correct. > >> >> 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. Agree. It sounds the best approach for now. Cheers,
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c index d0ff487cc6..3b7436030e 100644 --- a/xen/arch/arm/domain_build.c +++ b/xen/arch/arm/domain_build.c @@ -914,7 +914,22 @@ static int __init assign_shared_memory(struct domain *d, return ret; } -static int __init process_shm(struct domain *d, +static int __init append_shm_bank_to_domain(struct kernel_info *kinfo, + paddr_t start, paddr_t size, + const char *shm_id) +{ + if ( (kinfo->shm_mem.nr_banks + 1) > NR_MEM_BANKS ) + return -ENOMEM; + + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].start = start; + kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].size = size; + safe_strcpy(kinfo->shm_mem.bank[kinfo->shm_mem.nr_banks].shm_id, shm_id); + kinfo->shm_mem.nr_banks++; + + return 0; +} + +static int __init process_shm(struct domain *d, struct kernel_info *kinfo, const struct dt_device_node *node) { struct dt_device_node *shm_node; @@ -928,6 +943,7 @@ static int __init process_shm(struct domain *d, int ret = 0; unsigned int i; const char *role_str; + const char *shm_id; bool owner_dom_io = true; if ( !dt_device_is_compatible(shm_node, "xen,domain-shared-memory-v1") ) @@ -972,6 +988,9 @@ static int __init process_shm(struct domain *d, if ( dt_property_read_string(shm_node, "role", &role_str) == 0 ) owner_dom_io = false; + dt_property_read_string(shm_node, "xen,shm-id", &shm_id); + BUG_ON((strlen(shm_id) <= 0) || (strlen(shm_id) >= MAX_SHM_ID_LENGTH)); + /* * DOMID_IO is a fake domain and is not described in the Device-Tree. * Therefore when the owner of the shared region is DOMID_IO, we will @@ -999,6 +1018,14 @@ static int __init process_shm(struct domain *d, if ( ret ) return ret; } + + /* + * Record static shared memory region info for later setting + * up shm-node in guest device tree. + */ + ret = append_shm_bank_to_domain(kinfo, gbase, psize, shm_id); + if ( ret ) + return ret; } return 0; @@ -1329,6 +1356,117 @@ static int __init make_memory_node(const struct domain *d, return res; } +#ifdef CONFIG_STATIC_SHM +static int __init make_shm_memory_node(const struct domain *d, + void *fdt, + int addrcells, int sizecells, + const struct meminfo *mem) +{ + unsigned int i = 0; + int res = 0; + + if ( mem->nr_banks == 0 ) + return -ENOENT; + + /* + * 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>. + */ + dt_dprintk("Create xen-shmem node\n"); + + for ( ; i < mem->nr_banks; i++ ) + { + uint64_t start = mem->bank[i].start; + uint64_t size = mem->bank[i].size; + /* Placeholder for xen-shmem@ + a 64-bit number + \0 */ + char buf[27]; + const char compat[] = "xen,shared-memory-v1"; + /* Worst case addrcells + sizecells */ + __be32 reg[4]; + __be32 *cells; + unsigned int len = (addrcells + sizecells) * sizeof(__be32); + + snprintf(buf, sizeof(buf), "xen-shmem@%"PRIx64, mem->bank[i].start); + res = fdt_begin_node(fdt, buf); + if ( res ) + return res; + + res = fdt_property(fdt, "compatible", compat, sizeof(compat)); + if ( res ) + return res; + + cells = reg; + dt_child_set_range(&cells, addrcells, sizecells, start, size); + + res = fdt_property(fdt, "reg", reg, len); + if ( res ) + return res; + + dt_dprintk("Shared memory bank %u: %#"PRIx64"->%#"PRIx64"\n", + i, start, start + size); + + res = fdt_property_string(fdt, "xen,id", mem->bank[i].shm_id); + 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, + struct meminfo *mem) +{ + ASSERT_UNREACHABLE(); +} +#endif + +static int __init make_resv_memory_node(const struct domain *d, + void *fdt, + int addrcells, int sizecells, + struct meminfo *mem) +{ + int res = 0; + /* Placeholder for reserved-memory\0 */ + char resvbuf[16] = "reserved-memory"; + + if ( mem->nr_banks == 0 ) + /* No shared memory provided. */ + return 0; + + dt_dprintk("Create reserved-memory node\n"); + + res = fdt_begin_node(fdt, resvbuf); + if ( res ) + return res; + + res = fdt_property(fdt, "ranges", NULL, 0); + if ( res ) + return res; + + res = fdt_property_cell(fdt, "#address-cells", addrcells); + if ( res ) + return res; + + res = fdt_property_cell(fdt, "#size-cells", sizecells); + if ( res ) + return res; + + res = make_shm_memory_node(d, fdt, addrcells, sizecells, mem); + if ( res ) + return res; + + res = fdt_end_node(fdt); + + return res; +} + static int __init add_ext_regions(unsigned long s, unsigned long e, void *data) { struct meminfo *ext_regions = data; @@ -3106,6 +3244,11 @@ static int __init prepare_dtb_domU(struct domain *d, struct kernel_info *kinfo) if ( ret ) goto err; + ret = make_resv_memory_node(d, kinfo->fdt, addrcells, sizecells, + &kinfo->shm_mem); + if ( ret ) + goto err; + /* * domain_handle_dtb_bootmodule has to be called before the rest of * the device tree is generated because it depends on the value of @@ -3482,7 +3625,7 @@ static int __init construct_domU(struct domain *d, assign_static_memory_11(d, &kinfo, node); #ifdef CONFIG_STATIC_SHM - rc = process_shm(d, node); + rc = process_shm(d, &kinfo, node); if ( rc < 0 ) return rc; #endif diff --git a/xen/arch/arm/include/asm/kernel.h b/xen/arch/arm/include/asm/kernel.h index c4dc039b54..2cc506b100 100644 --- a/xen/arch/arm/include/asm/kernel.h +++ b/xen/arch/arm/include/asm/kernel.h @@ -19,6 +19,7 @@ struct kernel_info { void *fdt; /* flat device tree */ paddr_t unassigned_mem; /* RAM not (yet) assigned to a bank */ struct meminfo mem; + struct meminfo shm_mem; /* kernel entry point */ paddr_t entry;
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. Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- v7 changes: - allocate reg for worst case addrcells + sizecells - replace assert() with BUG_ON() since it is init code --- v6 change: - change "struct meminfo *mem" to "const struct meminfo *mem" - change "unsigned long i" to "unsigned int i" to match the type of nr_banks. - accroding to the Linux binding, "xen,id" is meant to be a string, not an integer --- v5 change: - no change --- v4 change: - no change --- v3 change: - move field "shm_mem" to kernel_info --- v2 change: - using xzalloc - shm_id should be uint8_t - make reg a local variable - add #address-cells and #size-cells properties - fix alignment --- xen/arch/arm/domain_build.c | 147 +++++++++++++++++++++++++++++- xen/arch/arm/include/asm/kernel.h | 1 + 2 files changed, 146 insertions(+), 2 deletions(-)