| Message ID | 20210824095045.2281500-3-penny.zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Domain on Static Allocation | expand |
On Tue, 24 Aug 2021, 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. > > Memory can be statically allocated to a domain using the property "xen,static- > mem" defined in the domain configuration. 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". > > This patch introduces this new `xen,static-mem` feature, and also documents > and parses this new attribute at boot time. > > This patch also introduces a new field "bool xen_domain" in "struct membank" > to tell the difference of one memory bank is reserved as the whole > hardware resource, or bind to one specific xen domain node, through > "xen,static-mem" > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > --- > v5 changes: > - check the node using the Xen domain binding whether contains the property > "xen,static-mem", not the property itself > - add "rc = ..." to get the error propagated > - introduce new field "bool xen_domain", then static memory shall be also stored > as reserved memory(bootinfo.reserved_mem), but being bind to one > specific Xen domain node. > - doc refinement > --- > docs/misc/arm/device-tree/booting.txt | 33 ++++++++++++++++++++++++ > xen/arch/arm/bootfdt.c | 36 +++++++++++++++++++++++++-- > xen/include/asm-arm/setup.h | 1 + > 3 files changed, 68 insertions(+), 2 deletions(-) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 5243bc7fd3..95b20ddc3a 100644 > --- a/docs/misc/arm/device-tree/booting.txt > +++ b/docs/misc/arm/device-tree/booting.txt > @@ -268,3 +268,36 @@ 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. > + > +Memory can be statically allocated to a domain using the property "xen,static- > +mem" defined in the domain configuration. 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". > + > +Below is an example on how to specify the static memory region in the > +device-tree: > + > + / { > + 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>; > + ... > + }; > + }; > + }; > + > +This will reserve a 512MB region starting at the host physical address > +0x30000000 to be exclusively used by DomU1 This binding is OK. We might want to clarify what is the purpose of the "memory" property when "xen,static-mem" is present. I would suggest to write that when "xen,static-mem" is present, the "memory" property becomes optional. Or even better: """ When present, the xen,static-mem property supersedes the memory property. """ In the future when Xen will support direct mapping, I assume that we'll also add a direct-map property to enable it. Something like: 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>; direct-map; ... }; Maybe I would add a statement to clarify it that xen,static-mem doesn't automatically imply direct mapping. Something like: """ The static memory will be mapped in the guest at the usual guest memory addresses (GUEST_RAM0_BASE, GUEST_RAM1_BASE) defined by xen/include/public/arch-arm.h. """ The rest of the patch looks OK. One minor NIT below. > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c > index 8c81be3379..00f34eec58 100644 > --- a/xen/arch/arm/bootfdt.c > +++ b/xen/arch/arm/bootfdt.c > @@ -66,7 +66,7 @@ void __init device_tree_get_reg(const __be32 **cell, u32 address_cells, > static int __init device_tree_get_meminfo(const void *fdt, int node, > const char *prop_name, > u32 address_cells, u32 size_cells, > - void *data) > + void *data, bool xen_domain) > { > const struct fdt_property *prop; > unsigned int i, banks; > @@ -90,6 +90,7 @@ static int __init device_tree_get_meminfo(const void *fdt, int node, > continue; > mem->bank[mem->nr_banks].start = start; > mem->bank[mem->nr_banks].size = size; > + mem->bank[mem->nr_banks].xen_domain = xen_domain; > mem->nr_banks++; > } > > @@ -184,7 +185,8 @@ static int __init process_memory_node(const void *fdt, int node, > return -EINVAL; > } > > - return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, data); > + return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, > + data, false); > } > > static int __init process_reserved_memory_node(const void *fdt, int node, > @@ -338,6 +340,34 @@ static void __init process_chosen_node(const void *fdt, int node, > add_boot_module(BOOTMOD_RAMDISK, start, end-start, false); > } > > +static int __init process_domain_node(const void *fdt, int node, > + const char *name, > + u32 address_cells, u32 size_cells) > +{ > + const struct fdt_property *prop; > + > + printk("Checking for \"xen,static-mem\" in domain node\n"); > + > + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL); > + if ( !prop ) > + /* No "xen,static-mem" present. */ > + return 0; > + > + 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 < 1 || size_cells < 1 ) > + { > + printk("fdt: node `%s': invalid #xen,static-mem-address-cells or #xen,static-mem-size-cells", > + name); > + return -EINVAL; > + } > + > + return device_tree_get_meminfo(fdt, node, "xen,static-mem", address_cells, > + size_cells, &bootinfo.reserved_mem, true); > +} > + > static int __init early_scan_node(const void *fdt, > int node, const char *name, int depth, > u32 address_cells, u32 size_cells, > @@ -356,6 +386,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 && device_tree_node_compatible(fdt, node, "xen,domain") ) > + rc = process_domain_node(fdt, node, name, address_cells, size_cells); > > 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..6c3c16294b 100644 > --- a/xen/include/asm-arm/setup.h > +++ b/xen/include/asm-arm/setup.h > @@ -24,6 +24,7 @@ typedef enum { > struct membank { > paddr_t start; > paddr_t size; > + bool xen_domain; /* whether memory bank is bind to Xen domain. */ ^ a or the ^ bound to a > }; > > struct meminfo { > -- > 2.25.1 >
Hi Stefano > -----Original Message----- > From: Stefano Stabellini <sstabellini@kernel.org> > Sent: Friday, September 3, 2021 5:31 AM > To: Penny Zheng <Penny.Zheng@arm.com> > Cc: xen-devel@lists.xenproject.org; sstabellini@kernel.org; julien@xen.org; > Bertrand Marquis <Bertrand.Marquis@arm.com>; Wei Chen > <Wei.Chen@arm.com>; nd <nd@arm.com> > Subject: Re: [PATCH v5 2/7] xen/arm: introduce domain on Static Allocation > > On Tue, 24 Aug 2021, 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. > > > > Memory can be statically allocated to a domain using the property > > "xen,static- mem" defined in the domain configuration. 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". > > > > This patch introduces this new `xen,static-mem` feature, and also > > documents and parses this new attribute at boot time. > > > > This patch also introduces a new field "bool xen_domain" in "struct > membank" > > to tell the difference of one memory bank is reserved as the whole > > hardware resource, or bind to one specific xen domain node, through > > "xen,static-mem" > > > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > > --- > > v5 changes: > > - check the node using the Xen domain binding whether contains the > > property "xen,static-mem", not the property itself > > - add "rc = ..." to get the error propagated > > - introduce new field "bool xen_domain", then static memory shall be > > also stored as reserved memory(bootinfo.reserved_mem), but being bind > > to one specific Xen domain node. > > - doc refinement > > --- > > docs/misc/arm/device-tree/booting.txt | 33 ++++++++++++++++++++++++ > > xen/arch/arm/bootfdt.c | 36 +++++++++++++++++++++++++-- > > xen/include/asm-arm/setup.h | 1 + > > 3 files changed, 68 insertions(+), 2 deletions(-) > > > > diff --git a/docs/misc/arm/device-tree/booting.txt > > b/docs/misc/arm/device-tree/booting.txt > > index 5243bc7fd3..95b20ddc3a 100644 > > --- a/docs/misc/arm/device-tree/booting.txt > > +++ b/docs/misc/arm/device-tree/booting.txt > > @@ -268,3 +268,36 @@ 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. > > + > > +Memory can be statically allocated to a domain using the property > > +"xen,static- mem" defined in the domain configuration. 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". > > + > > +Below is an example on how to specify the static memory region in the > > +device-tree: > > + > > + / { > > + 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>; > > + ... > > + }; > > + }; > > + }; > > + > > +This will reserve a 512MB region starting at the host physical > > +address > > +0x30000000 to be exclusively used by DomU1 > > This binding is OK. We might want to clarify what is the purpose of the > "memory" property when "xen,static-mem" is present. I would suggest to write > that when "xen,static-mem" is present, the "memory" property becomes > optional. Or even better: > > """ > When present, the xen,static-mem property supersedes the memory property. > """ > oh, "supersede" learned! ;) > > In the future when Xen will support direct mapping, I assume that we'll also > add a direct-map property to enable it. Something like: > > 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>; > direct-map; > ... > }; > > Maybe I would add a statement to clarify it that xen,static-mem doesn't > automatically imply direct mapping. Something like: > > """ > The static memory will be mapped in the guest at the usual guest memory > addresses (GUEST_RAM0_BASE, GUEST_RAM1_BASE) defined by > xen/include/public/arch-arm.h. > """ > Thanks for the detailed suggestion. I'll just take it. > The rest of the patch looks OK. One minor NIT below. > > > > > diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c index > > 8c81be3379..00f34eec58 100644 > > --- a/xen/arch/arm/bootfdt.c > > +++ b/xen/arch/arm/bootfdt.c > > @@ -66,7 +66,7 @@ void __init device_tree_get_reg(const __be32 **cell, > > u32 address_cells, static int __init device_tree_get_meminfo(const void *fdt, > int node, > > const char *prop_name, > > u32 address_cells, u32 size_cells, > > - void *data) > > + void *data, bool > > + xen_domain) > > { > > const struct fdt_property *prop; > > unsigned int i, banks; > > @@ -90,6 +90,7 @@ static int __init device_tree_get_meminfo(const void > *fdt, int node, > > continue; > > mem->bank[mem->nr_banks].start = start; > > mem->bank[mem->nr_banks].size = size; > > + mem->bank[mem->nr_banks].xen_domain = xen_domain; > > mem->nr_banks++; > > } > > > > @@ -184,7 +185,8 @@ static int __init process_memory_node(const void > *fdt, int node, > > return -EINVAL; > > } > > > > - return device_tree_get_meminfo(fdt, node, "reg", address_cells, > size_cells, data); > > + return device_tree_get_meminfo(fdt, node, "reg", address_cells, > size_cells, > > + data, false); > > } > > > > static int __init process_reserved_memory_node(const void *fdt, int > > node, @@ -338,6 +340,34 @@ static void __init process_chosen_node(const > void *fdt, int node, > > add_boot_module(BOOTMOD_RAMDISK, start, end-start, false); } > > > > +static int __init process_domain_node(const void *fdt, int node, > > + const char *name, > > + u32 address_cells, u32 > > +size_cells) { > > + const struct fdt_property *prop; > > + > > + printk("Checking for \"xen,static-mem\" in domain node\n"); > > + > > + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL); > > + if ( !prop ) > > + /* No "xen,static-mem" present. */ > > + return 0; > > + > > + 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 < 1 || size_cells < 1 ) > > + { > > + printk("fdt: node `%s': invalid #xen,static-mem-address-cells or > #xen,static-mem-size-cells", > > + name); > > + return -EINVAL; > > + } > > + > > + return device_tree_get_meminfo(fdt, node, "xen,static-mem", > address_cells, > > + size_cells, > > +&bootinfo.reserved_mem, true); } > > + > > static int __init early_scan_node(const void *fdt, > > int node, const char *name, int depth, > > u32 address_cells, u32 size_cells, > > @@ -356,6 +386,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 && device_tree_node_compatible(fdt, node, > "xen,domain") ) > > + rc = process_domain_node(fdt, node, name, address_cells, > > + size_cells); > > > > 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..6c3c16294b 100644 > > --- a/xen/include/asm-arm/setup.h > > +++ b/xen/include/asm-arm/setup.h > > @@ -24,6 +24,7 @@ typedef enum { > > struct membank { > > paddr_t start; > > paddr_t size; > > + bool xen_domain; /* whether memory bank is bind to Xen domain. */ > ^ a or the ^ bound to a > Sure. Will fix it. > > > }; > > > > struct meminfo { > > -- > > 2.25.1 > >
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 5243bc7fd3..95b20ddc3a 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -268,3 +268,36 @@ 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. + +Memory can be statically allocated to a domain using the property "xen,static- +mem" defined in the domain configuration. 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". + +Below is an example on how to specify the static memory region in the +device-tree: + + / { + 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>; + ... + }; + }; + }; + +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 8c81be3379..00f34eec58 100644 --- a/xen/arch/arm/bootfdt.c +++ b/xen/arch/arm/bootfdt.c @@ -66,7 +66,7 @@ void __init device_tree_get_reg(const __be32 **cell, u32 address_cells, static int __init device_tree_get_meminfo(const void *fdt, int node, const char *prop_name, u32 address_cells, u32 size_cells, - void *data) + void *data, bool xen_domain) { const struct fdt_property *prop; unsigned int i, banks; @@ -90,6 +90,7 @@ static int __init device_tree_get_meminfo(const void *fdt, int node, continue; mem->bank[mem->nr_banks].start = start; mem->bank[mem->nr_banks].size = size; + mem->bank[mem->nr_banks].xen_domain = xen_domain; mem->nr_banks++; } @@ -184,7 +185,8 @@ static int __init process_memory_node(const void *fdt, int node, return -EINVAL; } - return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, data); + return device_tree_get_meminfo(fdt, node, "reg", address_cells, size_cells, + data, false); } static int __init process_reserved_memory_node(const void *fdt, int node, @@ -338,6 +340,34 @@ static void __init process_chosen_node(const void *fdt, int node, add_boot_module(BOOTMOD_RAMDISK, start, end-start, false); } +static int __init process_domain_node(const void *fdt, int node, + const char *name, + u32 address_cells, u32 size_cells) +{ + const struct fdt_property *prop; + + printk("Checking for \"xen,static-mem\" in domain node\n"); + + prop = fdt_get_property(fdt, node, "xen,static-mem", NULL); + if ( !prop ) + /* No "xen,static-mem" present. */ + return 0; + + 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 < 1 || size_cells < 1 ) + { + printk("fdt: node `%s': invalid #xen,static-mem-address-cells or #xen,static-mem-size-cells", + name); + return -EINVAL; + } + + return device_tree_get_meminfo(fdt, node, "xen,static-mem", address_cells, + size_cells, &bootinfo.reserved_mem, true); +} + static int __init early_scan_node(const void *fdt, int node, const char *name, int depth, u32 address_cells, u32 size_cells, @@ -356,6 +386,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 && device_tree_node_compatible(fdt, node, "xen,domain") ) + rc = process_domain_node(fdt, node, name, address_cells, size_cells); 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..6c3c16294b 100644 --- a/xen/include/asm-arm/setup.h +++ b/xen/include/asm-arm/setup.h @@ -24,6 +24,7 @@ typedef enum { struct membank { paddr_t start; paddr_t size; + bool xen_domain; /* whether memory bank is bind to Xen domain. */ }; struct meminfo {
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. Memory can be statically allocated to a domain using the property "xen,static- mem" defined in the domain configuration. 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". This patch introduces this new `xen,static-mem` feature, and also documents and parses this new attribute at boot time. This patch also introduces a new field "bool xen_domain" in "struct membank" to tell the difference of one memory bank is reserved as the whole hardware resource, or bind to one specific xen domain node, through "xen,static-mem" Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- v5 changes: - check the node using the Xen domain binding whether contains the property "xen,static-mem", not the property itself - add "rc = ..." to get the error propagated - introduce new field "bool xen_domain", then static memory shall be also stored as reserved memory(bootinfo.reserved_mem), but being bind to one specific Xen domain node. - doc refinement --- docs/misc/arm/device-tree/booting.txt | 33 ++++++++++++++++++++++++ xen/arch/arm/bootfdt.c | 36 +++++++++++++++++++++++++-- xen/include/asm-arm/setup.h | 1 + 3 files changed, 68 insertions(+), 2 deletions(-)