| Message ID | 20240102095138.17933-1-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| Headers | show |
| Series | Arm cache coloring | expand |
Hi, On 02/01/2024 10:51, Carlo Nonato wrote: > > > Shared caches in multi-core CPU architectures represent a problem for > predictability of memory access latency. This jeopardizes applicability > of many Arm platform in real-time critical and mixed-criticality > scenarios. We introduce support for cache partitioning with page > coloring, a transparent software technique that enables isolation > between domains and Xen, and thus avoids cache interference. > > When creating a domain, a simple syntax (e.g. `0-3` or `4-11`) allows > the user to define assignments of cache partitions ids, called colors, > where assigning different colors guarantees no mutual eviction on cache > will ever happen. This instructs the Xen memory allocator to provide > the i-th color assignee only with pages that maps to color i, i.e. that > are indexed in the i-th cache partition. > > The proposed implementation supports the dom0less feature. > The proposed implementation doesn't support the static-mem feature. > The solution has been tested in several scenarios, including Xilinx Zynq > MPSoCs. > > In this patch series there are two major unacceptable workarounds for which > I want to ask you for comments: > - #3: allocate_memory() has been moved in dom0less_build.c, so I just copied > it back to make it compile. I would move it to domain_build.c and add a prototype in domain_build.h. You could guard it together with allocate_bank_memory() for DOM0LESS || LLC or just remove the guards (in former case, you would need to protect your call with #ifdef in construct_dom0()). > - #13: consider_modules() has been moved to arm32 only. Again I just copied it. I would move it to setup.c and add a prototype in setup.h. As for the guards, if we want them (personally I don't see the need), you would need LLC || (ARM_32 && MMU). BTW. Patchew reports some build issues in your series: https://gitlab.com/xen-project/patchew/xen/-/pipelines/1124313980 Make sure to build test your series on different arches. You can ask on IRC to become a member on gitlab so that you can trigger the pipeline by pushing the changes to your fork on people/<you>/xen. ~Michal
On Wed, 3 Jan 2024, Michal Orzel wrote: > Hi, > > On 02/01/2024 10:51, Carlo Nonato wrote: > > > > > > Shared caches in multi-core CPU architectures represent a problem for > > predictability of memory access latency. This jeopardizes applicability > > of many Arm platform in real-time critical and mixed-criticality > > scenarios. We introduce support for cache partitioning with page > > coloring, a transparent software technique that enables isolation > > between domains and Xen, and thus avoids cache interference. > > > > When creating a domain, a simple syntax (e.g. `0-3` or `4-11`) allows > > the user to define assignments of cache partitions ids, called colors, > > where assigning different colors guarantees no mutual eviction on cache > > will ever happen. This instructs the Xen memory allocator to provide > > the i-th color assignee only with pages that maps to color i, i.e. that > > are indexed in the i-th cache partition. > > > > The proposed implementation supports the dom0less feature. > > The proposed implementation doesn't support the static-mem feature. > > The solution has been tested in several scenarios, including Xilinx Zynq > > MPSoCs. > > > > In this patch series there are two major unacceptable workarounds for which > > I want to ask you for comments: > > - #3: allocate_memory() has been moved in dom0less_build.c, so I just copied > > it back to make it compile. > I would move it to domain_build.c and add a prototype in domain_build.h. > You could guard it together with allocate_bank_memory() for DOM0LESS || LLC or just > remove the guards (in former case, you would need to protect your call with #ifdef in construct_dom0()). > > > - #13: consider_modules() has been moved to arm32 only. Again I just copied it. > I would move it to setup.c and add a prototype in setup.h. > As for the guards, if we want them (personally I don't see the need), you would need LLC || (ARM_32 && MMU). > > BTW. Patchew reports some build issues in your series: > https://gitlab.com/xen-project/patchew/xen/-/pipelines/1124313980 > > Make sure to build test your series on different arches. You can ask on IRC to become a member > on gitlab so that you can trigger the pipeline by pushing the changes to your fork on people/<you>/xen. Also I tried this patch series on the "staging" branch and Xen failed to boot, no messages at all from Xen so it must be an early boot failure. I am passing this command line options to Xen and running it on QEMU: dom0_mem=1024M dom0_max_vcpus=1 xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=65535 llc-coloring=true
On Wed, 3 Jan 2024, Stefano Stabellini wrote: > Also I tried this patch series on the "staging" branch and Xen failed to > boot, no messages at all from Xen so it must be an early boot failure. I > am passing this command line options to Xen and running it on QEMU: > > dom0_mem=1024M dom0_max_vcpus=1 xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=65535 llc-coloring=true I managed to make it to work successfully with the following command line: xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=64K llc-coloring=on I think the problem was llc-way-size that needs to be rounded up (64K instead of 65535). Also I found a few build issues when building for other architectures or different kconfig options. This patch addresses those issues (however it is not to be taken as is as the build issues should not be introduced in the first place and there are probably better way to fix them, but I hope it can help). Cheers, Stefano diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c index f434efc45b..efe5cf3c23 100644 --- a/xen/arch/arm/llc-coloring.c +++ b/xen/arch/arm/llc-coloring.c @@ -39,7 +39,7 @@ static unsigned int __ro_after_init xen_num_colors; #define mfn_color_mask (nr_colors - 1) #define mfn_to_color(mfn) (mfn_x(mfn) & mfn_color_mask) -#define mfn_set_color(mfn, color) ((mfn_x(mfn) & ~mfn_color_mask) | (color)) +#define mfn_set_color(mfn, color) (_mfn((mfn_x(mfn) & ~mfn_color_mask) | (color))) /* * Parse the coloring configuration given in the buf string, following the diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index 3bb0c9221f..bf16703e24 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -610,10 +610,10 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) pte.pt.table = 1; xen_second[second_table_offset(FIXMAP_ADDR(0))] = pte; +#ifdef CONFIG_ARM_64 if ( llc_coloring_enabled ) ttbr = virt_to_maddr(virt_to_reloc_virt(xen_pgtable)); else -#ifdef CONFIG_ARM_64 ttbr = (uintptr_t) xen_pgtable + phys_offset; #else ttbr = (uintptr_t) cpu0_pgtable + phys_offset; diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h index 7cd481e955..516139c4ff 100644 --- a/xen/include/xen/llc-coloring.h +++ b/xen/include/xen/llc-coloring.h @@ -21,7 +21,27 @@ extern bool llc_coloring_enabled; #define llc_coloring_enabled (llc_coloring_enabled) #endif - +#else +static inline void *xen_remap_colored(mfn_t xen_fn, paddr_t xen_size) +{ + return NULL; +} +static inline int domain_set_llc_colors_from_str(struct domain *d, const char *str) +{ + return -ENOSYS; +} +static inline int dom0_set_llc_colors(struct domain *d) +{ + return 0; +} +static inline bool llc_coloring_init(void) +{ + return false; +} +static inline paddr_t xen_colored_map_size(paddr_t size) +{ + return 0; +} #endif #ifndef llc_coloring_enabled
Hi Stefano, On Thu, Jan 4, 2024 at 2:55 AM Stefano Stabellini <sstabellini@kernel.org> wrote: > > On Wed, 3 Jan 2024, Stefano Stabellini wrote: > > Also I tried this patch series on the "staging" branch and Xen failed to > > boot, no messages at all from Xen so it must be an early boot failure. I > > am passing this command line options to Xen and running it on QEMU: > > > > dom0_mem=1024M dom0_max_vcpus=1 xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=65535 llc-coloring=true > > I managed to make it to work successfully with the following command > line: > > xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=64K llc-coloring=on > > I think the problem was llc-way-size that needs to be rounded up (64K > instead of 65535). > > Also I found a few build issues when building for other architectures or > different kconfig options. This patch addresses those issues (however it > is not to be taken as is as the build issues should not be introduced in > the first place and there are probably better way to fix them, but I > hope it can help). > > Cheers, > > Stefano > > > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > index f434efc45b..efe5cf3c23 100644 > --- a/xen/arch/arm/llc-coloring.c > +++ b/xen/arch/arm/llc-coloring.c > @@ -39,7 +39,7 @@ static unsigned int __ro_after_init xen_num_colors; > > #define mfn_color_mask (nr_colors - 1) > #define mfn_to_color(mfn) (mfn_x(mfn) & mfn_color_mask) > -#define mfn_set_color(mfn, color) ((mfn_x(mfn) & ~mfn_color_mask) | (color)) > +#define mfn_set_color(mfn, color) (_mfn((mfn_x(mfn) & ~mfn_color_mask) | (color))) Thanks for spotting this. > /* > * Parse the coloring configuration given in the buf string, following the > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > index 3bb0c9221f..bf16703e24 100644 > --- a/xen/arch/arm/mm.c > +++ b/xen/arch/arm/mm.c > @@ -610,10 +610,10 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) > pte.pt.table = 1; > xen_second[second_table_offset(FIXMAP_ADDR(0))] = pte; > > +#ifdef CONFIG_ARM_64 > if ( llc_coloring_enabled ) > ttbr = virt_to_maddr(virt_to_reloc_virt(xen_pgtable)); > else > -#ifdef CONFIG_ARM_64 > ttbr = (uintptr_t) xen_pgtable + phys_offset; > #else > ttbr = (uintptr_t) cpu0_pgtable + phys_offset; > diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h > index 7cd481e955..516139c4ff 100644 > --- a/xen/include/xen/llc-coloring.h > +++ b/xen/include/xen/llc-coloring.h > @@ -21,7 +21,27 @@ > extern bool llc_coloring_enabled; > #define llc_coloring_enabled (llc_coloring_enabled) > #endif > - > +#else > +static inline void *xen_remap_colored(mfn_t xen_fn, paddr_t xen_size) > +{ > + return NULL; > +} > +static inline int domain_set_llc_colors_from_str(struct domain *d, const char *str) > +{ > + return -ENOSYS; > +} > +static inline int dom0_set_llc_colors(struct domain *d) > +{ > + return 0; > +} > +static inline bool llc_coloring_init(void) > +{ > + return false; > +} > +static inline paddr_t xen_colored_map_size(paddr_t size) > +{ > + return 0; > +} > #endif > > #ifndef llc_coloring_enabled Sorry for the compilation mess. This is definitely a solution, but I wonder if the best thing to do would be to move all signatures in the common header, without the stubs, relying again on DCE. This seems a little strange to me because users of some of those functions are only in arm code, and they always have to be protected with llc_coloring_enabled global variable/macro if, but it works (now I'm compiling also for arm32 and x86). Thanks.
On 04/01/2024 10:37, Carlo Nonato wrote: > > > Hi Stefano, > > On Thu, Jan 4, 2024 at 2:55 AM Stefano Stabellini > <sstabellini@kernel.org> wrote: >> >> On Wed, 3 Jan 2024, Stefano Stabellini wrote: >>> Also I tried this patch series on the "staging" branch and Xen failed to >>> boot, no messages at all from Xen so it must be an early boot failure. I >>> am passing this command line options to Xen and running it on QEMU: >>> >>> dom0_mem=1024M dom0_max_vcpus=1 xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=65535 llc-coloring=true >> >> I managed to make it to work successfully with the following command >> line: >> >> xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=64K llc-coloring=on >> >> I think the problem was llc-way-size that needs to be rounded up (64K >> instead of 65535). >> >> Also I found a few build issues when building for other architectures or >> different kconfig options. This patch addresses those issues (however it >> is not to be taken as is as the build issues should not be introduced in >> the first place and there are probably better way to fix them, but I >> hope it can help). >> >> Cheers, >> >> Stefano >> >> >> diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c >> index f434efc45b..efe5cf3c23 100644 >> --- a/xen/arch/arm/llc-coloring.c >> +++ b/xen/arch/arm/llc-coloring.c >> @@ -39,7 +39,7 @@ static unsigned int __ro_after_init xen_num_colors; >> >> #define mfn_color_mask (nr_colors - 1) >> #define mfn_to_color(mfn) (mfn_x(mfn) & mfn_color_mask) >> -#define mfn_set_color(mfn, color) ((mfn_x(mfn) & ~mfn_color_mask) | (color)) >> +#define mfn_set_color(mfn, color) (_mfn((mfn_x(mfn) & ~mfn_color_mask) | (color))) > > Thanks for spotting this. > >> /* >> * Parse the coloring configuration given in the buf string, following the >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c >> index 3bb0c9221f..bf16703e24 100644 >> --- a/xen/arch/arm/mm.c >> +++ b/xen/arch/arm/mm.c >> @@ -610,10 +610,10 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) >> pte.pt.table = 1; >> xen_second[second_table_offset(FIXMAP_ADDR(0))] = pte; >> >> +#ifdef CONFIG_ARM_64 >> if ( llc_coloring_enabled ) >> ttbr = virt_to_maddr(virt_to_reloc_virt(xen_pgtable)); >> else >> -#ifdef CONFIG_ARM_64 >> ttbr = (uintptr_t) xen_pgtable + phys_offset; >> #else >> ttbr = (uintptr_t) cpu0_pgtable + phys_offset; >> diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h >> index 7cd481e955..516139c4ff 100644 >> --- a/xen/include/xen/llc-coloring.h >> +++ b/xen/include/xen/llc-coloring.h >> @@ -21,7 +21,27 @@ >> extern bool llc_coloring_enabled; >> #define llc_coloring_enabled (llc_coloring_enabled) >> #endif >> - >> +#else >> +static inline void *xen_remap_colored(mfn_t xen_fn, paddr_t xen_size) >> +{ >> + return NULL; >> +} >> +static inline int domain_set_llc_colors_from_str(struct domain *d, const char *str) >> +{ >> + return -ENOSYS; >> +} >> +static inline int dom0_set_llc_colors(struct domain *d) >> +{ >> + return 0; >> +} >> +static inline bool llc_coloring_init(void) >> +{ >> + return false; >> +} >> +static inline paddr_t xen_colored_map_size(paddr_t size) >> +{ >> + return 0; >> +} >> #endif >> >> #ifndef llc_coloring_enabled > > Sorry for the compilation mess. > > This is definitely a solution, but I wonder if the best thing to do would be > to move all signatures in the common header, without the stubs, relying again > on DCE. This seems a little strange to me because users of some of those > functions are only in arm code, and they always have to be protected with > llc_coloring_enabled global variable/macro if, but it works (now I'm > compiling also for arm32 and x86). There are a lot of places in Xen relying on DCE, so I'd be ok with that (you can wait for Stefano's opinion). Furthermore, you already rely on that in case of e.g. domain_set_llc_colors_domctl, domain_llc_coloring_free. ~Michal
On Thu, 4 Jan 2024, Michal Orzel wrote: > On 04/01/2024 10:37, Carlo Nonato wrote: > > Hi Stefano, > > > > On Thu, Jan 4, 2024 at 2:55 AM Stefano Stabellini > > <sstabellini@kernel.org> wrote: > >> > >> On Wed, 3 Jan 2024, Stefano Stabellini wrote: > >>> Also I tried this patch series on the "staging" branch and Xen failed to > >>> boot, no messages at all from Xen so it must be an early boot failure. I > >>> am passing this command line options to Xen and running it on QEMU: > >>> > >>> dom0_mem=1024M dom0_max_vcpus=1 xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=65535 llc-coloring=true > >> > >> I managed to make it to work successfully with the following command > >> line: > >> > >> xen-llc-colors=0 dom0-llc-colors=1-5 llc-way-size=64K llc-coloring=on > >> > >> I think the problem was llc-way-size that needs to be rounded up (64K > >> instead of 65535). > >> > >> Also I found a few build issues when building for other architectures or > >> different kconfig options. This patch addresses those issues (however it > >> is not to be taken as is as the build issues should not be introduced in > >> the first place and there are probably better way to fix them, but I > >> hope it can help). > >> > >> Cheers, > >> > >> Stefano > >> > >> > >> diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > >> index f434efc45b..efe5cf3c23 100644 > >> --- a/xen/arch/arm/llc-coloring.c > >> +++ b/xen/arch/arm/llc-coloring.c > >> @@ -39,7 +39,7 @@ static unsigned int __ro_after_init xen_num_colors; > >> > >> #define mfn_color_mask (nr_colors - 1) > >> #define mfn_to_color(mfn) (mfn_x(mfn) & mfn_color_mask) > >> -#define mfn_set_color(mfn, color) ((mfn_x(mfn) & ~mfn_color_mask) | (color)) > >> +#define mfn_set_color(mfn, color) (_mfn((mfn_x(mfn) & ~mfn_color_mask) | (color))) > > > > Thanks for spotting this. > > > >> /* > >> * Parse the coloring configuration given in the buf string, following the > >> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c > >> index 3bb0c9221f..bf16703e24 100644 > >> --- a/xen/arch/arm/mm.c > >> +++ b/xen/arch/arm/mm.c > >> @@ -610,10 +610,10 @@ void __init setup_pagetables(unsigned long boot_phys_offset, paddr_t xen_paddr) > >> pte.pt.table = 1; > >> xen_second[second_table_offset(FIXMAP_ADDR(0))] = pte; > >> > >> +#ifdef CONFIG_ARM_64 > >> if ( llc_coloring_enabled ) > >> ttbr = virt_to_maddr(virt_to_reloc_virt(xen_pgtable)); > >> else > >> -#ifdef CONFIG_ARM_64 > >> ttbr = (uintptr_t) xen_pgtable + phys_offset; > >> #else > >> ttbr = (uintptr_t) cpu0_pgtable + phys_offset; > >> diff --git a/xen/include/xen/llc-coloring.h b/xen/include/xen/llc-coloring.h > >> index 7cd481e955..516139c4ff 100644 > >> --- a/xen/include/xen/llc-coloring.h > >> +++ b/xen/include/xen/llc-coloring.h > >> @@ -21,7 +21,27 @@ > >> extern bool llc_coloring_enabled; > >> #define llc_coloring_enabled (llc_coloring_enabled) > >> #endif > >> - > >> +#else > >> +static inline void *xen_remap_colored(mfn_t xen_fn, paddr_t xen_size) > >> +{ > >> + return NULL; > >> +} > >> +static inline int domain_set_llc_colors_from_str(struct domain *d, const char *str) > >> +{ > >> + return -ENOSYS; > >> +} > >> +static inline int dom0_set_llc_colors(struct domain *d) > >> +{ > >> + return 0; > >> +} > >> +static inline bool llc_coloring_init(void) > >> +{ > >> + return false; > >> +} > >> +static inline paddr_t xen_colored_map_size(paddr_t size) > >> +{ > >> + return 0; > >> +} > >> #endif > >> > >> #ifndef llc_coloring_enabled > > > > Sorry for the compilation mess. > > > > This is definitely a solution, but I wonder if the best thing to do would be > > to move all signatures in the common header, without the stubs, relying again > > on DCE. This seems a little strange to me because users of some of those > > functions are only in arm code, and they always have to be protected with > > llc_coloring_enabled global variable/macro if, but it works (now I'm > > compiling also for arm32 and x86). > There are a lot of places in Xen relying on DCE, so I'd be ok with that (you can wait > for Stefano's opinion). Furthermore, you already rely on that in case of e.g. domain_set_llc_colors_domctl, > domain_llc_coloring_free. Yeah I think that would be fine