[PATCH v5 00/13] Arm cache coloring

Carlo Nonato posted 13 patches 3 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20240102095138.17933-1-carlo.nonato@minervasys.tech
There is a newer version of this series
docs/man/xl.cfg.5.pod.in                |  10 +
docs/misc/arm/cache-coloring.rst        | 209 ++++++++++++
docs/misc/arm/device-tree/booting.txt   |   4 +
docs/misc/xen-command-line.pandoc       |  61 ++++
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 ++-
xen/arch/Kconfig                        |  28 ++
xen/arch/arm/Kconfig                    |   1 +
xen/arch/arm/Makefile                   |   1 +
xen/arch/arm/alternative.c              |   9 +-
xen/arch/arm/arm64/mmu/head.S           |  48 +++
xen/arch/arm/arm64/mmu/mm.c             |  26 +-
xen/arch/arm/dom0less-build.c           |  19 ++
xen/arch/arm/domain_build.c             |  60 +++-
xen/arch/arm/include/asm/llc-coloring.h |  46 +++
xen/arch/arm/include/asm/mm.h           |  12 +-
xen/arch/arm/include/asm/mmu/layout.h   |   2 +
xen/arch/arm/include/asm/processor.h    |  16 +
xen/arch/arm/llc-coloring.c             | 409 ++++++++++++++++++++++++
xen/arch/arm/mmu/p2m.c                  |   4 +-
xen/arch/arm/mmu/setup.c                |  83 ++++-
xen/arch/arm/mmu/smpboot.c              |  11 +-
xen/arch/arm/psci.c                     |   9 +-
xen/arch/arm/setup.c                    | 172 +++++++++-
xen/arch/arm/smpboot.c                  |   9 +-
xen/common/Kconfig                      |   3 +
xen/common/domain.c                     |   4 +
xen/common/domctl.c                     |  11 +
xen/common/keyhandler.c                 |   4 +
xen/common/page_alloc.c                 | 217 ++++++++++++-
xen/include/public/domctl.h             |  10 +-
xen/include/xen/llc-coloring.h          |  53 +++
xen/include/xen/sched.h                 |   5 +
37 files changed, 1610 insertions(+), 42 deletions(-)
create mode 100644 docs/misc/arm/cache-coloring.rst
create mode 100644 xen/arch/arm/include/asm/llc-coloring.h
create mode 100644 xen/arch/arm/llc-coloring.c
create mode 100644 xen/include/xen/llc-coloring.h
[PATCH v5 00/13] Arm cache coloring
Posted by Carlo Nonato 3 months, 3 weeks ago
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.
 - #13: consider_modules() has been moved to arm32 only. Again I just copied it.

Carlo Nonato (12):
  xen/common: add cache coloring common code
  xen/arm: add cache coloring initialization
  xen/arm: add Dom0 cache coloring support
  xen: extend domctl interface for cache coloring
  tools: add support for cache coloring configuration
  xen/arm: add support for cache coloring configuration via device-tree
  xen/page_alloc: introduce init_free_page_fields() helper
  xen/page_alloc: introduce preserved page flags macro
  xen: add cache coloring allocator for domains
  xen/arm: use domain memory to allocate p2m page tables
  Revert "xen/arm: Remove unused BOOT_RELOC_VIRT_START"
  xen/arm: add cache coloring support for Xen

Luca Miccio (1):
  xen/arm: add Xen cache colors command line parameter

 docs/man/xl.cfg.5.pod.in                |  10 +
 docs/misc/arm/cache-coloring.rst        | 209 ++++++++++++
 docs/misc/arm/device-tree/booting.txt   |   4 +
 docs/misc/xen-command-line.pandoc       |  61 ++++
 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 ++-
 xen/arch/Kconfig                        |  28 ++
 xen/arch/arm/Kconfig                    |   1 +
 xen/arch/arm/Makefile                   |   1 +
 xen/arch/arm/alternative.c              |   9 +-
 xen/arch/arm/arm64/mmu/head.S           |  48 +++
 xen/arch/arm/arm64/mmu/mm.c             |  26 +-
 xen/arch/arm/dom0less-build.c           |  19 ++
 xen/arch/arm/domain_build.c             |  60 +++-
 xen/arch/arm/include/asm/llc-coloring.h |  46 +++
 xen/arch/arm/include/asm/mm.h           |  12 +-
 xen/arch/arm/include/asm/mmu/layout.h   |   2 +
 xen/arch/arm/include/asm/processor.h    |  16 +
 xen/arch/arm/llc-coloring.c             | 409 ++++++++++++++++++++++++
 xen/arch/arm/mmu/p2m.c                  |   4 +-
 xen/arch/arm/mmu/setup.c                |  83 ++++-
 xen/arch/arm/mmu/smpboot.c              |  11 +-
 xen/arch/arm/psci.c                     |   9 +-
 xen/arch/arm/setup.c                    | 172 +++++++++-
 xen/arch/arm/smpboot.c                  |   9 +-
 xen/common/Kconfig                      |   3 +
 xen/common/domain.c                     |   4 +
 xen/common/domctl.c                     |  11 +
 xen/common/keyhandler.c                 |   4 +
 xen/common/page_alloc.c                 | 217 ++++++++++++-
 xen/include/public/domctl.h             |  10 +-
 xen/include/xen/llc-coloring.h          |  53 +++
 xen/include/xen/sched.h                 |   5 +
 37 files changed, 1610 insertions(+), 42 deletions(-)
 create mode 100644 docs/misc/arm/cache-coloring.rst
 create mode 100644 xen/arch/arm/include/asm/llc-coloring.h
 create mode 100644 xen/arch/arm/llc-coloring.c
 create mode 100644 xen/include/xen/llc-coloring.h

-- 
2.34.1
Re: [PATCH v5 00/13] Arm cache coloring
Posted by Michal Orzel 3 months, 3 weeks ago
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
Re: [PATCH v5 00/13] Arm cache coloring
Posted by Stefano Stabellini 3 months, 3 weeks ago
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
Re: [PATCH v5 00/13] Arm cache coloring
Posted by Stefano Stabellini 3 months, 3 weeks ago
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
Re: [PATCH v5 00/13] Arm cache coloring
Posted by Carlo Nonato 3 months, 3 weeks ago
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.
Re: [PATCH v5 00/13] Arm cache coloring
Posted by Michal Orzel 3 months, 3 weeks ago

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

Re: [PATCH v5 00/13] Arm cache coloring
Posted by Stefano Stabellini 3 months, 3 weeks ago
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