Static Allocation refers to system or sub-system(domains) for which memory
areas are pre-defined by configuration using physical address ranges.
Those pre-defined memory, -- Static Memory, as parts of RAM reserved in the
beginning, shall never go to heap allocator or boot allocator for any use.
Domains on Static Allocation is supported through device tree property
`xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
By default, they shall be mapped to the fixed guest RAM address
`GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
This patch introduces this new `xen,static-mem` feature, and also documents
and parses this new attribute at boot time and stores related info in
static_mem for later initialization.
Signed-off-by: Penny Zheng <penny.zheng@arm.com>
---
docs/misc/arm/device-tree/booting.txt | 40 +++++++++++++++++++++
xen/arch/arm/bootfdt.c | 51 +++++++++++++++++++++++++++
xen/include/asm-arm/setup.h | 2 ++
3 files changed, 93 insertions(+)
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
index 5243bc7fd3..2a1ddca29b 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -268,3 +268,43 @@ The DTB fragment is loaded at 0xc000000 in the example above. It should
follow the convention explained in docs/misc/arm/passthrough.txt. The
DTB fragment will be added to the guest device tree, so that the guest
kernel will be able to discover the device.
+
+
+Static Allocation
+=============
+
+Static Allocation refers to system or sub-system(domains) for which memory
+areas are pre-defined by configuration using physical address ranges.
+Those pre-defined memory, -- Static Memory, as parts of RAM reserved in the
+beginning, shall never go to heap allocator or boot allocator for any use.
+
+Domains on Static Allocation is supported through static memory property,
+defined under according /domUx in the name of "xen,static-mem", which are
+specifying physical RAM as this domain's guest RAM.
+The size of address-cells/size-cells must be defined in
+"#xen,static-mem-address-cells" and "#xen,static-mem-size-cells".
+
+On memory allocation, these pre-defined static memory ranges shall be
+firstly mapped to the fixed guest bank "GUEST_RAM0". Until it exhausts the
+`GUEST_RAM0_SIZE`, then it will seek to `GUEST_RAM1_BASE`, and so on.
+`GUEST_RAM0` may take up several pre-defined physical RAM regions.
+
+The dtb property should look like as follows:
+
+ / {
+ chosen {
+ domU1 {
+ compatible = "xen,domain";
+ #address-cells = <0x2>;
+ #size-cells = <0x2>;
+ cpus = <2>;
+ #xen,static-mem-address-cells = <0x1>;
+ #xen,static-mem-size-cells = <0x1>;
+ xen,static-mem = <0x30000000 0x20000000>;
+ ...
+ };
+ };
+ };
+
+DomU1 will have a static memory of 512MB reserved from the physical address
+0x30000000 to 0x50000000.
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 476e32e0f5..d2714446e1 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -193,6 +193,55 @@ static int __init process_reserved_memory_node(const void *fdt, int node,
return 0;
}
+static int __init process_static_memory(const void *fdt, int node, void *data)
+{
+ int i = 0, banks;
+ const __be32 *cell;
+ paddr_t start, size;
+ u32 address_cells, size_cells, reg_cells;
+ struct meminfo *mem = data;
+ const struct fdt_property *prop;
+
+
+ address_cells = device_tree_get_u32(fdt, node,
+ "#xen,static-mem-address-cells", 0);
+ size_cells = device_tree_get_u32(fdt, node,
+ "#xen,static-mem-size-cells", 0);
+ if ( (address_cells == 0) || (size_cells == 0) )
+ {
+ printk("Missing \"#xen,static-mem-address-cell\" or "
+ "\"#xen,static-mem-address-cell\".\n");
+ return -EINVAL;
+ }
+ reg_cells = address_cells + size_cells;
+
+ prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
+ /*
+ * Static memory shall belong to a specific domain, that is,
+ * its node `domUx` has compatible string "xen,domain".
+ */
+ if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
+ {
+ printk("xen,static-mem property can only be located under /domUx node.\n");
+ return -EINVAL;
+ }
+
+ cell = (const __be32 *)prop->data;
+ banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
+
+ for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
+ {
+ device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
+ mem->bank[mem->nr_banks].start = start;
+ mem->bank[mem->nr_banks].size = size;
+ mem->nr_banks++;
+ }
+
+ if ( i < banks )
+ return -ENOSPC;
+ return 0;
+}
+
static int __init process_reserved_memory(const void *fdt, int node,
const char *name, int depth,
u32 address_cells, u32 size_cells)
@@ -346,6 +395,8 @@ static int __init early_scan_node(const void *fdt,
process_multiboot_node(fdt, node, name, address_cells, size_cells);
else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
process_chosen_node(fdt, node, name, address_cells, size_cells);
+ else if ( depth == 2 && fdt_get_property(fdt, node, "xen,static-mem", NULL) )
+ process_static_memory(fdt, node, &bootinfo.static_mem);
if ( rc < 0 )
printk("fdt: node `%s': parsing failed\n", name);
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index c4b6af6029..e076329fc4 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -74,6 +74,8 @@ struct bootinfo {
#ifdef CONFIG_ACPI
struct meminfo acpi;
#endif
+ /* Static Memory */
+ struct meminfo static_mem;
};
extern struct bootinfo bootinfo;
--
2.25.1
Hi Penny,
On 28/07/2021 11:27, Penny Zheng wrote:
> Static Allocation refers to system or sub-system(domains) for which memory
> areas are pre-defined by configuration using physical address ranges.
> Those pre-defined memory, -- Static Memory, as parts of RAM reserved in the
> beginning, shall never go to heap allocator or boot allocator for any use.
>
> Domains on Static Allocation is supported through device tree property
> `xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
> By default, they shall be mapped to the fixed guest RAM address
> `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
>
> This patch introduces this new `xen,static-mem` feature, and also documents
> and parses this new attribute at boot time and stores related info in
> static_mem for later initialization.
>
> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> ---
> docs/misc/arm/device-tree/booting.txt | 40 +++++++++++++++++++++
> xen/arch/arm/bootfdt.c | 51 +++++++++++++++++++++++++++
> xen/include/asm-arm/setup.h | 2 ++
> 3 files changed, 93 insertions(+)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 5243bc7fd3..2a1ddca29b 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -268,3 +268,43 @@ The DTB fragment is loaded at 0xc000000 in the example above. It should
> follow the convention explained in docs/misc/arm/passthrough.txt. The
> DTB fragment will be added to the guest device tree, so that the guest
> kernel will be able to discover the device.
> +
> +
> +Static Allocation
> +=============
> +
> +Static Allocation refers to system or sub-system(domains) for which memory
> +areas are pre-defined by configuration using physical address ranges.
> +Those pre-defined memory, -- Static Memory, as parts of RAM reserved in the
> +beginning, shall never go to heap allocator or boot allocator for any use.
I don't understand "as parts of RAM reserved in the beginning". Could
you clarify it?
> +
> +Domains on Static Allocation is supported through static memory property,
> +defined under according /domUx in the name of "xen,static-mem", which are
We don't require the domU node to be called /domUx.
> +specifying physical RAM as this domain's guest RAM.
>
How about:
Memory can be statically allocated to a domain using the property
"xen,static-mem" defined in the domain configuration.
> +The size of address-cells/size-cells must be defined in
I would say "The number of cells for the address and the size must be
defined using respectively the properties..."
> +"#xen,static-mem-address-cells" and "#xen,static-mem-size-cells".
> +
> +On memory allocation, these pre-defined static memory ranges shall be
> +firstly mapped to the fixed guest bank "GUEST_RAM0". Until it exhausts the
> +`GUEST_RAM0_SIZE`, then it will seek to `GUEST_RAM1_BASE`, and so on.
> +`GUEST_RAM0` may take up several pre-defined physical RAM regions.
GUEST_RAM0 & co are not part of the stable ABI. So I don't think the
documentation should mention them.
But I am not convinced we should provide a guarantee how the allocation
will happen. Why does it matter?
> +
> +The dtb property should look like as follows:
Do you mean "node" rather than "property"?
> +
> + / {
> + chosen {
> + domU1 {
> + compatible = "xen,domain";
> + #address-cells = <0x2>;
> + #size-cells = <0x2>;
> + cpus = <2>;
> + #xen,static-mem-address-cells = <0x1>;
> + #xen,static-mem-size-cells = <0x1>;
> + xen,static-mem = <0x30000000 0x20000000>;
> + ...
> + };
> + };
> + };
> +
> +DomU1 will have a static memory of 512MB reserved from the physical address
> +0x30000000 to 0x50000000.
I would write "This will reserve a 512MB region starting at the host
physical address 0x30000000 to be exclusively used by DomU1".
> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
> index 476e32e0f5..d2714446e1 100644
> --- a/xen/arch/arm/bootfdt.c
> +++ b/xen/arch/arm/bootfdt.c
> @@ -193,6 +193,55 @@ static int __init process_reserved_memory_node(const void *fdt, int node,
> return 0;
> }
>
> +static int __init process_static_memory(const void *fdt, int node, void *data)
> +{
This is pretty much a copy of process_memory_node(). So can we avoid the
duplication?
I think I mentionned it in the past but I can't find the outcome.
> + int i = 0, banks;
> + const __be32 *cell;
> + paddr_t start, size;
> + u32 address_cells, size_cells, reg_cells;
> + struct meminfo *mem = data;
> + const struct fdt_property *prop;
> +
> +
> + address_cells = device_tree_get_u32(fdt, node,
> + "#xen,static-mem-address-cells", 0);
> + size_cells = device_tree_get_u32(fdt, node,
> + "#xen,static-mem-size-cells", 0);
> + if ( (address_cells == 0) || (size_cells == 0) )
> + {
> + printk("Missing \"#xen,static-mem-address-cell\" or "
> + "\"#xen,static-mem-address-cell\".\n");
> + return -EINVAL;
> + }
> + reg_cells = address_cells + size_cells;
> +
> + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
> + /*
> + * Static memory shall belong to a specific domain, that is,
> + * its node `domUx` has compatible string "xen,domain".
> + */
This code is just checking the node compatible is "xen,domain". So I
would drop the "domUx". This is also...
> + if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
> + {
> + printk("xen,static-mem property can only be located under /domUx node.\n");
... not correct.
> + return -EINVAL;
> + }
> +
> + cell = (const __be32 *)prop->data;
> + banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
> +
> + for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
> + {
> + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> + mem->bank[mem->nr_banks].start = start;
> + mem->bank[mem->nr_banks].size = size;
> + mem->nr_banks++;
> + }
> +
> + if ( i < banks )
> + return -ENOSPC;
> + return 0;
> +}
> +
> static int __init process_reserved_memory(const void *fdt, int node,
> const char *name, int depth,
> u32 address_cells, u32 size_cells)
> @@ -346,6 +395,8 @@ static int __init early_scan_node(const void *fdt,
> process_multiboot_node(fdt, node, name, address_cells, size_cells);
> else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
> process_chosen_node(fdt, node, name, address_cells, size_cells);
> + else if ( depth == 2 && fdt_get_property(fdt, node, "xen,static-mem", NULL) )
How about checking the compatible instead?
> + process_static_memory(fdt, node, &bootinfo.static_mem);
You want "rc = ..." so the error is propaged if there is an issue (e.g.
we don't have space for more static region).
>
> if ( rc < 0 )
> printk("fdt: node `%s': parsing failed\n", name);
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index c4b6af6029..e076329fc4 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -74,6 +74,8 @@ struct bootinfo {
> #ifdef CONFIG_ACPI
> struct meminfo acpi;
> #endif
> + /* Static Memory */
> + struct meminfo static_mem;
> };
>
> extern struct bootinfo bootinfo;
>
Cheers,
--
Julien Grall
Hi Julien
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Wednesday, August 11, 2021 9:32 PM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> <Wei.Chen@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH V4 01/10] xen/arm: introduce domain on Static Allocation
>
> Hi Penny,
>
> On 28/07/2021 11:27, Penny Zheng wrote:
> > Static Allocation refers to system or sub-system(domains) for which
> > memory areas are pre-defined by configuration using physical address
> ranges.
> > Those pre-defined memory, -- Static Memory, as parts of RAM reserved
> > in the beginning, shall never go to heap allocator or boot allocator for any
> use.
> >
> > Domains on Static Allocation is supported through device tree property
> > `xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
> > By default, they shall be mapped to the fixed guest RAM address
> > `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
> >
> > This patch introduces this new `xen,static-mem` feature, and also
> > documents and parses this new attribute at boot time and stores
> > related info in static_mem for later initialization.
> >
> > Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> > ---
> > docs/misc/arm/device-tree/booting.txt | 40 +++++++++++++++++++++
> > xen/arch/arm/bootfdt.c | 51 +++++++++++++++++++++++++++
> > xen/include/asm-arm/setup.h | 2 ++
> > 3 files changed, 93 insertions(+)
> >
> > diff --git a/docs/misc/arm/device-tree/booting.txt
> > b/docs/misc/arm/device-tree/booting.txt
> > index 5243bc7fd3..2a1ddca29b 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -268,3 +268,43 @@ The DTB fragment is loaded at 0xc000000 in the
> example above. It should
> > follow the convention explained in docs/misc/arm/passthrough.txt. The
> > DTB fragment will be added to the guest device tree, so that the guest
> > kernel will be able to discover the device.
> > +
> > +
> > +Static Allocation
> > +=============
> > +
> > +Static Allocation refers to system or sub-system(domains) for which
> > +memory areas are pre-defined by configuration using physical address
> ranges.
> > +Those pre-defined memory, -- Static Memory, as parts of RAM reserved
> > +in the beginning, shall never go to heap allocator or boot allocator for any
> use.
>
> I don't understand "as parts of RAM reserved in the beginning". Could you
> clarify it?
>
I mean, static memory is very alike reserved memory, reserved during system boot time,
not dynamically allocated at runtime.
> > +
> > +Domains on Static Allocation is supported through static memory
> > +property, defined under according /domUx in the name of
> > +"xen,static-mem", which are
>
> We don't require the domU node to be called /domUx.
>
Oh, thx for explanation. I will take domU node instead.
> > +specifying physical RAM as this domain's guest RAM.
> >
>
> How about:
>
> Memory can be statically allocated to a domain using the property "xen,static-
> mem" defined in the domain configuration.
>
> > +The size of address-cells/size-cells must be defined in
>
> I would say "The number of cells for the address and the size must be defined
> using respectively the properties..."
>
Sure. Thx for the rephrasing.
> > +"#xen,static-mem-address-cells" and "#xen,static-mem-size-cells".
> > +
> > +On memory allocation, these pre-defined static memory ranges shall be
> > +firstly mapped to the fixed guest bank "GUEST_RAM0". Until it
> > +exhausts the `GUEST_RAM0_SIZE`, then it will seek to `GUEST_RAM1_BASE`,
> and so on.
> > +`GUEST_RAM0` may take up several pre-defined physical RAM regions.
>
> GUEST_RAM0 & co are not part of the stable ABI. So I don't think the
> documentation should mention them.
>
> But I am not convinced we should provide a guarantee how the allocation will
> happen. Why does it matter?
>
Yeah, I put it here to be in comparison with the later 1:1 direct-map, however, it is truly not
part of the stable ABI, so I will delete it in documentation here,
> > +
> > +The dtb property should look like as follows:
>
> Do you mean "node" rather than "property"?
>
Oh, sure. Maybe "as an example" shall be more clarified.
> > + compatible = "xen,domain";
> > + #address-cells = <0x2>;
> > + #size-cells = <0x2>;
> > + cpus = <2>;
> > + #xen,static-mem-address-cells = <0x1>;
> > + #xen,static-mem-size-cells = <0x1>;
> > + xen,static-mem = <0x30000000 0x20000000>;
> > + ...
> > + };
> > + };
> > + };
> > +
> > +DomU1 will have a static memory of 512MB reserved from the physical
> > +address
> > +0x30000000 to 0x50000000.
>
> I would write "This will reserve a 512MB region starting at the host physical
> address 0x30000000 to be exclusively used by DomU1".
>
Sure, thx.
> > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index
> > 476e32e0f5..d2714446e1 100644
> > --- a/xen/arch/arm/bootfdt.c
> > +++ b/xen/arch/arm/bootfdt.c
> > @@ -193,6 +193,55 @@ static int __init
> process_reserved_memory_node(const void *fdt, int node,
> > return 0;
> > }
> >
> > +static int __init process_static_memory(const void *fdt, int node,
> > +void *data) {
>
> This is pretty much a copy of process_memory_node(). So can we avoid the
> duplication?
>
> I think I mentionned it in the past but I can't find the outcome.
>
> > + int i = 0, banks;
> > + const __be32 *cell;
> > + paddr_t start, size;
> > + u32 address_cells, size_cells, reg_cells;
> > + struct meminfo *mem = data;
> > + const struct fdt_property *prop;
> > +
> > +
> > + address_cells = device_tree_get_u32(fdt, node,
> > + "#xen,static-mem-address-cells", 0);
> > + size_cells = device_tree_get_u32(fdt, node,
> > + "#xen,static-mem-size-cells", 0);
> > + if ( (address_cells == 0) || (size_cells == 0) )
> > + {
> > + printk("Missing \"#xen,static-mem-address-cell\" or "
> > + "\"#xen,static-mem-address-cell\".\n");
> > + return -EINVAL;
> > + }
> > + reg_cells = address_cells + size_cells;
> > +
> > + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
> > + /*
> > + * Static memory shall belong to a specific domain, that is,
> > + * its node `domUx` has compatible string "xen,domain".
> > + */
>
> This code is just checking the node compatible is "xen,domain". So I would
> drop the "domUx". This is also...
>
> > + if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
> > + {
> > + printk("xen,static-mem property can only be located under
> > + /domUx node.\n");
>
> ... not correct.
>
I checked it here, to make sure the "xen,static-mem" property must be used in a domain node, since
for now, static memory could be only configured as guest RAM.
Which part do you think it is not appropriate here?
> > + return -EINVAL;
> > + }
> > +
> > + cell = (const __be32 *)prop->data;
> > + banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
> > +
> > + for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
> > + {
> > + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> > + mem->bank[mem->nr_banks].start = start;
> > + mem->bank[mem->nr_banks].size = size;
> > + mem->nr_banks++;
> > + }
> > +
> > + if ( i < banks )
> > + return -ENOSPC;
> > + return 0;
> > +}
> > +
> > static int __init process_reserved_memory(const void *fdt, int node,
> > const char *name, int depth,
> > u32 address_cells, u32
> > size_cells) @@ -346,6 +395,8 @@ static int __init early_scan_node(const
> void *fdt,
> > process_multiboot_node(fdt, node, name, address_cells, size_cells);
> > else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
> > process_chosen_node(fdt, node, name, address_cells,
> > size_cells);
> > + else if ( depth == 2 && fdt_get_property(fdt, node,
> > + "xen,static-mem", NULL) )
>
> How about checking the compatible instead?
>
hmm, since it is a property, not a node. so...
> > + process_static_memory(fdt, node, &bootinfo.static_mem);
>
> You want "rc = ..." so the error is propaged if there is an issue (e.g.
> we don't have space for more static region).
>
Yes, my neglect. I'll make sure the error get propagated here.
> >
> > if ( rc < 0 )
> > printk("fdt: node `%s': parsing failed\n", name); diff --git
> > a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h index
> > c4b6af6029..e076329fc4 100644
> > --- a/xen/include/asm-arm/setup.h
> > +++ b/xen/include/asm-arm/setup.h
> > @@ -74,6 +74,8 @@ struct bootinfo {
> > #ifdef CONFIG_ACPI
> > struct meminfo acpi;
> > #endif
> > + /* Static Memory */
> > + struct meminfo static_mem;
> > };
> >
> > extern struct bootinfo bootinfo;
> >
>
> Cheers,
>
> --
Cheers
Penny
> Julien Grall
On 16/08/2021 06:21, Penny Zheng wrote:
> Hi Julien
Hi Penny,
>> -----Original Message-----
>> From: Julien Grall <julien@xen.org>
>> Sent: Wednesday, August 11, 2021 9:32 PM
>> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org;
>> sstabellini@kernel.org
>> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
>> <Wei.Chen@arm.com>; nd <nd@arm.com>
>> Subject: Re: [PATCH V4 01/10] xen/arm: introduce domain on Static Allocation
>>
>> Hi Penny,
>>
>> On 28/07/2021 11:27, Penny Zheng wrote:
>>> Static Allocation refers to system or sub-system(domains) for which
>>> memory areas are pre-defined by configuration using physical address
>> ranges.
>>> Those pre-defined memory, -- Static Memory, as parts of RAM reserved
>>> in the beginning, shall never go to heap allocator or boot allocator for any
>> use.
>>>
>>> Domains on Static Allocation is supported through device tree property
>>> `xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
>>> By default, they shall be mapped to the fixed guest RAM address
>>> `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
>>>
>>> This patch introduces this new `xen,static-mem` feature, and also
>>> documents and parses this new attribute at boot time and stores
>>> related info in static_mem for later initialization.
>>>
>>> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
>>> ---
>>> docs/misc/arm/device-tree/booting.txt | 40 +++++++++++++++++++++
>>> xen/arch/arm/bootfdt.c | 51 +++++++++++++++++++++++++++
>>> xen/include/asm-arm/setup.h | 2 ++
>>> 3 files changed, 93 insertions(+)
>>>
>>> diff --git a/docs/misc/arm/device-tree/booting.txt
>>> b/docs/misc/arm/device-tree/booting.txt
>>> index 5243bc7fd3..2a1ddca29b 100644
>>> --- a/docs/misc/arm/device-tree/booting.txt
>>> +++ b/docs/misc/arm/device-tree/booting.txt
>>> @@ -268,3 +268,43 @@ The DTB fragment is loaded at 0xc000000 in the
>> example above. It should
>>> follow the convention explained in docs/misc/arm/passthrough.txt. The
>>> DTB fragment will be added to the guest device tree, so that the guest
>>> kernel will be able to discover the device.
>>> +
>>> +
>>> +Static Allocation
>>> +=============
>>> +
>>> +Static Allocation refers to system or sub-system(domains) for which
>>> +memory areas are pre-defined by configuration using physical address
>> ranges.
>>> +Those pre-defined memory, -- Static Memory, as parts of RAM reserved
>>> +in the beginning, shall never go to heap allocator or boot allocator for any
>> use.
>>
>> I don't understand "as parts of RAM reserved in the beginning". Could you
>> clarify it?
>>
>
> I mean, static memory is very alike reserved memory, reserved during system boot time,
> not dynamically allocated at runtime.
Thanks for the clarification. The documentation is meant to be for the
users, so I would suggest to drop the "-- Static memory, as parse of RAM
reserved" because it doesn't add any value to know we treat the static
memory and reserved memory the same way.
>>> +
>>> +The dtb property should look like as follows:
>>
>> Do you mean "node" rather than "property"?
>>
>
> Oh, sure. Maybe "as an example" shall be more clarified.
I would write "Below an example on how to specific the static memory
region in the device-tree".
>
>>> + compatible = "xen,domain";
>>> + #address-cells = <0x2>;
>>> + #size-cells = <0x2>;
>>> + cpus = <2>;
>>> + #xen,static-mem-address-cells = <0x1>;
>>> + #xen,static-mem-size-cells = <0x1>;
>>> + xen,static-mem = <0x30000000 0x20000000>;
>>> + ...
>>> + };
>>> + };
>>> + };
>>> +
>>> +DomU1 will have a static memory of 512MB reserved from the physical
>>> +address
>>> +0x30000000 to 0x50000000.
>>
>> I would write "This will reserve a 512MB region starting at the host physical
>> address 0x30000000 to be exclusively used by DomU1".
>>
>
> Sure, thx.
>
>>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index
>>> 476e32e0f5..d2714446e1 100644
>>> --- a/xen/arch/arm/bootfdt.c
>>> +++ b/xen/arch/arm/bootfdt.c
>>> @@ -193,6 +193,55 @@ static int __init
>> process_reserved_memory_node(const void *fdt, int node,
>>> return 0;
>>> }
>>>
>>> +static int __init process_static_memory(const void *fdt, int node,
>>> +void *data) {
>>
>> This is pretty much a copy of process_memory_node(). So can we avoid the
>> duplication?
>>
>> I think I mentionned it in the past but I can't find the outcome.
>>
>>> + int i = 0, banks;
>>> + const __be32 *cell;
>>> + paddr_t start, size;
>>> + u32 address_cells, size_cells, reg_cells;
>>> + struct meminfo *mem = data;
>>> + const struct fdt_property *prop;
>>> +
>>> +
>>> + address_cells = device_tree_get_u32(fdt, node,
>>> + "#xen,static-mem-address-cells", 0);
>>> + size_cells = device_tree_get_u32(fdt, node,
>>> + "#xen,static-mem-size-cells", 0);
>>> + if ( (address_cells == 0) || (size_cells == 0) )
>>> + {
>>> + printk("Missing \"#xen,static-mem-address-cell\" or "
>>> + "\"#xen,static-mem-address-cell\".\n");
>>> + return -EINVAL;
>>> + }
>>> + reg_cells = address_cells + size_cells;
>>> +
>>> + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
>>> + /*
>>> + * Static memory shall belong to a specific domain, that is,
>>> + * its node `domUx` has compatible string "xen,domain".
>>> + */
>>
>> This code is just checking the node compatible is "xen,domain". So I would
>> drop the "domUx". This is also...
>>
>>> + if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
>>> + {
>>> + printk("xen,static-mem property can only be located under
>>> + /domUx node.\n");
>>
>> ... not correct.
>>
>
> I checked it here, to make sure the "xen,static-mem" property must be used in a domain node, since
> for now, static memory could be only configured as guest RAM.
>
> Which part do you think it is not appropriate here?
You wrote "... can only be located under /domUx". That's not correct
because we don't force (or even mention to) the user to name the node
that way.
>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + cell = (const __be32 *)prop->data;
>>> + banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
>>> +
>>> + for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
>>> + {
>>> + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
>>> + mem->bank[mem->nr_banks].start = start;
>>> + mem->bank[mem->nr_banks].size = size;
>>> + mem->nr_banks++;
>>> + }
>>> +
>>> + if ( i < banks )
>>> + return -ENOSPC;
>>> + return 0;
>>> +}
>>> +
>>> static int __init process_reserved_memory(const void *fdt, int node,
>>> const char *name, int depth,
>>> u32 address_cells, u32
>>> size_cells) @@ -346,6 +395,8 @@ static int __init early_scan_node(const
>> void *fdt,
>>> process_multiboot_node(fdt, node, name, address_cells, size_cells);
>>> else if ( depth == 1 && device_tree_node_matches(fdt, node, "chosen") )
>>> process_chosen_node(fdt, node, name, address_cells,
>>> size_cells);
>>> + else if ( depth == 2 && fdt_get_property(fdt, node,
>>> + "xen,static-mem", NULL) )
>>
>> How about checking the compatible instead?
>>
>
> hmm, since it is a property, not a node. so...
Right, but you could write:
device_tree_node_compatible(fdt, node, "xen,domain")
This would be more correct because we are interested in node using the
Xen domain binding that contains the property "xen,static-mem".
All the other nodes with the property "xen,static-mem" should be left
alone because it may have a different meaning.
Cheers,
--
Julien Grall
Hi Julien
> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Tuesday, August 17, 2021 1:28 AM
> To: Penny Zheng <Penny.Zheng@arm.com>; xen-devel@lists.xenproject.org;
> sstabellini@kernel.org
> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> <Wei.Chen@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH V4 01/10] xen/arm: introduce domain on Static Allocation
>
>
>
> On 16/08/2021 06:21, Penny Zheng wrote:
> > Hi Julien
>
> Hi Penny,
>
> >> -----Original Message-----
> >> From: Julien Grall <julien@xen.org>
> >> Sent: Wednesday, August 11, 2021 9:32 PM
> >> To: Penny Zheng <Penny.Zheng@arm.com>;
> >> xen-devel@lists.xenproject.org; sstabellini@kernel.org
> >> Cc: Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen
> >> <Wei.Chen@arm.com>; nd <nd@arm.com>
> >> Subject: Re: [PATCH V4 01/10] xen/arm: introduce domain on Static
> >> Allocation
> >>
> >> Hi Penny,
> >>
> >> On 28/07/2021 11:27, Penny Zheng wrote:
> >>> Static Allocation refers to system or sub-system(domains) for which
> >>> memory areas are pre-defined by configuration using physical address
> >> ranges.
> >>> Those pre-defined memory, -- Static Memory, as parts of RAM reserved
> >>> in the beginning, shall never go to heap allocator or boot allocator
> >>> for any
> >> use.
> >>>
> >>> Domains on Static Allocation is supported through device tree
> >>> property `xen,static-mem` specifying reserved RAM banks as this domain's
> guest RAM.
> >>> By default, they shall be mapped to the fixed guest RAM address
> >>> `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
> >>>
> >>> This patch introduces this new `xen,static-mem` feature, and also
> >>> documents and parses this new attribute at boot time and stores
> >>> related info in static_mem for later initialization.
> >>>
> >>> Signed-off-by: Penny Zheng <penny.zheng@arm.com>
> >>> ---
> >>> docs/misc/arm/device-tree/booting.txt | 40 +++++++++++++++++++++
> >>> xen/arch/arm/bootfdt.c | 51 +++++++++++++++++++++++++++
> >>> xen/include/asm-arm/setup.h | 2 ++
> >>> 3 files changed, 93 insertions(+)
> >>>
> >>> diff --git a/docs/misc/arm/device-tree/booting.txt
> >>> b/docs/misc/arm/device-tree/booting.txt
> >>> index 5243bc7fd3..2a1ddca29b 100644
> >>> --- a/docs/misc/arm/device-tree/booting.txt
> >>> +++ b/docs/misc/arm/device-tree/booting.txt
> >>> @@ -268,3 +268,43 @@ The DTB fragment is loaded at 0xc000000 in the
> >> example above. It should
> >>> follow the convention explained in docs/misc/arm/passthrough.txt. The
> >>> DTB fragment will be added to the guest device tree, so that the guest
> >>> kernel will be able to discover the device.
> >>> +
> >>> +
> >>> +Static Allocation
> >>> +=============
> >>> +
> >>> +Static Allocation refers to system or sub-system(domains) for which
> >>> +memory areas are pre-defined by configuration using physical
> >>> +address
> >> ranges.
> >>> +Those pre-defined memory, -- Static Memory, as parts of RAM
> >>> +reserved in the beginning, shall never go to heap allocator or boot
> >>> +allocator for any
> >> use.
> >>
> >> I don't understand "as parts of RAM reserved in the beginning". Could
> >> you clarify it?
> >>
> >
> > I mean, static memory is very alike reserved memory, reserved during
> > system boot time, not dynamically allocated at runtime.
>
> Thanks for the clarification. The documentation is meant to be for the users, so
> I would suggest to drop the "-- Static memory, as parse of RAM reserved"
> because it doesn't add any value to know we treat the static memory and
> reserved memory the same way.
>
Got it. Thx
> >>> +
> >>> +The dtb property should look like as follows:
> >>
> >> Do you mean "node" rather than "property"?
> >>
> >
> > Oh, sure. Maybe "as an example" shall be more clarified.
>
> I would write "Below an example on how to specific the static memory region
> in the device-tree".
>
Thanks for the example. Will do
> >
> >>> + compatible = "xen,domain";
> >>> + #address-cells = <0x2>;
> >>> + #size-cells = <0x2>;
> >>> + cpus = <2>;
> >>> + #xen,static-mem-address-cells = <0x1>;
> >>> + #xen,static-mem-size-cells = <0x1>;
> >>> + xen,static-mem = <0x30000000 0x20000000>;
> >>> + ...
> >>> + };
> >>> + };
> >>> + };
> >>> +
> >>> +DomU1 will have a static memory of 512MB reserved from the physical
> >>> +address
> >>> +0x30000000 to 0x50000000.
> >>
> >> I would write "This will reserve a 512MB region starting at the host
> >> physical address 0x30000000 to be exclusively used by DomU1".
> >>
> >
> > Sure, thx.
> >
> >>> diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index
> >>> 476e32e0f5..d2714446e1 100644
> >>> --- a/xen/arch/arm/bootfdt.c
> >>> +++ b/xen/arch/arm/bootfdt.c
> >>> @@ -193,6 +193,55 @@ static int __init
> >> process_reserved_memory_node(const void *fdt, int node,
> >>> return 0;
> >>> }
> >>>
> >>> +static int __init process_static_memory(const void *fdt, int node,
> >>> +void *data) {
> >>
> >> This is pretty much a copy of process_memory_node(). So can we avoid
> >> the duplication?
> >>
> >> I think I mentionned it in the past but I can't find the outcome.
> >>
> >>> + int i = 0, banks;
> >>> + const __be32 *cell;
> >>> + paddr_t start, size;
> >>> + u32 address_cells, size_cells, reg_cells;
> >>> + struct meminfo *mem = data;
> >>> + const struct fdt_property *prop;
> >>> +
> >>> +
> >>> + address_cells = device_tree_get_u32(fdt, node,
> >>> + "#xen,static-mem-address-cells", 0);
> >>> + size_cells = device_tree_get_u32(fdt, node,
> >>> + "#xen,static-mem-size-cells", 0);
> >>> + if ( (address_cells == 0) || (size_cells == 0) )
> >>> + {
> >>> + printk("Missing \"#xen,static-mem-address-cell\" or "
> >>> + "\"#xen,static-mem-address-cell\".\n");
> >>> + return -EINVAL;
> >>> + }
> >>> + reg_cells = address_cells + size_cells;
> >>> +
> >>> + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL);
> >>> + /*
> >>> + * Static memory shall belong to a specific domain, that is,
> >>> + * its node `domUx` has compatible string "xen,domain".
> >>> + */
> >>
> >> This code is just checking the node compatible is "xen,domain". So I
> >> would drop the "domUx". This is also...
> >>
> >>> + if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
> >>> + {
> >>> + printk("xen,static-mem property can only be located under
> >>> + /domUx node.\n");
> >>
> >> ... not correct.
> >>
> >
> > I checked it here, to make sure the "xen,static-mem" property must be
> > used in a domain node, since for now, static memory could be only
> configured as guest RAM.
> >
> > Which part do you think it is not appropriate here?
>
> You wrote "... can only be located under /domUx". That's not correct because
> we don't force (or even mention to) the user to name the node that way.
>
>
> >
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + cell = (const __be32 *)prop->data;
> >>> + banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof (u32));
> >>> +
> >>> + for ( ; i < banks && mem->nr_banks < NR_MEM_BANKS; i++ )
> >>> + {
> >>> + device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> >>> + mem->bank[mem->nr_banks].start = start;
> >>> + mem->bank[mem->nr_banks].size = size;
> >>> + mem->nr_banks++;
> >>> + }
> >>> +
> >>> + if ( i < banks )
> >>> + return -ENOSPC;
> >>> + return 0;
> >>> +}
> >>> +
> >>> static int __init process_reserved_memory(const void *fdt, int node,
> >>> const char *name, int depth,
> >>> u32 address_cells, u32
> >>> size_cells) @@ -346,6 +395,8 @@ static int __init
> >>> early_scan_node(const
> >> void *fdt,
> >>> process_multiboot_node(fdt, node, name, address_cells, size_cells);
> >>> else if ( depth == 1 && device_tree_node_matches(fdt, node,
> "chosen") )
> >>> process_chosen_node(fdt, node, name, address_cells,
> >>> size_cells);
> >>> + else if ( depth == 2 && fdt_get_property(fdt, node,
> >>> + "xen,static-mem", NULL) )
> >>
> >> How about checking the compatible instead?
> >>
> >
> > hmm, since it is a property, not a node. so...
> Right, but you could write:
>
> device_tree_node_compatible(fdt, node, "xen,domain")
>
> This would be more correct because we are interested in node using the Xen
> domain binding that contains the property "xen,static-mem".
>
> All the other nodes with the property "xen,static-mem" should be left alone
> because it may have a different meaning.
>
True, true. Will do
> Cheers,
>
> --
> Julien Grall
© 2016 - 2026 Red Hat, Inc.