| Message ID | 20240129171811.21382-8-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm cache coloring | expand |
On 29.01.2024 18:18, Carlo Nonato wrote: > @@ -950,6 +951,11 @@ void __init create_domUs(void) > #endif > } > > + dt_property_read_string(node, "llc-colors", &llc_colors_str); > + if ( !llc_coloring_enabled && llc_colors_str) > + printk(XENLOG_WARNING > + "'llc-colors' found, but LLC coloring is disabled\n"); Why's this just a warning, when ... > @@ -960,6 +966,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); ... this results in panic()? > --- a/xen/common/llc-coloring.c > +++ b/xen/common/llc-coloring.c > @@ -254,6 +254,29 @@ 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) __init ? > +{ > + int err; > + unsigned int *colors; > + > + if ( !str ) > + return domain_set_default_colors(d); > + > + colors = alloc_colors(max_nr_colors); > + if ( !colors ) > + return -ENOMEM; > + > + err = parse_color_config(str, colors, max_nr_colors, &d->num_llc_colors); > + if ( err ) > + { > + printk(XENLOG_ERR "Error parsing LLC color configuration."); Nit: No full stop at the end of log messages please. > + return err; > + } > + d->llc_colors = colors; > + > + return domain_check_colors(d); Same ordering issue as in the earlier patch, I think. Jan
Hi Jan, On Thu, Feb 1, 2024 at 3:19 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 29.01.2024 18:18, Carlo Nonato wrote: > > @@ -950,6 +951,11 @@ void __init create_domUs(void) > > #endif > > } > > > > + dt_property_read_string(node, "llc-colors", &llc_colors_str); > > + if ( !llc_coloring_enabled && llc_colors_str) > > + printk(XENLOG_WARNING > > + "'llc-colors' found, but LLC coloring is disabled\n"); > > Why's this just a warning, when ... This informs the user that this configuration will be ignored, but the DomU can be constructed anyway... > > @@ -960,6 +966,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); > > ... this results in panic()? ... while here we can't continue because there's some error in the configuration and the DomU can't be constructed. Domains must have a valid coloring configuration. > > --- a/xen/common/llc-coloring.c > > +++ b/xen/common/llc-coloring.c > > @@ -254,6 +254,29 @@ 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) > > __init ? Yes. Thanks. > > +{ > > + int err; > > + unsigned int *colors; > > + > > + if ( !str ) > > + return domain_set_default_colors(d); > > + > > + colors = alloc_colors(max_nr_colors); > > + if ( !colors ) > > + return -ENOMEM; > > + > > + err = parse_color_config(str, colors, max_nr_colors, &d->num_llc_colors); > > + if ( err ) > > + { > > + printk(XENLOG_ERR "Error parsing LLC color configuration."); > > Nit: No full stop at the end of log messages please. > > > + return err; > > + } > > + d->llc_colors = colors; > > + > > + return domain_check_colors(d); > > Same ordering issue as in the earlier patch, I think. > > Jan
On 03.02.2024 12:43, Carlo Nonato wrote: > On Thu, Feb 1, 2024 at 3:19 PM Jan Beulich <jbeulich@suse.com> wrote: >> On 29.01.2024 18:18, Carlo Nonato wrote: >>> @@ -950,6 +951,11 @@ void __init create_domUs(void) >>> #endif >>> } >>> >>> + dt_property_read_string(node, "llc-colors", &llc_colors_str); >>> + if ( !llc_coloring_enabled && llc_colors_str) >>> + printk(XENLOG_WARNING >>> + "'llc-colors' found, but LLC coloring is disabled\n"); >> >> Why's this just a warning, when ... > > This informs the user that this configuration will be ignored, but the DomU can > be constructed anyway... Yet that's a violation of the principle that Julien had outlined when discussing whether to panic() in such cases. The property indicates to me that the domain ought to be run with coloring enabled, i.e. not much different from ... >>> @@ -960,6 +966,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); >> >> ... this results in panic()? > > ... while here we can't continue because there's some error in the > configuration and the DomU can't be constructed. Domains must have a valid > coloring configuration. ... the request not being possible to fulfill here. Jan
On 05/02/2024 10:39, Jan Beulich wrote: > > > On 03.02.2024 12:43, Carlo Nonato wrote: >> On Thu, Feb 1, 2024 at 3:19 PM Jan Beulich <jbeulich@suse.com> wrote: >>> On 29.01.2024 18:18, Carlo Nonato wrote: >>>> @@ -950,6 +951,11 @@ void __init create_domUs(void) >>>> #endif >>>> } >>>> >>>> + dt_property_read_string(node, "llc-colors", &llc_colors_str); >>>> + if ( !llc_coloring_enabled && llc_colors_str) >>>> + printk(XENLOG_WARNING >>>> + "'llc-colors' found, but LLC coloring is disabled\n"); >>> >>> Why's this just a warning, when ... >> >> This informs the user that this configuration will be ignored, but the DomU can >> be constructed anyway... > > Yet that's a violation of the principle that Julien had outlined when > discussing whether to panic() in such cases. The property indicates to > me that the domain ought to be run with coloring enabled, i.e. not much > different from ... > >>>> @@ -960,6 +966,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); >>> >>> ... this results in panic()? >> >> ... while here we can't continue because there's some error in the >> configuration and the DomU can't be constructed. Domains must have a valid >> coloring configuration. > > ... the request not being possible to fulfill here. +1 If the user requests a certain functionality which cannot be fulfilled, we shall panic. Take a look at e.g. sve, static-shmem, vpl011. ~Michal
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index bbd955e9c2..bbe49faadc 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/cache_coloring.rst for syntax. + - vpl011 An empty property to enable/disable a virtual pl011 for the guest to diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst index c347725525..bf055d7e7f 100644 --- a/docs/misc/cache-coloring.rst +++ b/docs/misc/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 ********** @@ -115,6 +115,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/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 992080e61a..efc1bbbc3e 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -807,6 +807,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) @@ -950,6 +951,11 @@ void __init create_domUs(void) #endif } + dt_property_read_string(node, "llc-colors", &llc_colors_str); + 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 @@ -960,6 +966,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/common/llc-coloring.c b/xen/common/llc-coloring.c index aaf0606c00..a932a61e0c 100644 --- a/xen/common/llc-coloring.c +++ b/xen/common/llc-coloring.c @@ -254,6 +254,29 @@ 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; + unsigned int *colors; + + if ( !str ) + return domain_set_default_colors(d); + + colors = alloc_colors(max_nr_colors); + if ( !colors ) + return -ENOMEM; + + err = parse_color_config(str, colors, max_nr_colors, &d->num_llc_colors); + if ( err ) + { + printk(XENLOG_ERR "Error parsing LLC color configuration."); + return err; + } + d->llc_colors = colors; + + return domain_check_colors(d); +} + /* * Local variables: * mode: C diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h index a82081367f..63785c8319 100644 --- a/xen/include/xen/llc-coloring.h +++ b/xen/include/xen/llc-coloring.h @@ -27,7 +27,7 @@ static inline void domain_dump_llc_colors(const struct domain *d) {} unsigned int get_llc_way_size(void); void arch_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); int domain_set_llc_colors_domctl(struct domain *d, const struct xen_domctl_set_llc_colors *config);