| Message ID | 20220304174701.1453977-23-marco.solieri@minervasys.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm cache coloring | expand |
Hi, On 04/03/2022 17:46, Marco Solieri wrote: > From: Luca Miccio <lucmiccio@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 <206497@studenti.unimore.it> > Signed-off-by: Marco Solieri <marco.solieri@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,
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 +#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]; /* 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]); + 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; }