| Message ID | 20240102095138.17933-7-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm cache coloring | expand |
Hi Carlo, On 02/01/2024 09:51, Carlo Nonato wrote: > This commit adds the "llc-colors" Device Tree attribute that can be used > for DomUs and Dom0less color configurations. The syntax is the same used > for every color config. > > Based on original work from: Luca Miccio <lucmiccio@gmail.com> > > Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech> > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > --- > v5: > - static-mem check has been moved in a previous patch > - added domain_set_llc_colors_from_str() to set colors after domain creation > --- > docs/misc/arm/cache-coloring.rst | 48 ++++++++++++++++++++++++- > docs/misc/arm/device-tree/booting.txt | 4 +++ > xen/arch/arm/dom0less-build.c | 13 +++++++ > xen/arch/arm/include/asm/llc-coloring.h | 1 + > xen/arch/arm/llc-coloring.c | 17 +++++++++ > 5 files changed, 82 insertions(+), 1 deletion(-) > > diff --git a/docs/misc/arm/cache-coloring.rst b/docs/misc/arm/cache-coloring.rst > index acf82c3df8..ae1dd8f4af 100644 > --- a/docs/misc/arm/cache-coloring.rst > +++ b/docs/misc/arm/cache-coloring.rst > @@ -10,7 +10,7 @@ If needed, change the maximum number of colors with > ``CONFIG_NR_LLC_COLORS=<n>``. > > Compile Xen and the toolstack and then configure it via > -`Command line parameters`_. > +`Command line parameters`_. For DomUs follow `DomUs configuration`_. > > Background > ********** > @@ -114,6 +114,52 @@ Examples: > | 0 | [0] | > +-------------------+-----------------------------+ > > +DomUs configuration > +******************* > + > +DomUs colors can be set via Device Tree, also for Dom0less configurations > +(documentation at `docs/misc/arm/device-tree/booting.txt`) using the > +``llc-colors`` option. For example: > + > +:: > + > + xen,xen-bootargs = "console=dtuart dtuart=serial0 dom0_mem=1G dom0_max_vcpus=1 sched=null llc-coloring=on llc-way-size=64K dom0-llc-colors=2-6"; > + xen,dom0-bootargs "console=hvc0 earlycon=xen earlyprintk=xen root=/dev/ram0" > + > + dom0 { > + compatible = "xen,linux-zimage" "xen,multiboot-module"; > + reg = <0x0 0x1000000 0x0 15858176>; > + }; > + > + dom0-ramdisk { > + compatible = "xen,linux-initrd" "xen,multiboot-module"; > + reg = <0x0 0x2000000 0x0 20638062>; > + }; > + > + domU0 { > + #address-cells = <0x1>; > + #size-cells = <0x1>; > + compatible = "xen,domain"; > + memory = <0x0 0x40000>; > + llc-colors = "4-8,10,11,12"; > + cpus = <0x1>; > + vpl011 = <0x1>; > + > + module@2000000 { > + compatible = "multiboot,kernel", "multiboot,module"; > + reg = <0x2000000 0xffffff>; > + bootargs = "console=ttyAMA0"; > + }; > + > + module@30000000 { > + compatible = "multiboot,ramdisk", "multiboot,module"; > + reg = <0x3000000 0xffffff>; > + }; > + }; > + > +**Note:** If no color configuration is provided for a domain, the default one, > +which corresponds to all available colors, is used instead. Looking at your code, it will use *all* the colors. Some of the colors might have been assigned to a domain. I am not entirely convinced this is good idea to allow this as a default setup. > + > Known issues and limitations > **************************** > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index bbd955e9c2..e9f9862e9c 100644 > --- a/docs/misc/arm/device-tree/booting.txt > +++ b/docs/misc/arm/device-tree/booting.txt > @@ -162,6 +162,10 @@ with the following properties: > > An integer specifying the number of vcpus to allocate to the guest. > > +- llc-colors > + A string specifying the LLC color configuration for the guest. > + Refer to "docs/misc/arm/cache_coloring.rst" for syntax. > + > - vpl011 > > An empty property to enable/disable a virtual pl011 for the guest to > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 1142f7f74a..eb39f5291f 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -850,6 +850,7 @@ void __init create_domUs(void) > struct dt_device_node *node; > const struct dt_device_node *cpupool_node, > *chosen = dt_find_node_by_path("/chosen"); > + const char *llc_colors_str = NULL; > > BUG_ON(chosen == NULL); > dt_for_each_child_node(chosen, node) > @@ -993,6 +994,13 @@ void __init create_domUs(void) > #endif > } > > + dt_property_read_string(node, "llc-colors", &llc_colors_str); > + if ( llc_coloring_enabled && !llc_colors_str ) > + panic("'llc-colors' is required when LLC coloring is enabled\n"); In the documentation you wrote: "**Note:** If no color configuration is provided for a domain, the default one,which corresponds to all available colors, is used instead." I interpret as you want to continue rather than panic-ing. That said, I much prefer the panic version. > + else if ( !llc_coloring_enabled && llc_colors_str) > + printk(XENLOG_WARNING > + "'llc-colors' found, but LLC coloring is disabled\n"); > + > /* > * The variable max_init_domid is initialized with zero, so here it's > * very important to use the pre-increment operator to call > @@ -1003,6 +1011,11 @@ void __init create_domUs(void) > panic("Error creating domain %s (rc = %ld)\n", > dt_node_name(node), PTR_ERR(d)); > > + if ( llc_coloring_enabled && > + (rc = domain_set_llc_colors_from_str(d, llc_colors_str)) ) > + panic("Error initializing LLC coloring for domain %s (rc = %d)\n", > + dt_node_name(node), rc); > + > d->is_console = true; > dt_device_set_used_by(node, d->domain_id); > > diff --git a/xen/arch/arm/include/asm/llc-coloring.h b/xen/arch/arm/include/asm/llc-coloring.h > index ee5551e3cc..5f9b0a8121 100644 > --- a/xen/arch/arm/include/asm/llc-coloring.h > +++ b/xen/arch/arm/include/asm/llc-coloring.h > @@ -15,6 +15,7 @@ > > bool __init llc_coloring_init(void); > int dom0_set_llc_colors(struct domain *d); > +int domain_set_llc_colors_from_str(struct domain *d, const char *str); > > #endif /* __ASM_ARM_COLORING_H__ */ > > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > index a08614ec36..d3de5f14cb 100644 > --- a/xen/arch/arm/llc-coloring.c > +++ b/xen/arch/arm/llc-coloring.c > @@ -295,6 +295,23 @@ int domain_set_llc_colors_domctl(struct domain *d, > return domain_check_colors(d); > } > > +int domain_set_llc_colors_from_str(struct domain *d, const char *str) > +{ > + int err; > + > + if ( domain_alloc_colors(d, nr_colors) ) This code is somewhat confusing and would deserve some explanation. AFAICT, you are allocating a large array because parse_color_config() expects an array of nr_colors. d->num_llc_colors will also be set to nr_colors but then overriden by parse_color_config(). It feels to me that maybe set num_llc_colors() in domain_alloc_colors() is not right. > + return -ENOMEM; > + > + err = parse_color_config(str, d->llc_colors, &d->num_llc_colors); > + if ( err ) > + { > + printk(XENLOG_ERR "Error parsing LLC color configuration."); > + return err; > + } > + > + return domain_check_colors(d); > +} > + > /* > * Local variables: > * mode: C Cheers,
Hi Julien, On Fri, Jan 5, 2024 at 6:38 PM Julien Grall <julien@xen.org> wrote: > > Hi Carlo, > > On 02/01/2024 09:51, Carlo Nonato wrote: > > This commit adds the "llc-colors" Device Tree attribute that can be used > > for DomUs and Dom0less color configurations. The syntax is the same used > > for every color config. > > > > Based on original work from: Luca Miccio <lucmiccio@gmail.com> > > > > Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech> > > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > > --- > > v5: > > - static-mem check has been moved in a previous patch > > - added domain_set_llc_colors_from_str() to set colors after domain creation > > --- > > docs/misc/arm/cache-coloring.rst | 48 ++++++++++++++++++++++++- > > docs/misc/arm/device-tree/booting.txt | 4 +++ > > xen/arch/arm/dom0less-build.c | 13 +++++++ > > xen/arch/arm/include/asm/llc-coloring.h | 1 + > > xen/arch/arm/llc-coloring.c | 17 +++++++++ > > 5 files changed, 82 insertions(+), 1 deletion(-) > > > > diff --git a/docs/misc/arm/cache-coloring.rst b/docs/misc/arm/cache-coloring.rst > > index acf82c3df8..ae1dd8f4af 100644 > > --- a/docs/misc/arm/cache-coloring.rst > > +++ b/docs/misc/arm/cache-coloring.rst > > @@ -10,7 +10,7 @@ If needed, change the maximum number of colors with > > ``CONFIG_NR_LLC_COLORS=<n>``. > > > > Compile Xen and the toolstack and then configure it via > > -`Command line parameters`_. > > +`Command line parameters`_. For DomUs follow `DomUs configuration`_. > > > > Background > > ********** > > @@ -114,6 +114,52 @@ Examples: > > | 0 | [0] | > > +-------------------+-----------------------------+ > > > > +DomUs configuration > > +******************* > > + > > +DomUs colors can be set via Device Tree, also for Dom0less configurations > > +(documentation at `docs/misc/arm/device-tree/booting.txt`) using the > > +``llc-colors`` option. For example: > > + > > +:: > > + > > + xen,xen-bootargs = "console=dtuart dtuart=serial0 dom0_mem=1G dom0_max_vcpus=1 sched=null llc-coloring=on llc-way-size=64K dom0-llc-colors=2-6"; > > + xen,dom0-bootargs "console=hvc0 earlycon=xen earlyprintk=xen root=/dev/ram0" > > + > > + dom0 { > > + compatible = "xen,linux-zimage" "xen,multiboot-module"; > > + reg = <0x0 0x1000000 0x0 15858176>; > > + }; > > + > > + dom0-ramdisk { > > + compatible = "xen,linux-initrd" "xen,multiboot-module"; > > + reg = <0x0 0x2000000 0x0 20638062>; > > + }; > > + > > + domU0 { > > + #address-cells = <0x1>; > > + #size-cells = <0x1>; > > + compatible = "xen,domain"; > > + memory = <0x0 0x40000>; > > + llc-colors = "4-8,10,11,12"; > > + cpus = <0x1>; > > + vpl011 = <0x1>; > > + > > + module@2000000 { > > + compatible = "multiboot,kernel", "multiboot,module"; > > + reg = <0x2000000 0xffffff>; > > + bootargs = "console=ttyAMA0"; > > + }; > > + > > + module@30000000 { > > + compatible = "multiboot,ramdisk", "multiboot,module"; > > + reg = <0x3000000 0xffffff>; > > + }; > > + }; > > + > > +**Note:** If no color configuration is provided for a domain, the default one, > > +which corresponds to all available colors, is used instead. > > Looking at your code, it will use *all* the colors. Some of the colors > might have been assigned to a domain. I am not entirely convinced this > is good idea to allow this as a default setup. > > > > + > > Known issues and limitations > > **************************** > > > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > > index bbd955e9c2..e9f9862e9c 100644 > > --- a/docs/misc/arm/device-tree/booting.txt > > +++ b/docs/misc/arm/device-tree/booting.txt > > @@ -162,6 +162,10 @@ with the following properties: > > > > An integer specifying the number of vcpus to allocate to the guest. > > > > +- llc-colors > > + A string specifying the LLC color configuration for the guest. > > + Refer to "docs/misc/arm/cache_coloring.rst" for syntax. > > + > > - vpl011 > > > > An empty property to enable/disable a virtual pl011 for the guest to > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > > index 1142f7f74a..eb39f5291f 100644 > > --- a/xen/arch/arm/dom0less-build.c > > +++ b/xen/arch/arm/dom0less-build.c > > @@ -850,6 +850,7 @@ void __init create_domUs(void) > > struct dt_device_node *node; > > const struct dt_device_node *cpupool_node, > > *chosen = dt_find_node_by_path("/chosen"); > > + const char *llc_colors_str = NULL; > > > > BUG_ON(chosen == NULL); > > dt_for_each_child_node(chosen, node) > > @@ -993,6 +994,13 @@ void __init create_domUs(void) > > #endif > > } > > > > + dt_property_read_string(node, "llc-colors", &llc_colors_str); > > + if ( llc_coloring_enabled && !llc_colors_str ) > > + panic("'llc-colors' is required when LLC coloring is enabled\n"); > > In the documentation you wrote: > > "**Note:** If no color configuration is provided for a domain, the > default one,which corresponds to all available colors, is used instead." > > I interpret as you want to continue rather than panic-ing. That said, I > much prefer the panic version. Will do as you said. No problem from my side. > > + else if ( !llc_coloring_enabled && llc_colors_str) > > + printk(XENLOG_WARNING > > + "'llc-colors' found, but LLC coloring is disabled\n"); > > + > > /* > > * The variable max_init_domid is initialized with zero, so here it's > > * very important to use the pre-increment operator to call > > @@ -1003,6 +1011,11 @@ void __init create_domUs(void) > > panic("Error creating domain %s (rc = %ld)\n", > > dt_node_name(node), PTR_ERR(d)); > > > > + if ( llc_coloring_enabled && > > + (rc = domain_set_llc_colors_from_str(d, llc_colors_str)) ) > > + panic("Error initializing LLC coloring for domain %s (rc = %d)\n", > > + dt_node_name(node), rc); > > + > > d->is_console = true; > > dt_device_set_used_by(node, d->domain_id); > > > > diff --git a/xen/arch/arm/include/asm/llc-coloring.h b/xen/arch/arm/include/asm/llc-coloring.h > > index ee5551e3cc..5f9b0a8121 100644 > > --- a/xen/arch/arm/include/asm/llc-coloring.h > > +++ b/xen/arch/arm/include/asm/llc-coloring.h > > @@ -15,6 +15,7 @@ > > > > bool __init llc_coloring_init(void); > > int dom0_set_llc_colors(struct domain *d); > > +int domain_set_llc_colors_from_str(struct domain *d, const char *str); > > > > #endif /* __ASM_ARM_COLORING_H__ */ > > > > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > > index a08614ec36..d3de5f14cb 100644 > > --- a/xen/arch/arm/llc-coloring.c > > +++ b/xen/arch/arm/llc-coloring.c > > @@ -295,6 +295,23 @@ int domain_set_llc_colors_domctl(struct domain *d, > > return domain_check_colors(d); > > } > > > > +int domain_set_llc_colors_from_str(struct domain *d, const char *str) > > +{ > > + int err; > > + > > + if ( domain_alloc_colors(d, nr_colors) ) > > This code is somewhat confusing and would deserve some explanation. > AFAICT, you are allocating a large array because parse_color_config() > expects an array of nr_colors. > > d->num_llc_colors will also be set to nr_colors but then overriden by > parse_color_config(). > > It feels to me that maybe set num_llc_colors() in domain_alloc_colors() > is not right. As previously discussed, I can move the d->num_llc_colors assignment out of that function. > > + return -ENOMEM; > > + > > + err = parse_color_config(str, d->llc_colors, &d->num_llc_colors); > > + if ( err ) > > + { > > + printk(XENLOG_ERR "Error parsing LLC color configuration."); > > + return err; > > + } > > + > > + return domain_check_colors(d); > > +} > > + > > /* > > * Local variables: > > * mode: C > > Cheers, Thanks. > > -- > Julien Grall
diff --git a/docs/misc/arm/cache-coloring.rst b/docs/misc/arm/cache-coloring.rst index acf82c3df8..ae1dd8f4af 100644 --- a/docs/misc/arm/cache-coloring.rst +++ b/docs/misc/arm/cache-coloring.rst @@ -10,7 +10,7 @@ If needed, change the maximum number of colors with ``CONFIG_NR_LLC_COLORS=<n>``. Compile Xen and the toolstack and then configure it via -`Command line parameters`_. +`Command line parameters`_. For DomUs follow `DomUs configuration`_. Background ********** @@ -114,6 +114,52 @@ Examples: | 0 | [0] | +-------------------+-----------------------------+ +DomUs configuration +******************* + +DomUs colors can be set via Device Tree, also for Dom0less configurations +(documentation at `docs/misc/arm/device-tree/booting.txt`) using the +``llc-colors`` option. For example: + +:: + + xen,xen-bootargs = "console=dtuart dtuart=serial0 dom0_mem=1G dom0_max_vcpus=1 sched=null llc-coloring=on llc-way-size=64K dom0-llc-colors=2-6"; + xen,dom0-bootargs "console=hvc0 earlycon=xen earlyprintk=xen root=/dev/ram0" + + dom0 { + compatible = "xen,linux-zimage" "xen,multiboot-module"; + reg = <0x0 0x1000000 0x0 15858176>; + }; + + dom0-ramdisk { + compatible = "xen,linux-initrd" "xen,multiboot-module"; + reg = <0x0 0x2000000 0x0 20638062>; + }; + + domU0 { + #address-cells = <0x1>; + #size-cells = <0x1>; + compatible = "xen,domain"; + memory = <0x0 0x40000>; + llc-colors = "4-8,10,11,12"; + cpus = <0x1>; + vpl011 = <0x1>; + + module@2000000 { + compatible = "multiboot,kernel", "multiboot,module"; + reg = <0x2000000 0xffffff>; + bootargs = "console=ttyAMA0"; + }; + + module@30000000 { + compatible = "multiboot,ramdisk", "multiboot,module"; + reg = <0x3000000 0xffffff>; + }; + }; + +**Note:** If no color configuration is provided for a domain, the default one, +which corresponds to all available colors, is used instead. + Known issues and limitations **************************** diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index bbd955e9c2..e9f9862e9c 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -162,6 +162,10 @@ with the following properties: An integer specifying the number of vcpus to allocate to the guest. +- llc-colors + A string specifying the LLC color configuration for the guest. + Refer to "docs/misc/arm/cache_coloring.rst" for syntax. + - vpl011 An empty property to enable/disable a virtual pl011 for the guest to diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 1142f7f74a..eb39f5291f 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -850,6 +850,7 @@ void __init create_domUs(void) struct dt_device_node *node; const struct dt_device_node *cpupool_node, *chosen = dt_find_node_by_path("/chosen"); + const char *llc_colors_str = NULL; BUG_ON(chosen == NULL); dt_for_each_child_node(chosen, node) @@ -993,6 +994,13 @@ void __init create_domUs(void) #endif } + dt_property_read_string(node, "llc-colors", &llc_colors_str); + if ( llc_coloring_enabled && !llc_colors_str ) + panic("'llc-colors' is required when LLC coloring is enabled\n"); + else if ( !llc_coloring_enabled && llc_colors_str) + printk(XENLOG_WARNING + "'llc-colors' found, but LLC coloring is disabled\n"); + /* * The variable max_init_domid is initialized with zero, so here it's * very important to use the pre-increment operator to call @@ -1003,6 +1011,11 @@ void __init create_domUs(void) panic("Error creating domain %s (rc = %ld)\n", dt_node_name(node), PTR_ERR(d)); + if ( llc_coloring_enabled && + (rc = domain_set_llc_colors_from_str(d, llc_colors_str)) ) + panic("Error initializing LLC coloring for domain %s (rc = %d)\n", + dt_node_name(node), rc); + d->is_console = true; dt_device_set_used_by(node, d->domain_id); diff --git a/xen/arch/arm/include/asm/llc-coloring.h b/xen/arch/arm/include/asm/llc-coloring.h index ee5551e3cc..5f9b0a8121 100644 --- a/xen/arch/arm/include/asm/llc-coloring.h +++ b/xen/arch/arm/include/asm/llc-coloring.h @@ -15,6 +15,7 @@ bool __init llc_coloring_init(void); int dom0_set_llc_colors(struct domain *d); +int domain_set_llc_colors_from_str(struct domain *d, const char *str); #endif /* __ASM_ARM_COLORING_H__ */ diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c index a08614ec36..d3de5f14cb 100644 --- a/xen/arch/arm/llc-coloring.c +++ b/xen/arch/arm/llc-coloring.c @@ -295,6 +295,23 @@ int domain_set_llc_colors_domctl(struct domain *d, return domain_check_colors(d); } +int domain_set_llc_colors_from_str(struct domain *d, const char *str) +{ + int err; + + if ( domain_alloc_colors(d, nr_colors) ) + return -ENOMEM; + + err = parse_color_config(str, d->llc_colors, &d->num_llc_colors); + if ( err ) + { + printk(XENLOG_ERR "Error parsing LLC color configuration."); + return err; + } + + return domain_check_colors(d); +} + /* * Local variables: * mode: C