Message ID | 20230123154735.74832-2-carlo.nonato@minervasys.tech (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Arm cache coloring | expand |
On 23.01.2023 16:47, Carlo Nonato wrote: > @@ -769,6 +776,13 @@ struct domain *domain_create(domid_t domid, > return ERR_PTR(err); > } > > +struct domain *domain_create(domid_t domid, > + struct xen_domctl_createdomain *config, > + unsigned int flags) > +{ > + return domain_create_llc_colored(domid, config, flags, 0, 0); Please can you use NULL when you mean a null pointer? > --- /dev/null > +++ b/xen/include/xen/llc_coloring.h > @@ -0,0 +1,54 @@ > +/* 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> > + > +extern bool llc_coloring_enabled; > + > +int domain_llc_coloring_init(struct domain *d, unsigned int *colors, > + unsigned int num_colors); > +void domain_llc_coloring_free(struct domain *d); > +void domain_dump_llc_colors(struct domain *d); > + > +#else > + > +#define llc_coloring_enabled (false) While I agree this is needed, ... > +static inline int domain_llc_coloring_init(struct domain *d, > + unsigned int *colors, > + unsigned int num_colors) > +{ > + return 0; > +} > +static inline void domain_llc_coloring_free(struct domain *d) {} > +static inline void domain_dump_llc_colors(struct domain *d) {} ... I don't think you need any of these. Instead the declarations above simply need to be visible unconditionally (to be visible to the compiler when processing consuming code). We rely on DCE to remove such references in many other places. > +#endif /* CONFIG_HAS_LLC_COLORING */ > + > +#define is_domain_llc_colored(d) (llc_coloring_enabled) > + > +#endif /* __COLORING_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > \ No newline at end of file This wants taking care of. > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -602,6 +602,9 @@ struct domain > > /* Holding CDF_* constant. Internal flags for domain creation. */ > unsigned int cdf; > + > + unsigned int *llc_colors; > + unsigned int num_llc_colors; > }; Why outside of any #ifdef, and why not in struct arch_domain? Jan
Hi Jan, Julien On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 23.01.2023 16:47, Carlo Nonato wrote: > > @@ -769,6 +776,13 @@ struct domain *domain_create(domid_t domid, > > return ERR_PTR(err); > > } > > > > +struct domain *domain_create(domid_t domid, > > + struct xen_domctl_createdomain *config, > > + unsigned int flags) > > +{ > > + return domain_create_llc_colored(domid, config, flags, 0, 0); > > Please can you use NULL when you mean a null pointer? > > > --- /dev/null > > +++ b/xen/include/xen/llc_coloring.h > > @@ -0,0 +1,54 @@ > > +/* 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> > > + > > +extern bool llc_coloring_enabled; > > + > > +int domain_llc_coloring_init(struct domain *d, unsigned int *colors, > > + unsigned int num_colors); > > +void domain_llc_coloring_free(struct domain *d); > > +void domain_dump_llc_colors(struct domain *d); > > + > > +#else > > + > > +#define llc_coloring_enabled (false) > > While I agree this is needed, ... > > > +static inline int domain_llc_coloring_init(struct domain *d, > > + unsigned int *colors, > > + unsigned int num_colors) > > +{ > > + return 0; > > +} > > +static inline void domain_llc_coloring_free(struct domain *d) {} > > +static inline void domain_dump_llc_colors(struct domain *d) {} > > ... I don't think you need any of these. Instead the declarations above > simply need to be visible unconditionally (to be visible to the compiler > when processing consuming code). We rely on DCE to remove such references > in many other places. So this is true for any other stub function that I used in the series, right? Since all of them are guarded by the same kind of if statement: checking for llc_coloring_enabled value which, in case of coloring disabled from Kconfig, is always false and then DCE comes in. Sorry for being so verbose, but I just want to be sure I understood. > > +#endif /* CONFIG_HAS_LLC_COLORING */ > > + > > +#define is_domain_llc_colored(d) (llc_coloring_enabled) > > + > > +#endif /* __COLORING_H__ */ > > + > > +/* > > + * Local variables: > > + * mode: C > > + * c-file-style: "BSD" > > + * c-basic-offset: 4 > > + * tab-width: 4 > > + * indent-tabs-mode: nil > > + * End: > > + */ > > \ No newline at end of file > > This wants taking care of. > > > --- a/xen/include/xen/sched.h > > +++ b/xen/include/xen/sched.h > > @@ -602,6 +602,9 @@ struct domain > > > > /* Holding CDF_* constant. Internal flags for domain creation. */ > > unsigned int cdf; > > + > > + unsigned int *llc_colors; > > + unsigned int num_llc_colors; > > }; > > Why outside of any #ifdef, and why not in struct arch_domain? Moving this in sched.h seemed like the natural continuation of the common + arch specific split. Notice that this split is also because Julien pointed out (as you did in some earlier revision) that cache coloring can be used by other arch in the future (even if x86 is excluded). Having two maintainers saying the same thing sounded like a good reason to do that. The missing #ifdef comes from a discussion I had with Julien in v2 about domctl interface where he suggested removing it (https://marc.info/?l=xen-devel&m=166151802002263). We were talking about a different struct, but I thought the principle was the same. Anyway I would like the #ifdef too. So @Jan, @Julien, can you help me fix this once for all? > Jan Thanks. - Carlo Nonato
On 25.01.2023 12:18, Carlo Nonato wrote: > On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: >> On 23.01.2023 16:47, Carlo Nonato wrote: >>> --- /dev/null >>> +++ b/xen/include/xen/llc_coloring.h >>> @@ -0,0 +1,54 @@ >>> +/* 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> >>> + >>> +extern bool llc_coloring_enabled; >>> + >>> +int domain_llc_coloring_init(struct domain *d, unsigned int *colors, >>> + unsigned int num_colors); >>> +void domain_llc_coloring_free(struct domain *d); >>> +void domain_dump_llc_colors(struct domain *d); >>> + >>> +#else >>> + >>> +#define llc_coloring_enabled (false) >> >> While I agree this is needed, ... >> >>> +static inline int domain_llc_coloring_init(struct domain *d, >>> + unsigned int *colors, >>> + unsigned int num_colors) >>> +{ >>> + return 0; >>> +} >>> +static inline void domain_llc_coloring_free(struct domain *d) {} >>> +static inline void domain_dump_llc_colors(struct domain *d) {} >> >> ... I don't think you need any of these. Instead the declarations above >> simply need to be visible unconditionally (to be visible to the compiler >> when processing consuming code). We rely on DCE to remove such references >> in many other places. > > So this is true for any other stub function that I used in the series, right? Likely. I didn't look at most of the Arm-only pieces. >>> --- a/xen/include/xen/sched.h >>> +++ b/xen/include/xen/sched.h >>> @@ -602,6 +602,9 @@ struct domain >>> >>> /* Holding CDF_* constant. Internal flags for domain creation. */ >>> unsigned int cdf; >>> + >>> + unsigned int *llc_colors; >>> + unsigned int num_llc_colors; >>> }; >> >> Why outside of any #ifdef, and why not in struct arch_domain? > > Moving this in sched.h seemed like the natural continuation of the common + > arch specific split. Notice that this split is also because Julien pointed > out (as you did in some earlier revision) that cache coloring can be used > by other arch in the future (even if x86 is excluded). Having two maintainers > saying the same thing sounded like a good reason to do that. If you mean this to be usable by other arch-es as well (which I would welcome, as I think I had expressed on an earlier version), then I think more pieces want to be in common code. But putting the fields here and all users of them in arch-specific code (which I think is the way I saw it) doesn't look very logical to me. IOW to me there exist only two possible approaches: As much as possible in common code, or common code being disturbed as little as possible. > The missing #ifdef comes from a discussion I had with Julien in v2 about > domctl interface where he suggested removing it > (https://marc.info/?l=xen-devel&m=166151802002263). I went about five levels deep in the replies, without finding any such reply from Julien. Can you please be more specific with the link, so readers don't need to endlessly dig? Jan > We were talking about > a different struct, but I thought the principle was the same. Anyway I would > like the #ifdef too. > > So @Jan, @Julien, can you help me fix this once for all? > > Thanks. > > - Carlo Nonato
On Wed, Jan 25, 2023 at 2:10 PM Jan Beulich <jbeulich@suse.com> wrote: > > On 25.01.2023 12:18, Carlo Nonato wrote: > > On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: > >> On 23.01.2023 16:47, Carlo Nonato wrote: > >>> --- /dev/null > >>> +++ b/xen/include/xen/llc_coloring.h > >>> @@ -0,0 +1,54 @@ > >>> +/* 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> > >>> + > >>> +extern bool llc_coloring_enabled; > >>> + > >>> +int domain_llc_coloring_init(struct domain *d, unsigned int *colors, > >>> + unsigned int num_colors); > >>> +void domain_llc_coloring_free(struct domain *d); > >>> +void domain_dump_llc_colors(struct domain *d); > >>> + > >>> +#else > >>> + > >>> +#define llc_coloring_enabled (false) > >> > >> While I agree this is needed, ... > >> > >>> +static inline int domain_llc_coloring_init(struct domain *d, > >>> + unsigned int *colors, > >>> + unsigned int num_colors) > >>> +{ > >>> + return 0; > >>> +} > >>> +static inline void domain_llc_coloring_free(struct domain *d) {} > >>> +static inline void domain_dump_llc_colors(struct domain *d) {} > >> > >> ... I don't think you need any of these. Instead the declarations above > >> simply need to be visible unconditionally (to be visible to the compiler > >> when processing consuming code). We rely on DCE to remove such references > >> in many other places. > > > > So this is true for any other stub function that I used in the series, right? > > Likely. I didn't look at most of the Arm-only pieces. > > >>> --- a/xen/include/xen/sched.h > >>> +++ b/xen/include/xen/sched.h > >>> @@ -602,6 +602,9 @@ struct domain > >>> > >>> /* Holding CDF_* constant. Internal flags for domain creation. */ > >>> unsigned int cdf; > >>> + > >>> + unsigned int *llc_colors; > >>> + unsigned int num_llc_colors; > >>> }; > >> > >> Why outside of any #ifdef, and why not in struct arch_domain? > > > > Moving this in sched.h seemed like the natural continuation of the common + > > arch specific split. Notice that this split is also because Julien pointed > > out (as you did in some earlier revision) that cache coloring can be used > > by other arch in the future (even if x86 is excluded). Having two maintainers > > saying the same thing sounded like a good reason to do that. > > If you mean this to be usable by other arch-es as well (which I would > welcome, as I think I had expressed on an earlier version), then I think > more pieces want to be in common code. But putting the fields here and all > users of them in arch-specific code (which I think is the way I saw it) > doesn't look very logical to me. IOW to me there exist only two possible > approaches: As much as possible in common code, or common code being > disturbed as little as possible. This means having a llc-coloring.c in common where to put the common implementation, right? Anyway right now there is also another user of such fields in common: page_alloc.c. > > The missing #ifdef comes from a discussion I had with Julien in v2 about > > domctl interface where he suggested removing it > > (https://marc.info/?l=xen-devel&m=166151802002263). > > I went about five levels deep in the replies, without finding any such reply > from Julien. Can you please be more specific with the link, so readers don't > need to endlessly dig? https://marc.info/?l=xen-devel&m=166669617917298 quote (me and then Julien): >> We can also think of moving the coloring fields from this >> struct to the common one (xen_domctl_createdomain) protecting them with >> the proper #ifdef (but we are targeting only arm64...). > Your code is targeting arm64 but fundamentally this is an arm64 specific > feature. IOW, this could be used in the future on other arch. So I think > it would make sense to define it in common without the #ifdef. > Jan > > > We were talking about > > a different struct, but I thought the principle was the same. Anyway I would > > like the #ifdef too. > > > > So @Jan, @Julien, can you help me fix this once for all? > > > > Thanks. > > > > - Carlo Nonato >
On 25.01.2023 17:18, Carlo Nonato wrote: > On Wed, Jan 25, 2023 at 2:10 PM Jan Beulich <jbeulich@suse.com> wrote: >> On 25.01.2023 12:18, Carlo Nonato wrote: >>> On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: >>>> On 23.01.2023 16:47, Carlo Nonato wrote: >>>>> --- a/xen/include/xen/sched.h >>>>> +++ b/xen/include/xen/sched.h >>>>> @@ -602,6 +602,9 @@ struct domain >>>>> >>>>> /* Holding CDF_* constant. Internal flags for domain creation. */ >>>>> unsigned int cdf; >>>>> + >>>>> + unsigned int *llc_colors; >>>>> + unsigned int num_llc_colors; >>>>> }; >>>> >>>> Why outside of any #ifdef, and why not in struct arch_domain? >>> >>> Moving this in sched.h seemed like the natural continuation of the common + >>> arch specific split. Notice that this split is also because Julien pointed >>> out (as you did in some earlier revision) that cache coloring can be used >>> by other arch in the future (even if x86 is excluded). Having two maintainers >>> saying the same thing sounded like a good reason to do that. >> >> If you mean this to be usable by other arch-es as well (which I would >> welcome, as I think I had expressed on an earlier version), then I think >> more pieces want to be in common code. But putting the fields here and all >> users of them in arch-specific code (which I think is the way I saw it) >> doesn't look very logical to me. IOW to me there exist only two possible >> approaches: As much as possible in common code, or common code being >> disturbed as little as possible. > > This means having a llc-coloring.c in common where to put the common > implementation, right? Likely, yes. > Anyway right now there is also another user of such fields in common: > page_alloc.c. Yet hopefully all inside suitable #ifdef. >>> The missing #ifdef comes from a discussion I had with Julien in v2 about >>> domctl interface where he suggested removing it >>> (https://marc.info/?l=xen-devel&m=166151802002263). >> >> I went about five levels deep in the replies, without finding any such reply >> from Julien. Can you please be more specific with the link, so readers don't >> need to endlessly dig? > > https://marc.info/?l=xen-devel&m=166669617917298 > > quote (me and then Julien): >>> We can also think of moving the coloring fields from this >>> struct to the common one (xen_domctl_createdomain) protecting them with >>> the proper #ifdef (but we are targeting only arm64...). > >> Your code is targeting arm64 but fundamentally this is an arm64 specific >> feature. IOW, this could be used in the future on other arch. So I think >> it would make sense to define it in common without the #ifdef. I'm inclined to read this as a dislike for "#ifdef CONFIG_ARM64", not for "#ifdef CONFIG_LLC_COLORING" (or whatever the name of the option was). But I guess only Julien can clarify this ... Jan
Hi Jan, On 26/01/2023 08:06, Jan Beulich wrote: > On 25.01.2023 17:18, Carlo Nonato wrote: >> On Wed, Jan 25, 2023 at 2:10 PM Jan Beulich <jbeulich@suse.com> wrote: >>> On 25.01.2023 12:18, Carlo Nonato wrote: >>>> On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: >>>>> On 23.01.2023 16:47, Carlo Nonato wrote: >>>>>> --- a/xen/include/xen/sched.h >>>>>> +++ b/xen/include/xen/sched.h >>>>>> @@ -602,6 +602,9 @@ struct domain >>>>>> >>>>>> /* Holding CDF_* constant. Internal flags for domain creation. */ >>>>>> unsigned int cdf; >>>>>> + >>>>>> + unsigned int *llc_colors; >>>>>> + unsigned int num_llc_colors; >>>>>> }; >>>>> >>>>> Why outside of any #ifdef, and why not in struct arch_domain? >>>> >>>> Moving this in sched.h seemed like the natural continuation of the common + >>>> arch specific split. Notice that this split is also because Julien pointed >>>> out (as you did in some earlier revision) that cache coloring can be used >>>> by other arch in the future (even if x86 is excluded). Having two maintainers >>>> saying the same thing sounded like a good reason to do that. >>> >>> If you mean this to be usable by other arch-es as well (which I would >>> welcome, as I think I had expressed on an earlier version), then I think >>> more pieces want to be in common code. But putting the fields here and all >>> users of them in arch-specific code (which I think is the way I saw it) >>> doesn't look very logical to me. IOW to me there exist only two possible >>> approaches: As much as possible in common code, or common code being >>> disturbed as little as possible. >> >> This means having a llc-coloring.c in common where to put the common >> implementation, right? > > Likely, yes. > >> Anyway right now there is also another user of such fields in common: >> page_alloc.c. > > Yet hopefully all inside suitable #ifdef. > >>>> The missing #ifdef comes from a discussion I had with Julien in v2 about >>>> domctl interface where he suggested removing it >>>> (https://marc.info/?l=xen-devel&m=166151802002263). >>> >>> I went about five levels deep in the replies, without finding any such reply >>> from Julien. Can you please be more specific with the link, so readers don't >>> need to endlessly dig? >> >> https://marc.info/?l=xen-devel&m=166669617917298 >> >> quote (me and then Julien): >>>> We can also think of moving the coloring fields from this >>>> struct to the common one (xen_domctl_createdomain) protecting them with >>>> the proper #ifdef (but we are targeting only arm64...). >> >>> Your code is targeting arm64 but fundamentally this is an arm64 specific >>> feature. IOW, this could be used in the future on other arch. So I think >>> it would make sense to define it in common without the #ifdef. > > I'm inclined to read this as a dislike for "#ifdef CONFIG_ARM64", not for > "#ifdef CONFIG_LLC_COLORING" (or whatever the name of the option was). But > I guess only Julien can clarify this ... Your interpretation is correct. I would prefer if the fields are protected with #ifdef CONFIG_LLC_COLORING. Cheers,
Hi Julien and Jan, On Thu, Jan 26, 2023 at 11:16 AM Julien Grall <julien@xen.org> wrote: > > Hi Jan, > > On 26/01/2023 08:06, Jan Beulich wrote: > > On 25.01.2023 17:18, Carlo Nonato wrote: > >> On Wed, Jan 25, 2023 at 2:10 PM Jan Beulich <jbeulich@suse.com> wrote: > >>> On 25.01.2023 12:18, Carlo Nonato wrote: > >>>> On Tue, Jan 24, 2023 at 5:37 PM Jan Beulich <jbeulich@suse.com> wrote: > >>>>> On 23.01.2023 16:47, Carlo Nonato wrote: > >>>>>> --- a/xen/include/xen/sched.h > >>>>>> +++ b/xen/include/xen/sched.h > >>>>>> @@ -602,6 +602,9 @@ struct domain > >>>>>> > >>>>>> /* Holding CDF_* constant. Internal flags for domain creation. */ > >>>>>> unsigned int cdf; > >>>>>> + > >>>>>> + unsigned int *llc_colors; > >>>>>> + unsigned int num_llc_colors; > >>>>>> }; > >>>>> > >>>>> Why outside of any #ifdef, and why not in struct arch_domain? > >>>> > >>>> Moving this in sched.h seemed like the natural continuation of the common + > >>>> arch specific split. Notice that this split is also because Julien pointed > >>>> out (as you did in some earlier revision) that cache coloring can be used > >>>> by other arch in the future (even if x86 is excluded). Having two maintainers > >>>> saying the same thing sounded like a good reason to do that. > >>> > >>> If you mean this to be usable by other arch-es as well (which I would > >>> welcome, as I think I had expressed on an earlier version), then I think > >>> more pieces want to be in common code. But putting the fields here and all > >>> users of them in arch-specific code (which I think is the way I saw it) > >>> doesn't look very logical to me. IOW to me there exist only two possible > >>> approaches: As much as possible in common code, or common code being > >>> disturbed as little as possible. > >> > >> This means having a llc-coloring.c in common where to put the common > >> implementation, right? > > > > Likely, yes. > > > >> Anyway right now there is also another user of such fields in common: > >> page_alloc.c. > > > > Yet hopefully all inside suitable #ifdef. > > > >>>> The missing #ifdef comes from a discussion I had with Julien in v2 about > >>>> domctl interface where he suggested removing it > >>>> (https://marc.info/?l=xen-devel&m=166151802002263). > >>> > >>> I went about five levels deep in the replies, without finding any such reply > >>> from Julien. Can you please be more specific with the link, so readers don't > >>> need to endlessly dig? > >> > >> https://marc.info/?l=xen-devel&m=166669617917298 > >> > >> quote (me and then Julien): > >>>> We can also think of moving the coloring fields from this > >>>> struct to the common one (xen_domctl_createdomain) protecting them with > >>>> the proper #ifdef (but we are targeting only arm64...). > >> > >>> Your code is targeting arm64 but fundamentally this is an arm64 specific > >>> feature. IOW, this could be used in the future on other arch. So I think > >>> it would make sense to define it in common without the #ifdef. > > > > I'm inclined to read this as a dislike for "#ifdef CONFIG_ARM64", not for > > "#ifdef CONFIG_LLC_COLORING" (or whatever the name of the option was). But > > I guess only Julien can clarify this ... > Your interpretation is correct. I would prefer if the fields are > protected with #ifdef CONFIG_LLC_COLORING. Understood. Thanks to both. > Cheers, > > -- > Julien Grall
diff --git a/xen/arch/Kconfig b/xen/arch/Kconfig index 7028f7b74f..39c23f2528 100644 --- a/xen/arch/Kconfig +++ b/xen/arch/Kconfig @@ -28,3 +28,20 @@ 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 the documentation for how to compute the number + of colors supported by the platform. + The default value corresponds to an 8 MiB 16-ways LLC, which should be + more than what needed in the general case. + Note that if, at any time, a color configuration with more colors than + the maximum is employed, an error is produced. diff --git a/xen/common/Kconfig b/xen/common/Kconfig index f1ea3199c8..c796c633f1 100644 --- a/xen/common/Kconfig +++ b/xen/common/Kconfig @@ -49,6 +49,9 @@ config HAS_IOPORTS config HAS_KEXEC bool +config HAS_LLC_COLORING + bool + config HAS_PDX bool diff --git a/xen/common/domain.c b/xen/common/domain.c index 626debbae0..87aae86081 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> @@ -549,9 +550,11 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) return arch_sanitise_domain_config(config); } -struct domain *domain_create(domid_t domid, - struct xen_domctl_createdomain *config, - unsigned int flags) +struct domain *domain_create_llc_colored(domid_t domid, + struct xen_domctl_createdomain *config, + unsigned int flags, + unsigned int *llc_colors, + unsigned int num_llc_colors) { struct domain *d, **pd, *old_hwdom = NULL; enum { INIT_watchdog = 1u<<1, @@ -663,6 +666,10 @@ struct domain *domain_create(domid_t domid, d->nr_pirqs = min(d->nr_pirqs, nr_irqs); radix_tree_init(&d->pirq_tree); + + if ( llc_coloring_enabled && + (err = domain_llc_coloring_init(d, llc_colors, num_llc_colors)) ) + return ERR_PTR(err); } if ( (err = arch_domain_create(d, config, flags)) != 0 ) @@ -769,6 +776,13 @@ struct domain *domain_create(domid_t domid, return ERR_PTR(err); } +struct domain *domain_create(domid_t domid, + struct xen_domctl_createdomain *config, + unsigned int flags) +{ + return domain_create_llc_colored(domid, config, flags, 0, 0); +} + void __init setup_system_domains(void) { /* @@ -1103,6 +1117,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 0a551033c4..56f7731595 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..625930d378 --- /dev/null +++ b/xen/include/xen/llc_coloring.h @@ -0,0 +1,54 @@ +/* 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> + +extern bool llc_coloring_enabled; + +int domain_llc_coloring_init(struct domain *d, unsigned int *colors, + unsigned int num_colors); +void domain_llc_coloring_free(struct domain *d); +void domain_dump_llc_colors(struct domain *d); + +#else + +#define llc_coloring_enabled (false) + +static inline int domain_llc_coloring_init(struct domain *d, + unsigned int *colors, + unsigned int num_colors) +{ + return 0; +} +static inline void domain_llc_coloring_free(struct domain *d) {} +static inline void domain_dump_llc_colors(struct domain *d) {} + +#endif /* CONFIG_HAS_LLC_COLORING */ + +#define is_domain_llc_colored(d) (llc_coloring_enabled) + +#endif /* __COLORING_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ \ No newline at end of file diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 12be794002..754f6cb1da 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -602,6 +602,9 @@ struct domain /* Holding CDF_* constant. Internal flags for domain creation. */ unsigned int cdf; + + unsigned int *llc_colors; + unsigned int num_llc_colors; }; static inline struct page_list_head *page_to_list( @@ -685,6 +688,12 @@ static inline void domain_update_node_affinity(struct domain *d) */ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config); +struct domain *domain_create_llc_colored(domid_t domid, + struct xen_domctl_createdomain *config, + unsigned int flags, + unsigned int *colors, + unsigned int num_colors); + /* * Create a domain: the configuration is only necessary for real domain * (domid < DOMID_FIRST_RESERVED).