[PATCH v10 04/12] xen/arm: add Dom0 cache coloring support

Carlo Nonato posted 12 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v10 04/12] xen/arm: add Dom0 cache coloring support
Posted by Carlo Nonato 1 month, 1 week ago
Add a command line parameter to allow the user to set the coloring
configuration for Dom0.
A common configuration syntax for cache colors is introduced and
documented.
Take the opportunity to also add:
 - default configuration notion.
 - function to check well-formed configurations.

Direct mapping Dom0 isn't possible when coloring is enabled, so
CDF_directmap flag is removed when creating it.

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>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v10:
- fixed array type for colors parameter in check_colors()
v9:
- moved domain_llc_coloring_free() in next patch cause it's never used for dom0
v8:
- added bound check on dom0_num_colors
- default colors array set just once
v7:
- parse_color_config() doesn't accept leading/trailing commas anymore
- removed alloc_colors() helper
v6:
- moved domain_llc_coloring_free() in this patch
- removed domain_alloc_colors() in favor of a more explicit allocation
- parse_color_config() now accepts the size of the array to be filled
- allocate_memory() moved in another patch
v5:
- Carlo Nonato as the new author
- moved dom0 colors parsing (parse_colors()) in this patch
- added dom0_set_llc_colors() to set dom0 colors after creation
- moved color allocation and checking in this patch
- error handling when allocating color arrays
- FIXME: copy pasted allocate_memory() cause it got moved
v4:
- dom0 colors are dynamically allocated as for any other domain
  (colors are duplicated in dom0_colors and in the new array, but logic
  is simpler)
---
 docs/misc/cache-coloring.rst      |  29 ++++++++
 docs/misc/xen-command-line.pandoc |   9 +++
 xen/arch/arm/domain_build.c       |  10 ++-
 xen/common/llc-coloring.c         | 119 +++++++++++++++++++++++++++++-
 xen/include/xen/llc-coloring.h    |   1 +
 5 files changed, 166 insertions(+), 2 deletions(-)

diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst
index b608284e9b..c5fb33996c 100644
--- a/docs/misc/cache-coloring.rst
+++ b/docs/misc/cache-coloring.rst
@@ -105,6 +105,35 @@ Specific documentation is available at `docs/misc/xen-command-line.pandoc`.
 +----------------------+-------------------------------+
 | ``llc-nr-ways``      | Set the LLC number of ways    |
 +----------------------+-------------------------------+
+| ``dom0-llc-colors``  | Dom0 color configuration      |
++----------------------+-------------------------------+
+
+Colors selection format
+***********************
+
+Regardless of the memory pool that has to be colored (Xen, Dom0/DomUs),
+the color selection can be expressed using the same syntax. In particular a
+comma-separated list of colors or ranges of colors is used.
+Ranges are hyphen-separated intervals (such as `0-4`) and are inclusive on both
+sides.
+
+Note that:
+
+- no spaces are allowed between values.
+- no overlapping ranges or duplicated colors are allowed.
+- values must be written in ascending order.
+
+Examples:
+
++-------------------+-----------------------------+
+| **Configuration** | **Actual selection**        |
++-------------------+-----------------------------+
+| 1-2,5-8           | [1, 2, 5, 6, 7, 8]          |
++-------------------+-----------------------------+
+| 4-8,10,11,12      | [4, 5, 6, 7, 8, 10, 11, 12] |
++-------------------+-----------------------------+
+| 0                 | [0]                         |
++-------------------+-----------------------------+
 
 Auto-probing of LLC specs
 #########################
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index abd8dae96f..bfdc8b0002 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -963,6 +963,15 @@ Controls for the dom0 IOMMU setup.
 
 Specify a list of IO ports to be excluded from dom0 access.
 
+### dom0-llc-colors (arm64)
+> `= List of [ <integer> | <integer>-<integer> ]`
+
+> Default: `All available LLC colors`
+
+Specify dom0 LLC color configuration. This option is available only when
+`CONFIG_LLC_COLORING` is enabled. If the parameter is not set, all available
+colors are used.
+
 ### dom0_max_vcpus
 
 Either:
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index a95376dcdc..01aa9016f5 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2,6 +2,7 @@
 #include <xen/init.h>
 #include <xen/compile.h>
 #include <xen/lib.h>
+#include <xen/llc-coloring.h>
 #include <xen/mm.h>
 #include <xen/param.h>
 #include <xen/domain_page.h>
@@ -2284,6 +2285,7 @@ void __init create_dom0(void)
         .max_maptrack_frames = -1,
         .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
     };
+    unsigned int flags = CDF_privileged;
     int rc;
 
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
@@ -2311,10 +2313,16 @@ void __init create_dom0(void)
             panic("SVE vector length error\n");
     }
 
-    dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
+    if ( !llc_coloring_enabled )
+        flags |= CDF_directmap;
+
+    dom0 = domain_create(0, &dom0_cfg, flags);
     if ( IS_ERR(dom0) )
         panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
 
+    if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
+        panic("Error initializing LLC coloring for domain 0 (rc = %d)", rc);
+
     if ( alloc_dom0_vcpu0(dom0) == NULL )
         panic("Error creating domain 0 vcpu0\n");
 
diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
index 45b001b105..740b5b9e4f 100644
--- a/xen/common/llc-coloring.c
+++ b/xen/common/llc-coloring.c
@@ -20,6 +20,66 @@ static unsigned int __initdata llc_nr_ways;
 integer_param("llc-nr-ways", llc_nr_ways);
 /* Number of colors available in the LLC */
 static unsigned int __ro_after_init max_nr_colors;
+/* Default coloring configuration */
+static unsigned int __ro_after_init default_colors[NR_LLC_COLORS];
+
+static unsigned int __initdata dom0_colors[NR_LLC_COLORS];
+static unsigned int __initdata dom0_num_colors;
+
+/*
+ * Parse the coloring configuration given in the buf string, following the
+ * syntax below.
+ *
+ * COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
+ * RANGE               ::= COLOR-COLOR
+ *
+ * Example: "0,2-6,15-16" represents the set of colors: 0,2,3,4,5,6,15,16.
+ */
+static int __init parse_color_config(const char *buf, unsigned int colors[],
+                                     unsigned int max_num_colors,
+                                     unsigned int *num_colors)
+{
+    const char *s = buf;
+
+    *num_colors = 0;
+
+    while ( *s != '\0' )
+    {
+        unsigned int color, start, end;
+
+        start = simple_strtoul(s, &s, 0);
+
+        if ( *s == '-' )    /* Range */
+        {
+            s++;
+            end = simple_strtoul(s, &s, 0);
+        }
+        else                /* Single value */
+            end = start;
+
+        if ( start > end || (end - start) > (UINT_MAX - *num_colors) ||
+             (*num_colors + (end - start)) >= max_num_colors )
+            return -EINVAL;
+
+        /* Colors are range checked in check_colors() */
+        for ( color = start; color <= end; color++ )
+            colors[(*num_colors)++] = color;
+
+        if ( *s == ',' )
+            s++;
+        else if ( *s != '\0' )
+            break;
+    }
+
+    return *s ? -EINVAL : 0;
+}
+
+static int __init parse_dom0_colors(const char *s)
+{
+    return parse_color_config(s, dom0_colors, ARRAY_SIZE(dom0_colors),
+                              &dom0_num_colors);
+}
+custom_param("dom0-llc-colors", parse_dom0_colors);
 
 static void print_colors(const unsigned int colors[], unsigned int num_colors)
 {
@@ -44,9 +104,26 @@ static void print_colors(const unsigned int colors[], unsigned int num_colors)
     printk(" }\n");
 }
 
+static bool __init check_colors(const unsigned int colors[],
+                                unsigned int num_colors)
+{
+    unsigned int i;
+
+    for ( i = 0; i < num_colors; i++ )
+    {
+        if ( colors[i] >= max_nr_colors )
+        {
+            printk(XENLOG_ERR "LLC color %u >= %u\n", colors[i], max_nr_colors);
+            return false;
+        }
+    }
+
+    return true;
+}
+
 void __init llc_coloring_init(void)
 {
-    unsigned int way_size;
+    unsigned int way_size, i;
 
     if ( llc_size && llc_nr_ways )
     {
@@ -83,6 +160,9 @@ void __init llc_coloring_init(void)
     } else if ( max_nr_colors < 2 )
         panic("Number of LLC colors %u < 2\n", max_nr_colors);
 
+    for ( i = 0; i < max_nr_colors; i++ )
+        default_colors[i] = i;
+
     arch_llc_coloring_init();
 }
 
@@ -104,6 +184,43 @@ void domain_dump_llc_colors(const struct domain *d)
     print_colors(d->llc_colors, d->num_llc_colors);
 }
 
+static void __init domain_set_default_colors(struct domain *d)
+{
+    printk(XENLOG_WARNING
+           "LLC color config not found for %pd, using all colors\n", d);
+
+    d->llc_colors = default_colors;
+    d->num_llc_colors = max_nr_colors;
+}
+
+int __init dom0_set_llc_colors(struct domain *d)
+{
+    typeof(*dom0_colors) *colors;
+
+    if ( !dom0_num_colors )
+    {
+        domain_set_default_colors(d);
+        return 0;
+    }
+
+    if ( dom0_num_colors > max_nr_colors ||
+         !check_colors(dom0_colors, dom0_num_colors) )
+    {
+        printk(XENLOG_ERR "%pd:  bad LLC color config\n", d);
+        return -EINVAL;
+    }
+
+    colors = xmalloc_array(typeof(*dom0_colors), dom0_num_colors);
+    if ( !colors )
+        return -ENOMEM;
+
+    memcpy(colors, dom0_colors, sizeof(*colors) * dom0_num_colors);
+    d->llc_colors = colors;
+    d->num_llc_colors = dom0_num_colors;
+
+    return 0;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h
index ad2525e153..9cf49f574e 100644
--- a/xen/include/xen/llc-coloring.h
+++ b/xen/include/xen/llc-coloring.h
@@ -27,6 +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);
 
 #endif /* __COLORING_H__ */
 
-- 
2.43.0
Re: [PATCH v10 04/12] xen/arm: add Dom0 cache coloring support
Posted by Michal Orzel 4 weeks ago

On 19/11/2024 15:13, Carlo Nonato wrote:
> 
> 
> Add a command line parameter to allow the user to set the coloring
> configuration for Dom0.
> A common configuration syntax for cache colors is introduced and
> documented.
> Take the opportunity to also add:
>  - default configuration notion.
>  - function to check well-formed configurations.
> 
> Direct mapping Dom0 isn't possible when coloring is enabled, so
> CDF_directmap flag is removed when creating it.
> 
> 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>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> v10:
> - fixed array type for colors parameter in check_colors()
> v9:
> - moved domain_llc_coloring_free() in next patch cause it's never used for dom0
> v8:
> - added bound check on dom0_num_colors
> - default colors array set just once
> v7:
> - parse_color_config() doesn't accept leading/trailing commas anymore
> - removed alloc_colors() helper
> v6:
> - moved domain_llc_coloring_free() in this patch
> - removed domain_alloc_colors() in favor of a more explicit allocation
> - parse_color_config() now accepts the size of the array to be filled
> - allocate_memory() moved in another patch
> v5:
> - Carlo Nonato as the new author
> - moved dom0 colors parsing (parse_colors()) in this patch
> - added dom0_set_llc_colors() to set dom0 colors after creation
> - moved color allocation and checking in this patch
> - error handling when allocating color arrays
> - FIXME: copy pasted allocate_memory() cause it got moved
> v4:
> - dom0 colors are dynamically allocated as for any other domain
>   (colors are duplicated in dom0_colors and in the new array, but logic
>   is simpler)
> ---
>  docs/misc/cache-coloring.rst      |  29 ++++++++
>  docs/misc/xen-command-line.pandoc |   9 +++
>  xen/arch/arm/domain_build.c       |  10 ++-
>  xen/common/llc-coloring.c         | 119 +++++++++++++++++++++++++++++-
>  xen/include/xen/llc-coloring.h    |   1 +
>  5 files changed, 166 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst
> index b608284e9b..c5fb33996c 100644
> --- a/docs/misc/cache-coloring.rst
> +++ b/docs/misc/cache-coloring.rst
> @@ -105,6 +105,35 @@ Specific documentation is available at `docs/misc/xen-command-line.pandoc`.
>  +----------------------+-------------------------------+
>  | ``llc-nr-ways``      | Set the LLC number of ways    |
>  +----------------------+-------------------------------+
> +| ``dom0-llc-colors``  | Dom0 color configuration      |
> ++----------------------+-------------------------------+
> +
> +Colors selection format
> +***********************
> +
> +Regardless of the memory pool that has to be colored (Xen, Dom0/DomUs),
> +the color selection can be expressed using the same syntax. In particular a
> +comma-separated list of colors or ranges of colors is used.
> +Ranges are hyphen-separated intervals (such as `0-4`) and are inclusive on both
> +sides.
> +
> +Note that:
> +
> +- no spaces are allowed between values.
> +- no overlapping ranges or duplicated colors are allowed.
> +- values must be written in ascending order.
> +
> +Examples:
> +
> ++-------------------+-----------------------------+
> +| **Configuration** | **Actual selection**        |
> ++-------------------+-----------------------------+
> +| 1-2,5-8           | [1, 2, 5, 6, 7, 8]          |
> ++-------------------+-----------------------------+
> +| 4-8,10,11,12      | [4, 5, 6, 7, 8, 10, 11, 12] |
> ++-------------------+-----------------------------+
> +| 0                 | [0]                         |
> ++-------------------+-----------------------------+
> 
>  Auto-probing of LLC specs
>  #########################
> diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
> index abd8dae96f..bfdc8b0002 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -963,6 +963,15 @@ Controls for the dom0 IOMMU setup.
> 
>  Specify a list of IO ports to be excluded from dom0 access.
> 
> +### dom0-llc-colors (arm64)
> +> `= List of [ <integer> | <integer>-<integer> ]`
> +
> +> Default: `All available LLC colors`
> +
> +Specify dom0 LLC color configuration. This option is available only when
> +`CONFIG_LLC_COLORING` is enabled. If the parameter is not set, all available
> +colors are used.
> +
>  ### dom0_max_vcpus
> 
>  Either:
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index a95376dcdc..01aa9016f5 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2,6 +2,7 @@
>  #include <xen/init.h>
>  #include <xen/compile.h>
>  #include <xen/lib.h>
> +#include <xen/llc-coloring.h>
>  #include <xen/mm.h>
>  #include <xen/param.h>
>  #include <xen/domain_page.h>
> @@ -2284,6 +2285,7 @@ void __init create_dom0(void)
>          .max_maptrack_frames = -1,
>          .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
>      };
> +    unsigned int flags = CDF_privileged;
>      int rc;
> 
>      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> @@ -2311,10 +2313,16 @@ void __init create_dom0(void)
>              panic("SVE vector length error\n");
>      }
> 
> -    dom0 = domain_create(0, &dom0_cfg, CDF_privileged | CDF_directmap);
> +    if ( !llc_coloring_enabled )
> +        flags |= CDF_directmap;
> +
> +    dom0 = domain_create(0, &dom0_cfg, flags);
>      if ( IS_ERR(dom0) )
>          panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
> 
> +    if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
> +        panic("Error initializing LLC coloring for domain 0 (rc = %d)", rc);
Missing newline \n

> +
>      if ( alloc_dom0_vcpu0(dom0) == NULL )
>          panic("Error creating domain 0 vcpu0\n");
> 
> diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
> index 45b001b105..740b5b9e4f 100644
> --- a/xen/common/llc-coloring.c
> +++ b/xen/common/llc-coloring.c
> @@ -20,6 +20,66 @@ static unsigned int __initdata llc_nr_ways;
>  integer_param("llc-nr-ways", llc_nr_ways);
>  /* Number of colors available in the LLC */
>  static unsigned int __ro_after_init max_nr_colors;
> +/* Default coloring configuration */
> +static unsigned int __ro_after_init default_colors[NR_LLC_COLORS];
> +
> +static unsigned int __initdata dom0_colors[NR_LLC_COLORS];
> +static unsigned int __initdata dom0_num_colors;
> +
> +/*
> + * Parse the coloring configuration given in the buf string, following the
> + * syntax below.
> + *
> + * COLOR_CONFIGURATION ::= COLOR | RANGE,...,COLOR | RANGE
> + * RANGE               ::= COLOR-COLOR
> + *
> + * Example: "0,2-6,15-16" represents the set of colors: 0,2,3,4,5,6,15,16.
> + */
> +static int __init parse_color_config(const char *buf, unsigned int colors[],
> +                                     unsigned int max_num_colors,
> +                                     unsigned int *num_colors)
> +{
> +    const char *s = buf;
> +
> +    *num_colors = 0;
> +
> +    while ( *s != '\0' )
> +    {
> +        unsigned int color, start, end;
> +
> +        start = simple_strtoul(s, &s, 0);
> +
> +        if ( *s == '-' )    /* Range */
> +        {
> +            s++;
> +            end = simple_strtoul(s, &s, 0);
> +        }
> +        else                /* Single value */
> +            end = start;
> +
> +        if ( start > end || (end - start) > (UINT_MAX - *num_colors) ||
> +             (*num_colors + (end - start)) >= max_num_colors )
> +            return -EINVAL;
> +
> +        /* Colors are range checked in check_colors() */
> +        for ( color = start; color <= end; color++ )
> +            colors[(*num_colors)++] = color;
> +
> +        if ( *s == ',' )
> +            s++;
> +        else if ( *s != '\0' )
> +            break;
> +    }
> +
> +    return *s ? -EINVAL : 0;
> +}
> +
> +static int __init parse_dom0_colors(const char *s)
> +{
> +    return parse_color_config(s, dom0_colors, ARRAY_SIZE(dom0_colors),
> +                              &dom0_num_colors);
> +}
> +custom_param("dom0-llc-colors", parse_dom0_colors);
> 
>  static void print_colors(const unsigned int colors[], unsigned int num_colors)
>  {
> @@ -44,9 +104,26 @@ static void print_colors(const unsigned int colors[], unsigned int num_colors)
>      printk(" }\n");
>  }
> 
> +static bool __init check_colors(const unsigned int colors[],
> +                                unsigned int num_colors)
> +{
> +    unsigned int i;
> +
> +    for ( i = 0; i < num_colors; i++ )
> +    {
> +        if ( colors[i] >= max_nr_colors )
> +        {
> +            printk(XENLOG_ERR "LLC color %u >= %u\n", colors[i], max_nr_colors);
The information about max colors is not printed at the start of day, so a message like:
LLC color 20 >= 10
is not that meaningful. Could we perhaps append (max allowable) or similar to the message?

> +            return false;
> +        }
> +    }
> +
> +    return true;
> +}
> +
>  void __init llc_coloring_init(void)
>  {
> -    unsigned int way_size;
> +    unsigned int way_size, i;
> 
>      if ( llc_size && llc_nr_ways )
>      {
> @@ -83,6 +160,9 @@ void __init llc_coloring_init(void)
>      } else if ( max_nr_colors < 2 )
>          panic("Number of LLC colors %u < 2\n", max_nr_colors);
> 
> +    for ( i = 0; i < max_nr_colors; i++ )
> +        default_colors[i] = i;
> +
>      arch_llc_coloring_init();
>  }
> 
> @@ -104,6 +184,43 @@ void domain_dump_llc_colors(const struct domain *d)
>      print_colors(d->llc_colors, d->num_llc_colors);
>  }
> 
> +static void __init domain_set_default_colors(struct domain *d)
> +{
> +    printk(XENLOG_WARNING
> +           "LLC color config not found for %pd, using all colors\n", d);
> +
> +    d->llc_colors = default_colors;
> +    d->num_llc_colors = max_nr_colors;
> +}
> +
> +int __init dom0_set_llc_colors(struct domain *d)
> +{
> +    typeof(*dom0_colors) *colors;
> +
> +    if ( !dom0_num_colors )
> +    {
> +        domain_set_default_colors(d);
> +        return 0;
> +    }
> +
> +    if ( dom0_num_colors > max_nr_colors ||
NIT: Can we surround dom0_num_colors > max_nr_colors with brackets?

Other than that:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal