| Message ID | 20240315105902.160047-13-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm cache coloring | expand |
On 15.03.2024 11:59, Carlo Nonato wrote: > From: Luca Miccio <lucmiccio@gmail.com> > > Add a new command line parameter to configure Xen cache colors. > These colors can be dumped with the cache coloring info debug-key. > > By default, Xen uses the first color. > Benchmarking the VM interrupt response time provides an estimation of > LLC usage by Xen's most latency-critical runtime task. Results on Arm > Cortex-A53 on Xilinx Zynq UltraScale+ XCZU9EG show that one color, which > reserves 64 KiB of L2, is enough to attain best responsiveness: > - Xen 1 color latency: 3.1 us > - Xen 2 color latency: 3.1 us > > More colors are instead very likely to be needed on processors whose L1 > cache is physically-indexed and physically-tagged, such as Cortex-A57. > In such cases, coloring applies to L1 also, and there typically are two > distinct L1-colors. Therefore, reserving only one color for Xen would > senselessly partitions a cache memory that is already private, i.e. > underutilize it. Here you say that using just a single color is undesirable on such systems. > The default amount of Xen colors is thus set to one. Yet then, without any further explanation you conclude that 1 is the universal default. > @@ -147,6 +159,21 @@ void __init llc_coloring_init(void) > panic("Number of LLC colors (%u) not in range [2, %u]\n", > max_nr_colors, CONFIG_NR_LLC_COLORS); > > + if ( !xen_num_colors ) > + { > + unsigned int i; > + > + xen_num_colors = MIN(XEN_DEFAULT_NUM_COLORS, max_nr_colors); > + > + printk(XENLOG_WARNING > + "Xen LLC color config not found. Using first %u colors\n", > + xen_num_colors); > + for ( i = 0; i < xen_num_colors; i++ ) > + xen_colors[i] = i; > + } > + else if ( !check_colors(xen_colors, xen_num_colors) ) > + panic("Bad LLC color config for Xen\n"); This "else" branch again lacks a bounds check against max_nr_colors, if I'm not mistaken. Jan
Hi Jan On Tue, Mar 19, 2024 at 4:54 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 15.03.2024 11:59, Carlo Nonato wrote: > > From: Luca Miccio <lucmiccio@gmail.com> > > > > Add a new command line parameter to configure Xen cache colors. > > These colors can be dumped with the cache coloring info debug-key. > > > > By default, Xen uses the first color. > > Benchmarking the VM interrupt response time provides an estimation of > > LLC usage by Xen's most latency-critical runtime task. Results on Arm > > Cortex-A53 on Xilinx Zynq UltraScale+ XCZU9EG show that one color, which > > reserves 64 KiB of L2, is enough to attain best responsiveness: > > - Xen 1 color latency: 3.1 us > > - Xen 2 color latency: 3.1 us > > > > More colors are instead very likely to be needed on processors whose L1 > > cache is physically-indexed and physically-tagged, such as Cortex-A57. > > In such cases, coloring applies to L1 also, and there typically are two > > distinct L1-colors. Therefore, reserving only one color for Xen would > > senselessly partitions a cache memory that is already private, i.e. > > underutilize it. > > Here you say that using just a single color is undesirable on such systems. > > > The default amount of Xen colors is thus set to one. > > Yet then, without any further explanation you conclude that 1 is the > universal default. A single default that suits every need doesn't exist, but we know that 1 is good for the most widespread target we have (Cortex-A53). Having that said, I think that a simple reorder of the description, while also making it more explicit, solves the issue. > > @@ -147,6 +159,21 @@ void __init llc_coloring_init(void) > > panic("Number of LLC colors (%u) not in range [2, %u]\n", > > max_nr_colors, CONFIG_NR_LLC_COLORS); > > > > + if ( !xen_num_colors ) > > + { > > + unsigned int i; > > + > > + xen_num_colors = MIN(XEN_DEFAULT_NUM_COLORS, max_nr_colors); > > + > > + printk(XENLOG_WARNING > > + "Xen LLC color config not found. Using first %u colors\n", > > + xen_num_colors); > > + for ( i = 0; i < xen_num_colors; i++ ) > > + xen_colors[i] = i; > > + } > > + else if ( !check_colors(xen_colors, xen_num_colors) ) > > + panic("Bad LLC color config for Xen\n"); > > This "else" branch again lacks a bounds check against max_nr_colors, if > I'm not mistaken. Yep. > Jan Thanks.
diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst index 50b6d94ffc..f427a14b65 100644 --- a/docs/misc/cache-coloring.rst +++ b/docs/misc/cache-coloring.rst @@ -122,6 +122,8 @@ Specific documentation is available at `docs/misc/xen-command-line.pandoc`. +----------------------+-------------------------------+ | ``buddy-alloc-size`` | Buddy allocator reserved size | +----------------------+-------------------------------+ +| ``xen-llc-colors`` | Xen color configuration | ++----------------------+-------------------------------+ Colors selection format *********************** diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc index 461403362f..fa18ec942e 100644 --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -2885,6 +2885,16 @@ mode. **WARNING: `x2apic_phys` is deprecated and superseded by `x2apic-mode`. The latter takes precedence if both are set.** +### xen-llc-colors (arm64) +> `= List of [ <integer> | <integer>-<integer> ]` + +> Default: `0: the lowermost color` + +Specify Xen LLC color configuration. This options is available only when +`CONFIG_LLC_COLORING` is enabled. +Two colors are most likely needed on platforms where private caches are +physically indexed, e.g. the L1 instruction cache of the Arm Cortex-A57. + ### xenheap_megabytes (arm32) > `= <size>` diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c index e34ba6b6ec..f1a7561d79 100644 --- a/xen/common/llc-coloring.c +++ b/xen/common/llc-coloring.c @@ -9,6 +9,8 @@ #include <xen/llc-coloring.h> #include <xen/param.h> +#define XEN_DEFAULT_NUM_COLORS 1 + bool __ro_after_init llc_coloring_enabled; boolean_param("llc-coloring", llc_coloring_enabled); @@ -22,6 +24,9 @@ static unsigned int __ro_after_init max_nr_colors; static unsigned int __initdata dom0_colors[CONFIG_NR_LLC_COLORS]; static unsigned int __initdata dom0_num_colors; +static unsigned int __ro_after_init xen_colors[CONFIG_NR_LLC_COLORS]; +static unsigned int __ro_after_init xen_num_colors; + #define mfn_color_mask (max_nr_colors - 1) #define mfn_to_color(mfn) (mfn_x(mfn) & mfn_color_mask) @@ -79,6 +84,13 @@ static int __init parse_dom0_colors(const char *s) } custom_param("dom0-llc-colors", parse_dom0_colors); +static int __init parse_xen_colors(const char *s) +{ + return parse_color_config(s, xen_colors, ARRAY_SIZE(xen_colors), + &xen_num_colors); +} +custom_param("xen-llc-colors", parse_xen_colors); + static void print_colors(const unsigned int *colors, unsigned int num_colors) { unsigned int i; @@ -147,6 +159,21 @@ void __init llc_coloring_init(void) panic("Number of LLC colors (%u) not in range [2, %u]\n", max_nr_colors, CONFIG_NR_LLC_COLORS); + if ( !xen_num_colors ) + { + unsigned int i; + + xen_num_colors = MIN(XEN_DEFAULT_NUM_COLORS, max_nr_colors); + + printk(XENLOG_WARNING + "Xen LLC color config not found. Using first %u colors\n", + xen_num_colors); + for ( i = 0; i < xen_num_colors; i++ ) + xen_colors[i] = i; + } + else if ( !check_colors(xen_colors, xen_num_colors) ) + panic("Bad LLC color config for Xen\n"); + arch_llc_coloring_init(); } @@ -157,6 +184,8 @@ void cf_check dump_llc_coloring_info(void) printk("LLC coloring info:\n"); printk(" Number of LLC colors supported: %u\n", max_nr_colors); + printk(" Xen LLC colors (%u): ", xen_num_colors); + print_colors(xen_colors, xen_num_colors); } void cf_check domain_dump_llc_colors(const struct domain *d)