| Message ID | 20240102095138.17933-2-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm cache coloring | expand |
Hi Carlo, On 02/01/2024 09:51, Carlo Nonato wrote: > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > options and functions. Since this is an arch specific feature, actual > implementation is postponed to later patches and Kconfig options are placed > under xen/arch. > > LLC colors are a property of the domain, so the domain struct has to be > extended. The interface below looks ok. I have left some comments below. > > 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> > --- > v5: > - used - instead of _ for filenames > - removed domain_create_llc_colored() > - removed stub functions > - coloring domain fields are now #ifdef protected > v4: > - Kconfig options moved to xen/arch > - removed range for CONFIG_NR_LLC_COLORS > - added "llc_coloring_enabled" global to later implement the boot-time > switch > - added domain_create_llc_colored() to be able to pass colors > - added is_domain_llc_colored() macro > --- > xen/arch/Kconfig | 16 ++++++++++++ > xen/common/Kconfig | 3 +++ > xen/common/domain.c | 4 +++ > xen/common/keyhandler.c | 4 +++ > xen/include/xen/llc-coloring.h | 46 ++++++++++++++++++++++++++++++++++ > xen/include/xen/sched.h | 5 ++++ > 6 files changed, 78 insertions(+) > create mode 100644 xen/include/xen/llc-coloring.h > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > index 67ba38f32f..aad7e9da38 100644 > --- a/xen/arch/Kconfig > +++ b/xen/arch/Kconfig > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > associated with multiple-nodes management. It is the upper bound of > the number of NUMA nodes that the scheduler, memory allocation and > other NUMA-aware components can handle. > + > +config LLC_COLORING > + bool "Last Level Cache (LLC) coloring" if EXPERT > + depends on HAS_LLC_COLORING > + > +config NR_LLC_COLORS > + int "Maximum number of LLC colors" > + default 128 > + depends on LLC_COLORING > + help > + Controls the build-time size of various arrays associated with LLC > + coloring. Refer to cache coloring documentation for how to compute the > + number of colors supported by the platform. This is only an upper > + bound. The runtime value is autocomputed or manually set via cmdline. > + The default value corresponds to an 8 MiB 16-ways LLC, which should be > + more than what needed in the general case. > diff --git a/xen/common/Kconfig b/xen/common/Kconfig > index 310ad4229c..e383f09d97 100644 > --- a/xen/common/Kconfig > +++ b/xen/common/Kconfig > @@ -71,6 +71,9 @@ config HAS_IOPORTS > config HAS_KEXEC > bool > > +config HAS_LLC_COLORING > + bool > + > config HAS_PMAP > bool > > diff --git a/xen/common/domain.c b/xen/common/domain.c > index f6f5574996..491585b0bb 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -7,6 +7,7 @@ > #include <xen/compat.h> > #include <xen/init.h> > #include <xen/lib.h> > +#include <xen/llc-coloring.h> > #include <xen/ctype.h> > #include <xen/err.h> > #include <xen/param.h> > @@ -1144,6 +1145,9 @@ static void cf_check complete_domain_destroy(struct rcu_head *head) > struct vcpu *v; > int i; > > + if ( is_domain_llc_colored(d) ) > + domain_llc_coloring_free(d); > + > /* > * Flush all state for the vCPU previously having run on the current CPU. > * This is in particular relevant for x86 HVM ones on VMX, so that this > diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c > index 99a2d72a02..27c2d324d8 100644 > --- a/xen/common/keyhandler.c > +++ b/xen/common/keyhandler.c > @@ -6,6 +6,7 @@ > #include <xen/debugger.h> > #include <xen/delay.h> > #include <xen/keyhandler.h> > +#include <xen/llc-coloring.h> > #include <xen/param.h> > #include <xen/shutdown.h> > #include <xen/event.h> > @@ -307,6 +308,9 @@ static void cf_check dump_domains(unsigned char key) > > arch_dump_domain_info(d); > > + if ( is_domain_llc_colored(d) ) > + domain_dump_llc_colors(d); > + > rangeset_domain_printk(d); > > dump_pageframe_info(d); > diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h > new file mode 100644 > index 0000000000..cedd97d4b5 > --- /dev/null > +++ b/xen/include/xen/llc-coloring.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ For new SPDX we should use GPL-2.0-only. AFAIK, this is equivalent license but with a clearer name. > +/* > + * Last Level Cache (LLC) coloring common header > + * > + * Copyright (C) 2022 Xilinx Inc. > + * > + * Authors: > + * Carlo Nonato <carlo.nonato@minervasys.tech> NIT: We don't usually add the authors in the code and instead rely on the Author/Signed-off-by in the commit. If you want to keep it, then I will query why you added yourself but not Marco Solieri or Luca Miccio. > + */ > +#ifndef __COLORING_H__ > +#define __COLORING_H__ > + > +#include <xen/sched.h> > +#include <public/domctl.h> > + > +#ifdef CONFIG_HAS_LLC_COLORING > + > +#include <asm/llc-coloring.h> > + > +#ifdef CONFIG_LLC_COLORING > +extern bool llc_coloring_enabled; > +#define llc_coloring_enabled (llc_coloring_enabled) I don't really understand why you need the define here. Wouldn't it be clearer to have a #else and then ... > +#endif > + > +#endif > + > +#ifndef llc_coloring_enabled > +#define llc_coloring_enabled (false) ... define llc_coloring_enabled? Also NIT: The parentheses are not necessary. > +#endif > + > +#define is_domain_llc_colored(d) (llc_coloring_enabled) You want to evaluate d. But here I would consider to use a static inline. Nowadays, we favor static inline helpers over macros as they add typesafety and you don't need to do trick (e.v. (void)(d)) to interpret the arguments. > + > +void domain_llc_coloring_free(struct domain *d); > +void domain_dump_llc_colors(struct domain *d); Looking at the usage, 'd' could be const. > + > +#endif /* __COLORING_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index 9da91e0e62..dae7fab673 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -626,6 +626,11 @@ struct domain > > /* Holding CDF_* constant. Internal flags for domain creation. */ > unsigned int cdf; > + > +#ifdef CONFIG_LLC_COLORING > + unsigned int *llc_colors; > + unsigned int num_llc_colors; > +#endif > }; > > static inline struct page_list_head *page_to_list( Cheers,
Hi, On 02/01/2024 09:51, Carlo Nonato wrote: > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > options and functions. Since this is an arch specific feature, actual > implementation is postponed to later patches and Kconfig options are placed > under xen/arch. > > LLC colors are a property of the domain, so the domain struct has to be > extended. > > 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> > --- > v5: > - used - instead of _ for filenames > - removed domain_create_llc_colored() > - removed stub functions > - coloring domain fields are now #ifdef protected > v4: > - Kconfig options moved to xen/arch > - removed range for CONFIG_NR_LLC_COLORS > - added "llc_coloring_enabled" global to later implement the boot-time > switch > - added domain_create_llc_colored() to be able to pass colors > - added is_domain_llc_colored() macro > --- > xen/arch/Kconfig | 16 ++++++++++++ > xen/common/Kconfig | 3 +++ > xen/common/domain.c | 4 +++ > xen/common/keyhandler.c | 4 +++ > xen/include/xen/llc-coloring.h | 46 ++++++++++++++++++++++++++++++++++ > xen/include/xen/sched.h | 5 ++++ > 6 files changed, 78 insertions(+) > create mode 100644 xen/include/xen/llc-coloring.h > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > index 67ba38f32f..aad7e9da38 100644 > --- a/xen/arch/Kconfig > +++ b/xen/arch/Kconfig > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > associated with multiple-nodes management. It is the upper bound of > the number of NUMA nodes that the scheduler, memory allocation and > other NUMA-aware components can handle. > + > +config LLC_COLORING > + bool "Last Level Cache (LLC) coloring" if EXPERT While look at the rest of the series, I noticed that SUPPORT.md is not updated. Can this be done? I think the feature should be in experimental for now. We can decide to switch to tech preview before Xen 4.19 is out and the support is completed. Stefano, what do you think? Cheers
On Thu, 4 Jan 2024, Julien Grall wrote: > Hi, > > On 02/01/2024 09:51, Carlo Nonato wrote: > > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > > options and functions. Since this is an arch specific feature, actual > > implementation is postponed to later patches and Kconfig options are placed > > under xen/arch. > > > > LLC colors are a property of the domain, so the domain struct has to be > > extended. > > > > 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> > > --- > > v5: > > - used - instead of _ for filenames > > - removed domain_create_llc_colored() > > - removed stub functions > > - coloring domain fields are now #ifdef protected > > v4: > > - Kconfig options moved to xen/arch > > - removed range for CONFIG_NR_LLC_COLORS > > - added "llc_coloring_enabled" global to later implement the boot-time > > switch > > - added domain_create_llc_colored() to be able to pass colors > > - added is_domain_llc_colored() macro > > --- > > xen/arch/Kconfig | 16 ++++++++++++ > > xen/common/Kconfig | 3 +++ > > xen/common/domain.c | 4 +++ > > xen/common/keyhandler.c | 4 +++ > > xen/include/xen/llc-coloring.h | 46 ++++++++++++++++++++++++++++++++++ > > xen/include/xen/sched.h | 5 ++++ > > 6 files changed, 78 insertions(+) > > create mode 100644 xen/include/xen/llc-coloring.h > > > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > > index 67ba38f32f..aad7e9da38 100644 > > --- a/xen/arch/Kconfig > > +++ b/xen/arch/Kconfig > > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > > associated with multiple-nodes management. It is the upper bound of > > the number of NUMA nodes that the scheduler, memory allocation and > > other NUMA-aware components can handle. > > + > > +config LLC_COLORING > > + bool "Last Level Cache (LLC) coloring" if EXPERT > > While look at the rest of the series, I noticed that SUPPORT.md is not > updated. Can this be done? > > I think the feature should be in experimental for now. We can decide to switch > to tech preview before Xen 4.19 is out and the support is completed. > > Stefano, what do you think? That's reasonable
Hi Stefano, Julien, On Thu, Jan 4, 2024 at 10:43 PM Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Thu, 4 Jan 2024, Julien Grall wrote: > > Hi, > > > > On 02/01/2024 09:51, Carlo Nonato wrote: > > > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > > > options and functions. Since this is an arch specific feature, actual > > > implementation is postponed to later patches and Kconfig options are placed > > > under xen/arch. > > > > > > LLC colors are a property of the domain, so the domain struct has to be > > > extended. > > > > > > 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> > > > --- > > > v5: > > > - used - instead of _ for filenames > > > - removed domain_create_llc_colored() > > > - removed stub functions > > > - coloring domain fields are now #ifdef protected > > > v4: > > > - Kconfig options moved to xen/arch > > > - removed range for CONFIG_NR_LLC_COLORS > > > - added "llc_coloring_enabled" global to later implement the boot-time > > > switch > > > - added domain_create_llc_colored() to be able to pass colors > > > - added is_domain_llc_colored() macro > > > --- > > > xen/arch/Kconfig | 16 ++++++++++++ > > > xen/common/Kconfig | 3 +++ > > > xen/common/domain.c | 4 +++ > > > xen/common/keyhandler.c | 4 +++ > > > xen/include/xen/llc-coloring.h | 46 ++++++++++++++++++++++++++++++++++ > > > xen/include/xen/sched.h | 5 ++++ > > > 6 files changed, 78 insertions(+) > > > create mode 100644 xen/include/xen/llc-coloring.h > > > > > > diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig > > > index 67ba38f32f..aad7e9da38 100644 > > > --- a/xen/arch/Kconfig > > > +++ b/xen/arch/Kconfig > > > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > > > associated with multiple-nodes management. It is the upper bound of > > > the number of NUMA nodes that the scheduler, memory allocation and > > > other NUMA-aware components can handle. > > > + > > > +config LLC_COLORING > > > + bool "Last Level Cache (LLC) coloring" if EXPERT > > > > While look at the rest of the series, I noticed that SUPPORT.md is not > > updated. Can this be done? > > > > I think the feature should be in experimental for now. We can decide to switch > > to tech preview before Xen 4.19 is out and the support is completed. > > > > Stefano, what do you think? > > That's reasonable I would put it under "Resource management" features. Are you ok with it? Thanks.
On 02.01.2024 10:51, Carlo Nonato wrote: > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > options and functions. Since this is an arch specific feature, actual > implementation is postponed to later patches and Kconfig options are placed > under xen/arch. As a general remark / nit: "This commit", "this patch", or alike aren't well suited for descriptions. > --- a/xen/arch/Kconfig > +++ b/xen/arch/Kconfig > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > associated with multiple-nodes management. It is the upper bound of > the number of NUMA nodes that the scheduler, memory allocation and > other NUMA-aware components can handle. > + > +config LLC_COLORING > + bool "Last Level Cache (LLC) coloring" if EXPERT > + depends on HAS_LLC_COLORING > + > +config NR_LLC_COLORS > + int "Maximum number of LLC colors" > + default 128 What if I set to value to 0? Or to an unreasonably large one? You don't bound the value range at all. > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -7,6 +7,7 @@ > #include <xen/compat.h> > #include <xen/init.h> > #include <xen/lib.h> > +#include <xen/llc-coloring.h> > #include <xen/ctype.h> > #include <xen/err.h> > #include <xen/param.h> > @@ -1144,6 +1145,9 @@ static void cf_check complete_domain_destroy(struct rcu_head *head) > struct vcpu *v; > int i; > > + if ( is_domain_llc_colored(d) ) > + domain_llc_coloring_free(d); Would be nice if the freeing function could be called unconditionally, being a no-op for non-colored domains. Further - is it really necessary to do this freeing this late? > --- a/xen/common/keyhandler.c > +++ b/xen/common/keyhandler.c > @@ -6,6 +6,7 @@ > #include <xen/debugger.h> > #include <xen/delay.h> > #include <xen/keyhandler.h> > +#include <xen/llc-coloring.h> > #include <xen/param.h> > #include <xen/shutdown.h> > #include <xen/event.h> > @@ -307,6 +308,9 @@ static void cf_check dump_domains(unsigned char key) > > arch_dump_domain_info(d); > > + if ( is_domain_llc_colored(d) ) > + domain_dump_llc_colors(d); I'm less concerned of the conditional here, but along the lines of the comment above, it could of course again be the function that simply is a no-op for non-colored domains. > --- /dev/null > +++ b/xen/include/xen/llc-coloring.h > @@ -0,0 +1,46 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Last Level Cache (LLC) coloring common header > + * > + * Copyright (C) 2022 Xilinx Inc. > + * > + * Authors: > + * Carlo Nonato <carlo.nonato@minervasys.tech> > + */ > +#ifndef __COLORING_H__ > +#define __COLORING_H__ > + > +#include <xen/sched.h> > +#include <public/domctl.h> > + > +#ifdef CONFIG_HAS_LLC_COLORING Why does this matter here? IOW why ... > +#include <asm/llc-coloring.h> > + > +#ifdef CONFIG_LLC_COLORING ... is it not just this which is checked? > +extern bool llc_coloring_enabled; > +#define llc_coloring_enabled (llc_coloring_enabled) > +#endif > + > +#endif > + > +#ifndef llc_coloring_enabled > +#define llc_coloring_enabled (false) > +#endif +1 to the question Julien has raised here. > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -626,6 +626,11 @@ struct domain > > /* Holding CDF_* constant. Internal flags for domain creation. */ > unsigned int cdf; > + > +#ifdef CONFIG_LLC_COLORING > + unsigned int *llc_colors; Can the color values change over the lifetime of a domain? If not, it may be prudent to have this be pointer-to-const. Jan > + unsigned int num_llc_colors; > +#endif > }; > > static inline struct page_list_head *page_to_list(
Hi Jan, On Mon, Jan 8, 2024 at 5:53 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 02.01.2024 10:51, Carlo Nonato wrote: > > This commit adds the Last Level Cache (LLC) coloring common header, Kconfig > > options and functions. Since this is an arch specific feature, actual > > implementation is postponed to later patches and Kconfig options are placed > > under xen/arch. > > As a general remark / nit: "This commit", "this patch", or alike aren't > well suited for descriptions. > > > --- a/xen/arch/Kconfig > > +++ b/xen/arch/Kconfig > > @@ -31,3 +31,19 @@ config NR_NUMA_NODES > > associated with multiple-nodes management. It is the upper bound of > > the number of NUMA nodes that the scheduler, memory allocation and > > other NUMA-aware components can handle. > > + > > +config LLC_COLORING > > + bool "Last Level Cache (LLC) coloring" if EXPERT > > + depends on HAS_LLC_COLORING > > + > > +config NR_LLC_COLORS > > + int "Maximum number of LLC colors" > > + default 128 > > What if I set to value to 0? Or to an unreasonably large one? You don't > bound the value range at all. I can reintroduce the range (it was there in previous versions). I just don't know what numbers to put. range 2 1024 seems reasonable since having 1 color only actually lowers performances because of the obvious sharing of resources. 1024 are the colors that can fit into a standard 4KiB page. It's big enough for currently supported hardware that normally has 16 or 32 colors. > > --- a/xen/common/domain.c > > +++ b/xen/common/domain.c > > @@ -7,6 +7,7 @@ > > #include <xen/compat.h> > > #include <xen/init.h> > > #include <xen/lib.h> > > +#include <xen/llc-coloring.h> > > #include <xen/ctype.h> > > #include <xen/err.h> > > #include <xen/param.h> > > @@ -1144,6 +1145,9 @@ static void cf_check complete_domain_destroy(struct rcu_head *head) > > struct vcpu *v; > > int i; > > > > + if ( is_domain_llc_colored(d) ) > > + domain_llc_coloring_free(d); > > Would be nice if the freeing function could be called unconditionally, > being a no-op for non-colored domains. Ok. > Further - is it really necessary to do this freeing this late? No, I can move it in domain_destroy(). > > --- a/xen/common/keyhandler.c > > +++ b/xen/common/keyhandler.c > > @@ -6,6 +6,7 @@ > > #include <xen/debugger.h> > > #include <xen/delay.h> > > #include <xen/keyhandler.h> > > +#include <xen/llc-coloring.h> > > #include <xen/param.h> > > #include <xen/shutdown.h> > > #include <xen/event.h> > > @@ -307,6 +308,9 @@ static void cf_check dump_domains(unsigned char key) > > > > arch_dump_domain_info(d); > > > > + if ( is_domain_llc_colored(d) ) > > + domain_dump_llc_colors(d); > > I'm less concerned of the conditional here, but along the lines of the > comment above, it could of course again be the function that simply is > a no-op for non-colored domains. Ok. > > --- /dev/null > > +++ b/xen/include/xen/llc-coloring.h > > @@ -0,0 +1,46 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Last Level Cache (LLC) coloring common header > > + * > > + * Copyright (C) 2022 Xilinx Inc. > > + * > > + * Authors: > > + * Carlo Nonato <carlo.nonato@minervasys.tech> > > + */ > > +#ifndef __COLORING_H__ > > +#define __COLORING_H__ > > + > > +#include <xen/sched.h> > > +#include <public/domctl.h> > > + > > +#ifdef CONFIG_HAS_LLC_COLORING > > Why does this matter here? IOW why ... > > > +#include <asm/llc-coloring.h> > > + > > +#ifdef CONFIG_LLC_COLORING > > ... is it not just this which is checked? > > > +extern bool llc_coloring_enabled; > > +#define llc_coloring_enabled (llc_coloring_enabled) > > +#endif > > + > > +#endif > > + > > +#ifndef llc_coloring_enabled > > +#define llc_coloring_enabled (false) > > +#endif > > +1 to the question Julien has raised here. Yes this whole block can be better structured. > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -626,6 +626,11 @@ struct domain > > > > /* Holding CDF_* constant. Internal flags for domain creation. */ > > unsigned int cdf; > > + > > +#ifdef CONFIG_LLC_COLORING > > + unsigned int *llc_colors; > > Can the color values change over the lifetime of a domain? If not, > it may be prudent to have this be pointer-to-const. Can I free a pointer-to-const array? > Jan > > > + unsigned int num_llc_colors; > > +#endif > > }; > > > > static inline struct page_list_head *page_to_list( > Thanks.
On 11.01.2024 11:10, Carlo Nonato wrote: > On Mon, Jan 8, 2024 at 5:53 PM Jan Beulich <jbeulich@suse.com> wrote: >> On 02.01.2024 10:51, Carlo Nonato wrote: >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -626,6 +626,11 @@ struct domain >>> >>> /* Holding CDF_* constant. Internal flags for domain creation. */ >>> unsigned int cdf; >>> + >>> +#ifdef CONFIG_LLC_COLORING >>> + unsigned int *llc_colors; >> >> Can the color values change over the lifetime of a domain? If not, >> it may be prudent to have this be pointer-to-const. > > Can I free a pointer-to-const array? Well, you certainly can by using a cast. Avoiding the need for such call- site casts is why I've been advocating to make xfree() / vfree() match e.g. vunmap() / _vfree() in this regard. Yet other opinions are that the const there ought to be dropped (without me really following the reasoning) ... There's also the option of circumventing the need for a cast by doing xfree(__va(__pa(ptr))); Jan
Hi Carlo, On 05/01/2024 16:32, Carlo Nonato wrote: > Hi Stefano, Julien, > > On Thu, Jan 4, 2024 at 10:43 PM Stefano Stabellini > <sstabellini@kernel.org> wrote: >> >> On Thu, 4 Jan 2024, Julien Grall wrote: >>> Hi, >>> >>> On 02/01/2024 09:51, Carlo Nonato wrote: >>>> This commit adds the Last Level Cache (LLC) coloring common header, Kconfig >>>> options and functions. Since this is an arch specific feature, actual >>>> implementation is postponed to later patches and Kconfig options are placed >>>> under xen/arch. >>>> >>>> LLC colors are a property of the domain, so the domain struct has to be >>>> extended. >>>> >>>> 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> >>>> --- >>>> v5: >>>> - used - instead of _ for filenames >>>> - removed domain_create_llc_colored() >>>> - removed stub functions >>>> - coloring domain fields are now #ifdef protected >>>> v4: >>>> - Kconfig options moved to xen/arch >>>> - removed range for CONFIG_NR_LLC_COLORS >>>> - added "llc_coloring_enabled" global to later implement the boot-time >>>> switch >>>> - added domain_create_llc_colored() to be able to pass colors >>>> - added is_domain_llc_colored() macro >>>> --- >>>> xen/arch/Kconfig | 16 ++++++++++++ >>>> xen/common/Kconfig | 3 +++ >>>> xen/common/domain.c | 4 +++ >>>> xen/common/keyhandler.c | 4 +++ >>>> xen/include/xen/llc-coloring.h | 46 ++++++++++++++++++++++++++++++++++ >>>> xen/include/xen/sched.h | 5 ++++ >>>> 6 files changed, 78 insertions(+) >>>> create mode 100644 xen/include/xen/llc-coloring.h >>>> >>>> diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig >>>> index 67ba38f32f..aad7e9da38 100644 >>>> --- a/xen/arch/Kconfig >>>> +++ b/xen/arch/Kconfig >>>> @@ -31,3 +31,19 @@ config NR_NUMA_NODES >>>> associated with multiple-nodes management. It is the upper bound of >>>> the number of NUMA nodes that the scheduler, memory allocation and >>>> other NUMA-aware components can handle. >>>> + >>>> +config LLC_COLORING >>>> + bool "Last Level Cache (LLC) coloring" if EXPERT >>> >>> While look at the rest of the series, I noticed that SUPPORT.md is not >>> updated. Can this be done? >>> >>> I think the feature should be in experimental for now. We can decide to switch >>> to tech preview before Xen 4.19 is out and the support is completed. >>> >>> Stefano, what do you think? >> >> That's reasonable > > I would put it under "Resource management" features. Are you ok with it? I think this wants to go under "## Memory Management". The section "Resource management" seems to be more related to the scheduler. Cheers,
diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig index 67ba38f32f..aad7e9da38 100644 --- a/xen/arch/Kconfig +++ b/xen/arch/Kconfig @@ -31,3 +31,19 @@ config NR_NUMA_NODES associated with multiple-nodes management. It is the upper bound of the number of NUMA nodes that the scheduler, memory allocation and other NUMA-aware components can handle. + +config LLC_COLORING + bool "Last Level Cache (LLC) coloring" if EXPERT + depends on HAS_LLC_COLORING + +config NR_LLC_COLORS + int "Maximum number of LLC colors" + default 128 + depends on LLC_COLORING + help + Controls the build-time size of various arrays associated with LLC + coloring. Refer to cache coloring documentation for how to compute the + number of colors supported by the platform. This is only an upper + bound. The runtime value is autocomputed or manually set via cmdline. + The default value corresponds to an 8 MiB 16-ways LLC, which should be + more than what needed in the general case. diff --git a/xen/common/Kconfig b/xen/common/Kconfig index 310ad4229c..e383f09d97 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -71,6 +71,9 @@ config HAS_IOPORTS config HAS_KEXEC bool +config HAS_LLC_COLORING + bool + config HAS_PMAP bool diff --git a/xen/common/domain.c b/xen/common/domain.c index f6f5574996..491585b0bb 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -7,6 +7,7 @@ #include <xen/compat.h> #include <xen/init.h> #include <xen/lib.h> +#include <xen/llc-coloring.h> #include <xen/ctype.h> #include <xen/err.h> #include <xen/param.h> @@ -1144,6 +1145,9 @@ static void cf_check complete_domain_destroy(struct rcu_head *head) struct vcpu *v; int i; + if ( is_domain_llc_colored(d) ) + domain_llc_coloring_free(d); + /* * Flush all state for the vCPU previously having run on the current CPU. * This is in particular relevant for x86 HVM ones on VMX, so that this diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c index 99a2d72a02..27c2d324d8 100644 --- a/xen/common/keyhandler.c +++ b/xen/common/keyhandler.c @@ -6,6 +6,7 @@ #include <xen/debugger.h> #include <xen/delay.h> #include <xen/keyhandler.h> +#include <xen/llc-coloring.h> #include <xen/param.h> #include <xen/shutdown.h> #include <xen/event.h> @@ -307,6 +308,9 @@ static void cf_check dump_domains(unsigned char key) arch_dump_domain_info(d); + if ( is_domain_llc_colored(d) ) + domain_dump_llc_colors(d); + rangeset_domain_printk(d); dump_pageframe_info(d); diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h new file mode 100644 index 0000000000..cedd97d4b5 --- /dev/null +++ b/xen/include/xen/llc-coloring.h @@ -0,0 +1,46 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Last Level Cache (LLC) coloring common header + * + * Copyright (C) 2022 Xilinx Inc. + * + * Authors: + * Carlo Nonato <carlo.nonato@minervasys.tech> + */ +#ifndef __COLORING_H__ +#define __COLORING_H__ + +#include <xen/sched.h> +#include <public/domctl.h> + +#ifdef CONFIG_HAS_LLC_COLORING + +#include <asm/llc-coloring.h> + +#ifdef CONFIG_LLC_COLORING +extern bool llc_coloring_enabled; +#define llc_coloring_enabled (llc_coloring_enabled) +#endif + +#endif + +#ifndef llc_coloring_enabled +#define llc_coloring_enabled (false) +#endif + +#define is_domain_llc_colored(d) (llc_coloring_enabled) + +void domain_llc_coloring_free(struct domain *d); +void domain_dump_llc_colors(struct domain *d); + +#endif /* __COLORING_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 9da91e0e62..dae7fab673 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -626,6 +626,11 @@ struct domain /* Holding CDF_* constant. Internal flags for domain creation. */ unsigned int cdf; + +#ifdef CONFIG_LLC_COLORING + unsigned int *llc_colors; + unsigned int num_llc_colors; +#endif }; static inline struct page_list_head *page_to_list(