Hi,
On 04/03/2022 17:46, Marco Solieri wrote:
From: Luca Miccio <lucmic...@gmail.com>
Add initialization for Xen coloring data. By default, use the lowest
color index available.
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.
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. The default amount of Xen colors is thus set to one.
Signed-off-by: Luca Miccio <206...@studenti.unimore.it>
Signed-off-by: Marco Solieri <marco.soli...@minervasys.tech>
---
xen/arch/arm/coloring.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/coloring.c b/xen/arch/arm/coloring.c
index d1ac193a80..761414fcd7 100644
--- a/xen/arch/arm/coloring.c
+++ b/xen/arch/arm/coloring.c
@@ -30,10 +30,18 @@
#include <asm/coloring.h>
#include <asm/io.h>
+/* By default Xen uses the lowestmost color */
+#define XEN_COLOR_DEFAULT_MASK 0x0001
You are setting a uint32_t value. So it should be 0x00000001.
But I think it is a bit confusing to define a mask here. Instead, I
would define the color ID and set the bit.
+#define XEN_COLOR_DEFAULT_NUM 1
+/* Current maximum useful colors */
+#define MAX_XEN_COLOR 128 > +
/* Number of color(s) assigned to Xen */
static uint32_t xen_col_num;
/* Coloring configuration of Xen as bitmask */
static uint32_t xen_col_mask[MAX_COLORS_CELLS];
+/* Xen colors IDs */
+static uint32_t xen_col_list[MAX_XEN_COLOR];
Why do we need to store xen colors in both a bitmask form and an array
of color ID?
/* Number of color(s) assigned to Dom0 */
static uint32_t dom0_col_num;
@@ -216,7 +224,7 @@ uint32_t get_max_colors(void)
bool __init coloring_init(void)
{
- int i;
+ int i, rc;
printk(XENLOG_INFO "Initialize XEN coloring: \n");
/*
@@ -266,6 +274,27 @@ bool __init coloring_init(void)
printk(XENLOG_INFO "Color bits in address: 0x%"PRIx64"\n", addr_col_mask);
printk(XENLOG_INFO "Max number of colors: %u\n", max_col_num);
+ if ( !xen_col_num )
+ {
+ xen_col_mask[0] = XEN_COLOR_DEFAULT_MASK;
+ xen_col_num = XEN_COLOR_DEFAULT_NUM;
+ printk(XENLOG_WARNING "Xen color configuration not found. Using
default\n");
+ }
+
+ printk(XENLOG_INFO "Xen color configuration:
0x%"PRIx32"%"PRIx32"%"PRIx32"%"PRIx32"\n",
+ xen_col_mask[3], xen_col_mask[2], xen_col_mask[1],
xen_col_mask[0]);
You are making the assumption that MAX_COLORS_CELLS is always 4. This
may be more or worse less. So this should be rework to avoid making any
assumption on the size.
I expect switching to the generic bitmask will help here.
+ rc = copy_mask_to_list(xen_col_mask, xen_col_list, xen_col_num);
+
+ if ( rc )
+ return false;
+
+ for ( i = 0; i < xen_col_num; i++ )
+ if ( xen_col_list[i] > (max_col_num - 1) )
+ {
+ printk(XENLOG_ERR "ERROR: max. color value not supported\n");
+ return false;
+ }
+
return true;
}
Cheers,
--
Julien Grall
