| Message ID | 20240129171811.21382-3-carlo.nonato@minervasys.tech (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Arm cache coloring | expand |
Hi Carlo, On 29/01/2024 18:17, Carlo Nonato wrote: > > > LLC coloring needs to know the last level cache layout in order to make the > best use of it. This can be probed by inspecting the CLIDR_EL1 register, > so the Last Level is defined as the last level visible by this register. > Note that this excludes system caches in some platforms. > > Static memory allocation and cache coloring are incompatible because static > memory can't be guaranteed to use only colors assigned to the domain. > Panic during DomUs creation when both are enabled. > > Based on original work from: Luca Miccio <lucmiccio@gmail.com> > > Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech> > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > --- > v6: > - get_llc_way_size() now checks for at least separate I/D caches > v5: > - used - instead of _ for filenames > - moved static-mem check in this patch > - moved dom0 colors parsing in next patch > - moved color allocation and configuration in next patch > - moved check_colors() in next patch > - colors are now printed in short form > v4: > - added "llc-coloring" cmdline option for the boot-time switch > - dom0 colors are now checked during domain init as for any other domain > - fixed processor.h masks bit width > - check for overflow in parse_color_config() > - check_colors() now checks also that colors are sorted and unique > --- > docs/misc/cache-coloring.rst | 11 ++++ > xen/arch/arm/Kconfig | 1 + > xen/arch/arm/Makefile | 1 + > xen/arch/arm/dom0less-build.c | 6 +++ > xen/arch/arm/include/asm/processor.h | 16 ++++++ > xen/arch/arm/llc-coloring.c | 75 ++++++++++++++++++++++++++++ > xen/arch/arm/setup.c | 3 ++ > 7 files changed, 113 insertions(+) > create mode 100644 xen/arch/arm/llc-coloring.c > > diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst > index 9fe01e99e1..0535b5c656 100644 > --- a/docs/misc/cache-coloring.rst > +++ b/docs/misc/cache-coloring.rst > @@ -85,3 +85,14 @@ More specific documentation is available at `docs/misc/xen-command-line.pandoc`. > +----------------------+-------------------------------+ > | ``llc-way-size`` | set the LLC way size | > +----------------------+-------------------------------+ > + > +Known issues and limitations > +**************************** > + > +"xen,static-mem" isn't supported when coloring is enabled > +######################################################### > + > +In the domain configuration, "xen,static-mem" allows memory to be statically > +allocated to the domain. This isn't possible when LLC coloring is enabled, > +because that memory can't be guaranteed to use only colors assigned to the > +domain. > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > index 50e9bfae1a..55143f86a9 100644 > --- a/xen/arch/arm/Kconfig > +++ b/xen/arch/arm/Kconfig > @@ -8,6 +8,7 @@ config ARM_64 > depends on !ARM_32 > select 64BIT > select HAS_FAST_MULTIPLY > + select HAS_LLC_COLORING > > config ARM > def_bool y > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > index 33c677672f..c9a1cd298d 100644 > --- a/xen/arch/arm/Makefile > +++ b/xen/arch/arm/Makefile > @@ -37,6 +37,7 @@ obj-$(CONFIG_IOREQ_SERVER) += ioreq.o > obj-y += irq.o > obj-y += kernel.init.o > obj-$(CONFIG_LIVEPATCH) += livepatch.o > +obj-$(CONFIG_LLC_COLORING) += llc-coloring.o > obj-y += mem_access.o > obj-y += mm.o > obj-y += monitor.o > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index fb63ec6fd1..1142f7f74a 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -5,6 +5,7 @@ > #include <xen/grant_table.h> > #include <xen/iocap.h> > #include <xen/libfdt/libfdt.h> > +#include <xen/llc-coloring.h> > #include <xen/sched.h> > #include <xen/serial.h> > #include <xen/sizes.h> > @@ -879,7 +880,12 @@ void __init create_domUs(void) > panic("No more domain IDs available\n"); > > if ( dt_find_property(node, "xen,static-mem", NULL) ) > + { > + if ( llc_coloring_enabled ) > + panic("LLC coloring and static memory are incompatible\n"); > + > flags |= CDF_staticmem; > + } > > if ( dt_property_read_bool(node, "direct-map") ) > { > diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h > index 8e02410465..336933ee62 100644 > --- a/xen/arch/arm/include/asm/processor.h > +++ b/xen/arch/arm/include/asm/processor.h > @@ -18,6 +18,22 @@ > #define CTR_IDC_SHIFT 28 > #define CTR_DIC_SHIFT 29 > > +/* CCSIDR Current Cache Size ID Register */ > +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) Why ULL and not UL? ccsidr is of register_t type > +#define CCSIDR_NUMSETS_SHIFT 13 > +#define CCSIDR_NUMSETS_MASK _AC(0x3fff, ULL) > +#define CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX 32 > +#define CCSIDR_NUMSETS_MASK_FEAT_CCIDX _AC(0xffffff, ULL) > + > +/* CSSELR Cache Size Selection Register */ > +#define CSSELR_LEVEL_MASK _AC(0x7, UL) > +#define CSSELR_LEVEL_SHIFT 1 > + > +/* CLIDR Cache Level ID Register */ > +#define CLIDR_CTYPEn_SHIFT(n) (3 * (n - 1)) n should be within parentheses > +#define CLIDR_CTYPEn_MASK _AC(0x7, UL) > +#define CLIDR_CTYPEn_LEVELS 7 > + > #define ICACHE_POLICY_VPIPT 0 > #define ICACHE_POLICY_AIVIVT 1 > #define ICACHE_POLICY_VIPT 2 > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > new file mode 100644 > index 0000000000..eee1e80e2d > --- /dev/null > +++ b/xen/arch/arm/llc-coloring.c > @@ -0,0 +1,75 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Last Level Cache (LLC) coloring support for ARM > + * > + * Copyright (C) 2022 Xilinx Inc. > + */ > +#include <xen/llc-coloring.h> > +#include <xen/types.h> > + > +#include <asm/processor.h> > +#include <asm/sysregs.h> > + > +/* Return the LLC way size by probing the hardware */ > +unsigned int __init get_llc_way_size(void) > +{ > + register_t ccsidr_el1; > + register_t clidr_el1 = READ_SYSREG(CLIDR_EL1); > + register_t csselr_el1 = READ_SYSREG(CSSELR_EL1); > + register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1); > + uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT; > + uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK; > + unsigned int n, line_size, num_sets; > + > + for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- ) > + { > + uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) & > + CLIDR_CTYPEn_MASK; > + > + /* At least separate I/D caches (see Arm ARM DDI 0487H.a D13.2.27) */ I'm a bit confused here given that this comment does not reflect the line below (also please refer to the latest spec). Since 0b011 is "Separate instruction and data caches" you would break only for Unified cache. That said, we care about last level cache that is visible to SW and I'm not aware of any Arm CPU where L2,L3 is not unified. > + if ( ctype_n > 0b011 ) > + break; > + } > + > + if ( n == 0 ) > + return 0; > + > + WRITE_SYSREG((n - 1) << CSSELR_LEVEL_SHIFT, CSSELR_EL1); > + no need for this empty line > + isb(); > + > + ccsidr_el1 = READ_SYSREG(CCSIDR_EL1); > + > + /* Arm ARM: (Log2(Number of bytes in cache line)) - 4 */ > + line_size = 1U << ((ccsidr_el1 & CCSIDR_LINESIZE_MASK) + 4); > + > + /* If FEAT_CCIDX is enabled, CCSIDR_EL1 has a different bit layout */ > + if ( (id_aa64mmfr2_el1 >> ID_AA64MMFR2_CCIDX_SHIFT) & 0x7 ) > + { > + ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX; > + ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK_FEAT_CCIDX; > + } empty line here please > + /* Arm ARM: (Number of sets in cache) - 1 */ > + num_sets = ((ccsidr_el1 >> ccsidr_numsets_shift) & ccsidr_numsets_mask) + 1; > + > + printk(XENLOG_INFO "LLC found: L%u (line size: %u bytes, sets num: %u)\n", > + n, line_size, num_sets); > + > + /* Restore value in CSSELR_EL1 */ > + WRITE_SYSREG(csselr_el1, CSSELR_EL1); > + isb(); > + > + return line_size * num_sets; > +} > + > +void __init arch_llc_coloring_init(void) {} > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 59dd9bb25a..14cb023783 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -12,6 +12,7 @@ > #include <xen/device_tree.h> > #include <xen/domain_page.h> > #include <xen/grant_table.h> > +#include <xen/llc-coloring.h> > #include <xen/types.h> > #include <xen/string.h> > #include <xen/serial.h> > @@ -746,6 +747,8 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, > printk("Command line: %s\n", cmdline); > cmdline_parse(cmdline); > > + llc_coloring_init(); I think a check with llc_coloring_enabled is missing, given there is none in llc_coloring_init ~Michal
Hi Michal, On Wed, Feb 14, 2024 at 11:14 AM Michal Orzel <michal.orzel@amd.com> wrote: > > Hi Carlo, > > On 29/01/2024 18:17, Carlo Nonato wrote: > > > > > > LLC coloring needs to know the last level cache layout in order to make the > > best use of it. This can be probed by inspecting the CLIDR_EL1 register, > > so the Last Level is defined as the last level visible by this register. > > Note that this excludes system caches in some platforms. > > > > Static memory allocation and cache coloring are incompatible because static > > memory can't be guaranteed to use only colors assigned to the domain. > > Panic during DomUs creation when both are enabled. > > > > Based on original work from: Luca Miccio <lucmiccio@gmail.com> > > > > Signed-off-by: Carlo Nonato <carlo.nonato@minervasys.tech> > > Signed-off-by: Marco Solieri <marco.solieri@minervasys.tech> > > --- > > v6: > > - get_llc_way_size() now checks for at least separate I/D caches > > v5: > > - used - instead of _ for filenames > > - moved static-mem check in this patch > > - moved dom0 colors parsing in next patch > > - moved color allocation and configuration in next patch > > - moved check_colors() in next patch > > - colors are now printed in short form > > v4: > > - added "llc-coloring" cmdline option for the boot-time switch > > - dom0 colors are now checked during domain init as for any other domain > > - fixed processor.h masks bit width > > - check for overflow in parse_color_config() > > - check_colors() now checks also that colors are sorted and unique > > --- > > docs/misc/cache-coloring.rst | 11 ++++ > > xen/arch/arm/Kconfig | 1 + > > xen/arch/arm/Makefile | 1 + > > xen/arch/arm/dom0less-build.c | 6 +++ > > xen/arch/arm/include/asm/processor.h | 16 ++++++ > > xen/arch/arm/llc-coloring.c | 75 ++++++++++++++++++++++++++++ > > xen/arch/arm/setup.c | 3 ++ > > 7 files changed, 113 insertions(+) > > create mode 100644 xen/arch/arm/llc-coloring.c > > > > diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst > > index 9fe01e99e1..0535b5c656 100644 > > --- a/docs/misc/cache-coloring.rst > > +++ b/docs/misc/cache-coloring.rst > > @@ -85,3 +85,14 @@ More specific documentation is available at `docs/misc/xen-command-line.pandoc`. > > +----------------------+-------------------------------+ > > | ``llc-way-size`` | set the LLC way size | > > +----------------------+-------------------------------+ > > + > > +Known issues and limitations > > +**************************** > > + > > +"xen,static-mem" isn't supported when coloring is enabled > > +######################################################### > > + > > +In the domain configuration, "xen,static-mem" allows memory to be statically > > +allocated to the domain. This isn't possible when LLC coloring is enabled, > > +because that memory can't be guaranteed to use only colors assigned to the > > +domain. > > diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig > > index 50e9bfae1a..55143f86a9 100644 > > --- a/xen/arch/arm/Kconfig > > +++ b/xen/arch/arm/Kconfig > > @@ -8,6 +8,7 @@ config ARM_64 > > depends on !ARM_32 > > select 64BIT > > select HAS_FAST_MULTIPLY > > + select HAS_LLC_COLORING > > > > config ARM > > def_bool y > > diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile > > index 33c677672f..c9a1cd298d 100644 > > --- a/xen/arch/arm/Makefile > > +++ b/xen/arch/arm/Makefile > > @@ -37,6 +37,7 @@ obj-$(CONFIG_IOREQ_SERVER) += ioreq.o > > obj-y += irq.o > > obj-y += kernel.init.o > > obj-$(CONFIG_LIVEPATCH) += livepatch.o > > +obj-$(CONFIG_LLC_COLORING) += llc-coloring.o > > obj-y += mem_access.o > > obj-y += mm.o > > obj-y += monitor.o > > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > > index fb63ec6fd1..1142f7f74a 100644 > > --- a/xen/arch/arm/dom0less-build.c > > +++ b/xen/arch/arm/dom0less-build.c > > @@ -5,6 +5,7 @@ > > #include <xen/grant_table.h> > > #include <xen/iocap.h> > > #include <xen/libfdt/libfdt.h> > > +#include <xen/llc-coloring.h> > > #include <xen/sched.h> > > #include <xen/serial.h> > > #include <xen/sizes.h> > > @@ -879,7 +880,12 @@ void __init create_domUs(void) > > panic("No more domain IDs available\n"); > > > > if ( dt_find_property(node, "xen,static-mem", NULL) ) > > + { > > + if ( llc_coloring_enabled ) > > + panic("LLC coloring and static memory are incompatible\n"); > > + > > flags |= CDF_staticmem; > > + } > > > > if ( dt_property_read_bool(node, "direct-map") ) > > { > > diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h > > index 8e02410465..336933ee62 100644 > > --- a/xen/arch/arm/include/asm/processor.h > > +++ b/xen/arch/arm/include/asm/processor.h > > @@ -18,6 +18,22 @@ > > #define CTR_IDC_SHIFT 28 > > #define CTR_DIC_SHIFT 29 > > > > +/* CCSIDR Current Cache Size ID Register */ > > +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) > Why ULL and not UL? ccsidr is of register_t type Julien, while reviewing an earlier version: > Please use ULL here otherwise someone using MASK << SHIFT will have the > expected result. https://patchew.org/Xen/20220826125111.152261-1-carlo.nonato@minervasys.tech/20220826125111.152261-2-carlo.nonato@minervasys.tech/#08956082-c194-8bae-cb25-44e4e3227689@xen.org > > > +#define CCSIDR_NUMSETS_SHIFT 13 > > +#define CCSIDR_NUMSETS_MASK _AC(0x3fff, ULL) > > +#define CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX 32 > > +#define CCSIDR_NUMSETS_MASK_FEAT_CCIDX _AC(0xffffff, ULL) > > + > > +/* CSSELR Cache Size Selection Register */ > > +#define CSSELR_LEVEL_MASK _AC(0x7, UL) > > +#define CSSELR_LEVEL_SHIFT 1 > > + > > +/* CLIDR Cache Level ID Register */ > > +#define CLIDR_CTYPEn_SHIFT(n) (3 * (n - 1)) > n should be within parentheses > > > +#define CLIDR_CTYPEn_MASK _AC(0x7, UL) > > +#define CLIDR_CTYPEn_LEVELS 7 > > + > > #define ICACHE_POLICY_VPIPT 0 > > #define ICACHE_POLICY_AIVIVT 1 > > #define ICACHE_POLICY_VIPT 2 > > diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c > > new file mode 100644 > > index 0000000000..eee1e80e2d > > --- /dev/null > > +++ b/xen/arch/arm/llc-coloring.c > > @@ -0,0 +1,75 @@ > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > +/* > > + * Last Level Cache (LLC) coloring support for ARM > > + * > > + * Copyright (C) 2022 Xilinx Inc. > > + */ > > +#include <xen/llc-coloring.h> > > +#include <xen/types.h> > > + > > +#include <asm/processor.h> > > +#include <asm/sysregs.h> > > + > > +/* Return the LLC way size by probing the hardware */ > > +unsigned int __init get_llc_way_size(void) > > +{ > > + register_t ccsidr_el1; > > + register_t clidr_el1 = READ_SYSREG(CLIDR_EL1); > > + register_t csselr_el1 = READ_SYSREG(CSSELR_EL1); > > + register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1); > > + uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT; > > + uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK; > > + unsigned int n, line_size, num_sets; > > + > > + for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- ) > > + { > > + uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) & > > + CLIDR_CTYPEn_MASK; > > + > > + /* At least separate I/D caches (see Arm ARM DDI 0487H.a D13.2.27) */ > I'm a bit confused here given that this comment does not reflect the line below (also please refer to the latest spec). > Since 0b011 is "Separate instruction and data caches" you would break only for Unified cache. > That said, we care about last level cache that is visible to SW and I'm not aware of any Arm CPU where L2,L3 is not unified. You're right, that should have been >=. Anyway I can check more explicitly for == 0b100. > > + if ( ctype_n > 0b011 ) > > + break; > > + } > > + > > + if ( n == 0 ) > > + return 0; > > + > > + WRITE_SYSREG((n - 1) << CSSELR_LEVEL_SHIFT, CSSELR_EL1); > > + > no need for this empty line > > > + isb(); > > + > > + ccsidr_el1 = READ_SYSREG(CCSIDR_EL1); > > + > > + /* Arm ARM: (Log2(Number of bytes in cache line)) - 4 */ > > + line_size = 1U << ((ccsidr_el1 & CCSIDR_LINESIZE_MASK) + 4); > > + > > + /* If FEAT_CCIDX is enabled, CCSIDR_EL1 has a different bit layout */ > > + if ( (id_aa64mmfr2_el1 >> ID_AA64MMFR2_CCIDX_SHIFT) & 0x7 ) > > + { > > + ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX; > > + ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK_FEAT_CCIDX; > > + } > empty line here please > > > + /* Arm ARM: (Number of sets in cache) - 1 */ > > + num_sets = ((ccsidr_el1 >> ccsidr_numsets_shift) & ccsidr_numsets_mask) + 1; > > + > > + printk(XENLOG_INFO "LLC found: L%u (line size: %u bytes, sets num: %u)\n", > > + n, line_size, num_sets); > > + > > + /* Restore value in CSSELR_EL1 */ > > + WRITE_SYSREG(csselr_el1, CSSELR_EL1); > > + isb(); > > + > > + return line_size * num_sets; > > +} > > + > > +void __init arch_llc_coloring_init(void) {} > > + > > +/* > > + * Local variables: > > + * mode: C > > + * c-file-style: "BSD" > > + * c-basic-offset: 4 > > + * tab-width: 4 > > + * indent-tabs-mode: nil > > + * End: > > + */ > > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > > index 59dd9bb25a..14cb023783 100644 > > --- a/xen/arch/arm/setup.c > > +++ b/xen/arch/arm/setup.c > > @@ -12,6 +12,7 @@ > > #include <xen/device_tree.h> > > #include <xen/domain_page.h> > > #include <xen/grant_table.h> > > +#include <xen/llc-coloring.h> > > #include <xen/types.h> > > #include <xen/string.h> > > #include <xen/serial.h> > > @@ -746,6 +747,8 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, > > printk("Command line: %s\n", cmdline); > > cmdline_parse(cmdline); > > > > + llc_coloring_init(); > I think a check with llc_coloring_enabled is missing, given there is none in llc_coloring_init You're right. It should have been in llc_coloring_init(), my bad. Thanks. > ~Michal >
Hi, On 14/02/2024 13:52, Carlo Nonato wrote: > On Wed, Feb 14, 2024 at 11:14 AM Michal Orzel <michal.orzel@amd.com> wrote: >>> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h >>> index 8e02410465..336933ee62 100644 >>> --- a/xen/arch/arm/include/asm/processor.h >>> +++ b/xen/arch/arm/include/asm/processor.h >>> @@ -18,6 +18,22 @@ >>> #define CTR_IDC_SHIFT 28 >>> #define CTR_DIC_SHIFT 29 >>> >>> +/* CCSIDR Current Cache Size ID Register */ >>> +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) >> Why ULL and not UL? ccsidr is of register_t type > > Julien, while reviewing an earlier version: > >> Please use ULL here otherwise someone using MASK << SHIFT will have the >> expected result. > > https://patchew.org/Xen/20220826125111.152261-1-carlo.nonato@minervasys.tech/20220826125111.152261-2-carlo.nonato@minervasys.tech/#08956082-c194-8bae-cb25-44e4e3227689@xen.org Michal is right. This should be UL. Not sure why I suggested ULL back then. Sorry. Cheers,
Hi Julien On Tue, Feb 20, 2024 at 12:06 AM Julien Grall <julien@xen.org> wrote: > > Hi, > > On 14/02/2024 13:52, Carlo Nonato wrote: > > On Wed, Feb 14, 2024 at 11:14 AM Michal Orzel <michal.orzel@amd.com> wrote: > >>> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h > >>> index 8e02410465..336933ee62 100644 > >>> --- a/xen/arch/arm/include/asm/processor.h > >>> +++ b/xen/arch/arm/include/asm/processor.h > >>> @@ -18,6 +18,22 @@ > >>> #define CTR_IDC_SHIFT 28 > >>> #define CTR_DIC_SHIFT 29 > >>> > >>> +/* CCSIDR Current Cache Size ID Register */ > >>> +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) > >> Why ULL and not UL? ccsidr is of register_t type > > > > Julien, while reviewing an earlier version: > > > >> Please use ULL here otherwise someone using MASK << SHIFT will have the > >> expected result. > > > > https://patchew.org/Xen/20220826125111.152261-1-carlo.nonato@minervasys.tech/20220826125111.152261-2-carlo.nonato@minervasys.tech/#08956082-c194-8bae-cb25-44e4e3227689@xen.org > > Michal is right. This should be UL. Not sure why I suggested ULL back > then. Sorry. No problem. If there aren't any other comments I will proceed with sending the v7. Do you guys want to add something on the arm part? Thanks to both. > Cheers, > > -- > Julien Grall
Hi Carlo, On 20/02/2024 09:16, Carlo Nonato wrote: > Hi Julien > > On Tue, Feb 20, 2024 at 12:06 AM Julien Grall <julien@xen.org> wrote: >> >> Hi, >> >> On 14/02/2024 13:52, Carlo Nonato wrote: >>> On Wed, Feb 14, 2024 at 11:14 AM Michal Orzel <michal.orzel@amd.com> wrote: >>>>> diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h >>>>> index 8e02410465..336933ee62 100644 >>>>> --- a/xen/arch/arm/include/asm/processor.h >>>>> +++ b/xen/arch/arm/include/asm/processor.h >>>>> @@ -18,6 +18,22 @@ >>>>> #define CTR_IDC_SHIFT 28 >>>>> #define CTR_DIC_SHIFT 29 >>>>> >>>>> +/* CCSIDR Current Cache Size ID Register */ >>>>> +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) >>>> Why ULL and not UL? ccsidr is of register_t type >>> >>> Julien, while reviewing an earlier version: >>> >>>> Please use ULL here otherwise someone using MASK << SHIFT will have the >>>> expected result. >>> >>> https://patchew.org/Xen/20220826125111.152261-1-carlo.nonato@minervasys.tech/20220826125111.152261-2-carlo.nonato@minervasys.tech/#08956082-c194-8bae-cb25-44e4e3227689@xen.org >> >> Michal is right. This should be UL. Not sure why I suggested ULL back >> then. Sorry. > > No problem. > > If there aren't any other comments I will proceed with sending the v7. > Do you guys want to add something on the arm part? I haven't yet had the chance to fully review the series. It is in my TODO list though. Cheers,
diff --git a/docs/misc/cache-coloring.rst b/docs/misc/cache-coloring.rst index 9fe01e99e1..0535b5c656 100644 --- a/docs/misc/cache-coloring.rst +++ b/docs/misc/cache-coloring.rst @@ -85,3 +85,14 @@ More specific documentation is available at `docs/misc/xen-command-line.pandoc`. +----------------------+-------------------------------+ | ``llc-way-size`` | set the LLC way size | +----------------------+-------------------------------+ + +Known issues and limitations +**************************** + +"xen,static-mem" isn't supported when coloring is enabled +######################################################### + +In the domain configuration, "xen,static-mem" allows memory to be statically +allocated to the domain. This isn't possible when LLC coloring is enabled, +because that memory can't be guaranteed to use only colors assigned to the +domain. diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig index 50e9bfae1a..55143f86a9 100644 --- a/xen/arch/arm/Kconfig +++ b/xen/arch/arm/Kconfig @@ -8,6 +8,7 @@ config ARM_64 depends on !ARM_32 select 64BIT select HAS_FAST_MULTIPLY + select HAS_LLC_COLORING config ARM def_bool y diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile index 33c677672f..c9a1cd298d 100644 --- a/xen/arch/arm/Makefile +++ b/xen/arch/arm/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_IOREQ_SERVER) += ioreq.o obj-y += irq.o obj-y += kernel.init.o obj-$(CONFIG_LIVEPATCH) += livepatch.o +obj-$(CONFIG_LLC_COLORING) += llc-coloring.o obj-y += mem_access.o obj-y += mm.o obj-y += monitor.o diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index fb63ec6fd1..1142f7f74a 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -5,6 +5,7 @@ #include <xen/grant_table.h> #include <xen/iocap.h> #include <xen/libfdt/libfdt.h> +#include <xen/llc-coloring.h> #include <xen/sched.h> #include <xen/serial.h> #include <xen/sizes.h> @@ -879,7 +880,12 @@ void __init create_domUs(void) panic("No more domain IDs available\n"); if ( dt_find_property(node, "xen,static-mem", NULL) ) + { + if ( llc_coloring_enabled ) + panic("LLC coloring and static memory are incompatible\n"); + flags |= CDF_staticmem; + } if ( dt_property_read_bool(node, "direct-map") ) { diff --git a/xen/arch/arm/include/asm/processor.h b/xen/arch/arm/include/asm/processor.h index 8e02410465..336933ee62 100644 --- a/xen/arch/arm/include/asm/processor.h +++ b/xen/arch/arm/include/asm/processor.h @@ -18,6 +18,22 @@ #define CTR_IDC_SHIFT 28 #define CTR_DIC_SHIFT 29 +/* CCSIDR Current Cache Size ID Register */ +#define CCSIDR_LINESIZE_MASK _AC(0x7, ULL) +#define CCSIDR_NUMSETS_SHIFT 13 +#define CCSIDR_NUMSETS_MASK _AC(0x3fff, ULL) +#define CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX 32 +#define CCSIDR_NUMSETS_MASK_FEAT_CCIDX _AC(0xffffff, ULL) + +/* CSSELR Cache Size Selection Register */ +#define CSSELR_LEVEL_MASK _AC(0x7, UL) +#define CSSELR_LEVEL_SHIFT 1 + +/* CLIDR Cache Level ID Register */ +#define CLIDR_CTYPEn_SHIFT(n) (3 * (n - 1)) +#define CLIDR_CTYPEn_MASK _AC(0x7, UL) +#define CLIDR_CTYPEn_LEVELS 7 + #define ICACHE_POLICY_VPIPT 0 #define ICACHE_POLICY_AIVIVT 1 #define ICACHE_POLICY_VIPT 2 diff --git a/xen/arch/arm/llc-coloring.c b/xen/arch/arm/llc-coloring.c new file mode 100644 index 0000000000..eee1e80e2d --- /dev/null +++ b/xen/arch/arm/llc-coloring.c @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Last Level Cache (LLC) coloring support for ARM + * + * Copyright (C) 2022 Xilinx Inc. + */ +#include <xen/llc-coloring.h> +#include <xen/types.h> + +#include <asm/processor.h> +#include <asm/sysregs.h> + +/* Return the LLC way size by probing the hardware */ +unsigned int __init get_llc_way_size(void) +{ + register_t ccsidr_el1; + register_t clidr_el1 = READ_SYSREG(CLIDR_EL1); + register_t csselr_el1 = READ_SYSREG(CSSELR_EL1); + register_t id_aa64mmfr2_el1 = READ_SYSREG(ID_AA64MMFR2_EL1); + uint32_t ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT; + uint32_t ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK; + unsigned int n, line_size, num_sets; + + for ( n = CLIDR_CTYPEn_LEVELS; n != 0; n-- ) + { + uint8_t ctype_n = (clidr_el1 >> CLIDR_CTYPEn_SHIFT(n)) & + CLIDR_CTYPEn_MASK; + + /* At least separate I/D caches (see Arm ARM DDI 0487H.a D13.2.27) */ + if ( ctype_n > 0b011 ) + break; + } + + if ( n == 0 ) + return 0; + + WRITE_SYSREG((n - 1) << CSSELR_LEVEL_SHIFT, CSSELR_EL1); + + isb(); + + ccsidr_el1 = READ_SYSREG(CCSIDR_EL1); + + /* Arm ARM: (Log2(Number of bytes in cache line)) - 4 */ + line_size = 1U << ((ccsidr_el1 & CCSIDR_LINESIZE_MASK) + 4); + + /* If FEAT_CCIDX is enabled, CCSIDR_EL1 has a different bit layout */ + if ( (id_aa64mmfr2_el1 >> ID_AA64MMFR2_CCIDX_SHIFT) & 0x7 ) + { + ccsidr_numsets_shift = CCSIDR_NUMSETS_SHIFT_FEAT_CCIDX; + ccsidr_numsets_mask = CCSIDR_NUMSETS_MASK_FEAT_CCIDX; + } + /* Arm ARM: (Number of sets in cache) - 1 */ + num_sets = ((ccsidr_el1 >> ccsidr_numsets_shift) & ccsidr_numsets_mask) + 1; + + printk(XENLOG_INFO "LLC found: L%u (line size: %u bytes, sets num: %u)\n", + n, line_size, num_sets); + + /* Restore value in CSSELR_EL1 */ + WRITE_SYSREG(csselr_el1, CSSELR_EL1); + isb(); + + return line_size * num_sets; +} + +void __init arch_llc_coloring_init(void) {} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index 59dd9bb25a..14cb023783 100644 --- a/xen/arch/arm/setup.c +++ b/xen/arch/arm/setup.c @@ -12,6 +12,7 @@ #include <xen/device_tree.h> #include <xen/domain_page.h> #include <xen/grant_table.h> +#include <xen/llc-coloring.h> #include <xen/types.h> #include <xen/string.h> #include <xen/serial.h> @@ -746,6 +747,8 @@ void asmlinkage __init start_xen(unsigned long boot_phys_offset, printk("Command line: %s\n", cmdline); cmdline_parse(cmdline); + llc_coloring_init(); + setup_mm(); /* Parse the ACPI tables for possible boot-time configuration */