diff mbox series

[v6,06/15] tools: add support for cache coloring configuration

Message ID 20240129171811.21382-7-carlo.nonato@minervasys.tech (mailing list archive)
State Superseded
Headers show
Series Arm cache coloring | expand

Commit Message

Carlo Nonato Jan. 29, 2024, 5:18 p.m. UTC 
Add a new "llc_colors" parameter that defines the LLC color assignment for
a domain. The user can specify one or more color ranges using the same
syntax used everywhere else for color config described in the
documentation.
The parameter is defined as a list of strings that represent the color
ranges.

Documentation is also added.

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>
---
v6:
- no edits
v5:
- added LIBXL_HAVE_BUILDINFO_LLC_COLORS
- moved color configuration in xc_domain_set_llc_colors() cause of the new
  hypercall
v4:
- removed overlapping color ranges checks during parsing
- moved hypercall buffer initialization in libxenctrl
---
 docs/man/xl.cfg.5.pod.in         | 10 +++++++++
 tools/include/libxl.h            |  5 +++++
 tools/include/xenctrl.h          |  9 ++++++++
 tools/libs/ctrl/xc_domain.c      | 34 ++++++++++++++++++++++++++++
 tools/libs/light/libxl_create.c  |  9 ++++++++
 tools/libs/light/libxl_types.idl |  1 +
 tools/xl/xl_parse.c              | 38 +++++++++++++++++++++++++++++++-
 7 files changed, 105 insertions(+), 1 deletion(-)

Comments

Anthony PERARD Feb. 28, 2024, 6:05 p.m. UTC | #1 
On Mon, Jan 29, 2024 at 06:18:02PM +0100, Carlo Nonato wrote:
> diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> index 2ef8b4e054..4b541fffd2 100644
> --- a/tools/include/xenctrl.h
> +++ b/tools/include/xenctrl.h
> @@ -2653,6 +2653,15 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32
>  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
>                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
>  
> +/*
> + * Set LLC colors for a domain.
> + * This is an internal hypercall. It can only be used directly after domain

What is an "internal hypercall"? Can those even exist?

> + * creation. An attempt to use it afterwards will result in an error.
> + */
> +int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
> +                             const unsigned int *llc_colors,
> +                             unsigned int num_llc_colors);
> +
>  #if defined(__arm__) || defined(__aarch64__)
>  int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
>                    uint32_t overlay_fdt_size, uint8_t overlay_op);
> diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> index f2d9d14b4d..ad02288659 100644
> --- a/tools/libs/ctrl/xc_domain.c
> +++ b/tools/libs/ctrl/xc_domain.c
> @@ -2180,6 +2180,40 @@ int xc_domain_soft_reset(xc_interface *xch,
>      domctl.domain = domid;
>      return do_domctl(xch, &domctl);
>  }
> +
> +int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
> +                             const unsigned int *llc_colors,
> +                             unsigned int num_llc_colors)
> +{
> +    struct xen_domctl domctl = {};
> +    DECLARE_HYPERCALL_BUFFER(uint32_t, local);
> +    int ret = -1;
> +
> +    if ( num_llc_colors )
> +    {
> +        size_t bytes = sizeof(uint32_t) * num_llc_colors;

Isn't there a risk of overflow, maybe only on 32bit platform? Or maybe
that doesn't matter because the hypervisor should be able to find out if
the buffer is too short, right?

> +        local = xc_hypercall_buffer_alloc(xch, local, bytes);
> +        if ( local == NULL )
> +        {
> +            PERROR("Could not allocate LLC colors for set_llc_colors");
> +            return -ENOMEM;
> +        }
> +        memcpy(local, llc_colors, bytes);
> +        set_xen_guest_handle(domctl.u.set_llc_colors.llc_colors, local);
> +    }
> +
> +    domctl.cmd = XEN_DOMCTL_set_llc_colors;
> +    domctl.domain = domid;
> +    domctl.u.set_llc_colors.num_llc_colors = num_llc_colors;
> +
> +    ret = do_domctl(xch, &domctl);
> +
> +    if ( local )
> +        xc_hypercall_buffer_free(xch, local);

It doesn't looks like you need to check if "local != NULL" before
calling xc_hypercall_buffer_free(), it should work even with
local==NULL. This is even used multiple time in xc_kexec.

> +
> +    return ret;
> +}

Thanks,
Carlo Nonato March 1, 2024, 4:01 p.m. UTC | #2 
Hi Anthony,

On Wed, Feb 28, 2024 at 7:05 PM Anthony PERARD <anthony.perard@cloud.com> wrote:
>
> On Mon, Jan 29, 2024 at 06:18:02PM +0100, Carlo Nonato wrote:
> > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
> > index 2ef8b4e054..4b541fffd2 100644
> > --- a/tools/include/xenctrl.h
> > +++ b/tools/include/xenctrl.h
> > @@ -2653,6 +2653,15 @@ int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32
> >  int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
> >                           xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
> >
> > +/*
> > + * Set LLC colors for a domain.
> > + * This is an internal hypercall. It can only be used directly after domain
>
> What is an "internal hypercall"? Can those even exist?

Bad naming from my side. I'll remove that piece from the comment.

> > + * creation. An attempt to use it afterwards will result in an error.
> > + */
> > +int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
> > +                             const unsigned int *llc_colors,
> > +                             unsigned int num_llc_colors);
> > +
> >  #if defined(__arm__) || defined(__aarch64__)
> >  int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
> >                    uint32_t overlay_fdt_size, uint8_t overlay_op);
> > diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
> > index f2d9d14b4d..ad02288659 100644
> > --- a/tools/libs/ctrl/xc_domain.c
> > +++ b/tools/libs/ctrl/xc_domain.c
> > @@ -2180,6 +2180,40 @@ int xc_domain_soft_reset(xc_interface *xch,
> >      domctl.domain = domid;
> >      return do_domctl(xch, &domctl);
> >  }
> > +
> > +int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
> > +                             const unsigned int *llc_colors,
> > +                             unsigned int num_llc_colors)
> > +{
> > +    struct xen_domctl domctl = {};
> > +    DECLARE_HYPERCALL_BUFFER(uint32_t, local);
> > +    int ret = -1;
> > +
> > +    if ( num_llc_colors )
> > +    {
> > +        size_t bytes = sizeof(uint32_t) * num_llc_colors;
>
> Isn't there a risk of overflow, maybe only on 32bit platform? Or maybe
> that doesn't matter because the hypervisor should be able to find out if
> the buffer is too short, right?

I'm not aware of a way to find out the actual buffer size in the hypervisor,
so to answer your question, yes there's a risk of overflow, but that would
require a huge number of colors, which would probably fail the allocation
or would fail the check on the platform supported number of color in the
hypervisor.
If you prefer I can add the check here anyway.

> > +        local = xc_hypercall_buffer_alloc(xch, local, bytes);
> > +        if ( local == NULL )
> > +        {
> > +            PERROR("Could not allocate LLC colors for set_llc_colors");
> > +            return -ENOMEM;
> > +        }
> > +        memcpy(local, llc_colors, bytes);
> > +        set_xen_guest_handle(domctl.u.set_llc_colors.llc_colors, local);
> > +    }
> > +
> > +    domctl.cmd = XEN_DOMCTL_set_llc_colors;
> > +    domctl.domain = domid;
> > +    domctl.u.set_llc_colors.num_llc_colors = num_llc_colors;
> > +
> > +    ret = do_domctl(xch, &domctl);
> > +
> > +    if ( local )
> > +        xc_hypercall_buffer_free(xch, local);
>
> It doesn't looks like you need to check if "local != NULL" before
> calling xc_hypercall_buffer_free(), it should work even with
> local==NULL. This is even used multiple time in xc_kexec.

Ok, thanks.

> > +
> > +    return ret;
> > +}
>
> Thanks,
>
> --
> Anthony PERARD
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index ea8d41727d..d1140976d4 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -3038,6 +3038,16 @@  raised.
 
 =back
 
+=over 4
+
+=item B<llc_colors=[ "RANGE", "RANGE", ...]>
+
+Specify the Last Level Cache (LLC) color configuration for the guest.
+B<RANGE> can be either a single color value or a hypen-separated closed
+interval of colors (such as "0-4").
+
+=back
+
 =head3 x86
 
 =over 4
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f1652b1664..24aa2c1c2e 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1347,6 +1347,11 @@  void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, const libxl_mac *src);
  */
 #define LIBXL_HAVE_BUILDINFO_HVM_SYSTEM_FIRMWARE
 
+/*
+ * The libxl_domain_build_info has the llc_colors array.
+ */
+#define LIBXL_HAVE_BUILDINFO_LLC_COLORS 1
+
 /*
  * ERROR_REMUS_XXX error code only exists from Xen 4.5, Xen 4.6 and it
  * is changed to ERROR_CHECKPOINT_XXX in Xen 4.7
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 2ef8b4e054..4b541fffd2 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2653,6 +2653,15 @@  int xc_livepatch_replace(xc_interface *xch, char *name, uint32_t timeout, uint32
 int xc_domain_cacheflush(xc_interface *xch, uint32_t domid,
                          xen_pfn_t start_pfn, xen_pfn_t nr_pfns);
 
+/*
+ * Set LLC colors for a domain.
+ * This is an internal hypercall. It can only be used directly after domain
+ * creation. An attempt to use it afterwards will result in an error.
+ */
+int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
+                             const unsigned int *llc_colors,
+                             unsigned int num_llc_colors);
+
 #if defined(__arm__) || defined(__aarch64__)
 int xc_dt_overlay(xc_interface *xch, void *overlay_fdt,
                   uint32_t overlay_fdt_size, uint8_t overlay_op);
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index f2d9d14b4d..ad02288659 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -2180,6 +2180,40 @@  int xc_domain_soft_reset(xc_interface *xch,
     domctl.domain = domid;
     return do_domctl(xch, &domctl);
 }
+
+int xc_domain_set_llc_colors(xc_interface *xch, uint32_t domid,
+                             const unsigned int *llc_colors,
+                             unsigned int num_llc_colors)
+{
+    struct xen_domctl domctl = {};
+    DECLARE_HYPERCALL_BUFFER(uint32_t, local);
+    int ret = -1;
+
+    if ( num_llc_colors )
+    {
+        size_t bytes = sizeof(uint32_t) * num_llc_colors;
+
+        local = xc_hypercall_buffer_alloc(xch, local, bytes);
+        if ( local == NULL )
+        {
+            PERROR("Could not allocate LLC colors for set_llc_colors");
+            return -ENOMEM;
+        }
+        memcpy(local, llc_colors, bytes);
+        set_xen_guest_handle(domctl.u.set_llc_colors.llc_colors, local);
+    }
+
+    domctl.cmd = XEN_DOMCTL_set_llc_colors;
+    domctl.domain = domid;
+    domctl.u.set_llc_colors.num_llc_colors = num_llc_colors;
+
+    ret = do_domctl(xch, &domctl);
+
+    if ( local )
+        xc_hypercall_buffer_free(xch, local);
+
+    return ret;
+}
 /*
  * Local variables:
  * mode: C
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 0008fac607..4db9f574f6 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -726,6 +726,15 @@  int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
             /* A new domain now exists */
             *domid = local_domid;
 
+            ret = xc_domain_set_llc_colors(ctx->xch, local_domid,
+                                           b_info->llc_colors,
+                                           b_info->num_llc_colors);
+            if (ret < 0) {
+                LOGED(ERROR, local_domid, "LLC colors allocation failed");
+                rc = ERROR_FAIL;
+                goto out;
+            }
+
             rc = libxl__is_domid_recent(gc, local_domid, &recent);
             if (rc)
                 goto out;
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 899ad30969..0208283492 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -610,6 +610,7 @@  libxl_domain_build_info = Struct("domain_build_info",[
     ("ioports",          Array(libxl_ioport_range, "num_ioports")),
     ("irqs",             Array(uint32, "num_irqs")),
     ("iomem",            Array(libxl_iomem_range, "num_iomem")),
+    ("llc_colors",       Array(uint32, "num_llc_colors")),
     ("claim_mode",	     libxl_defbool),
     ("event_channels",   uint32),
     ("kernel",           string),
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 9b358f11b8..0ad1e2109e 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1294,7 +1294,7 @@  void parse_config_data(const char *config_source,
     XLU_ConfigList *cpus, *vbds, *nics, *pcis, *cvfbs, *cpuids, *vtpms,
                    *usbctrls, *usbdevs, *p9devs, *vdispls, *pvcallsifs_devs;
     XLU_ConfigList *channels, *ioports, *irqs, *iomem, *viridian, *dtdevs,
-                   *mca_caps, *smbios;
+                   *mca_caps, *smbios, *llc_colors;
     int num_ioports, num_irqs, num_iomem, num_cpus, num_viridian, num_mca_caps;
     int num_smbios;
     int pci_power_mgmt = 0;
@@ -1302,6 +1302,7 @@  void parse_config_data(const char *config_source,
     int pci_permissive = 0;
     int pci_seize = 0;
     int i, e;
+    int num_llc_colors;
     char *kernel_basename;
 
     libxl_domain_create_info *c_info = &d_config->c_info;
@@ -1445,6 +1446,41 @@  void parse_config_data(const char *config_source,
     if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
         b_info->max_memkb = l * 1024;
 
+    if (!xlu_cfg_get_list(config, "llc_colors", &llc_colors, &num_llc_colors, 0)) {
+        int cur_index = 0;
+
+        b_info->num_llc_colors = 0;
+        for (i = 0; i < num_llc_colors; i++) {
+            uint32_t start = 0, end = 0, k;
+
+            buf = xlu_cfg_get_listitem(llc_colors, i);
+            if (!buf) {
+                fprintf(stderr,
+                        "xl: Can't get element %d in LLC color list\n", i);
+                exit(1);
+            }
+
+            if (sscanf(buf, "%" SCNu32 "-%" SCNu32, &start, &end) != 2) {
+                if (sscanf(buf, "%" SCNu32, &start) != 1) {
+                    fprintf(stderr, "xl: Invalid LLC color range: %s\n", buf);
+                    exit(1);
+                }
+                end = start;
+            } else if (start > end) {
+                fprintf(stderr,
+                        "xl: Start LLC color is greater than end: %s\n", buf);
+                exit(1);
+            }
+
+            b_info->num_llc_colors += (end - start) + 1;
+            b_info->llc_colors = (uint32_t *)realloc(b_info->llc_colors,
+                        sizeof(*b_info->llc_colors) * b_info->num_llc_colors);
+
+            for (k = start; k <= end; k++)
+                b_info->llc_colors[cur_index++] = k;
+        }
+    }
+
     if (!xlu_cfg_get_long (config, "vcpus", &l, 0)) {
         vcpus = l;
         if (libxl_cpu_bitmap_alloc(ctx, &b_info->avail_vcpus, l)) {