Re: [PATCH RESEND 14/18] i386: Add cache topology info in CPUCacheInfo

[wangyanan (Y)]

Hi Zhao,

在 2023/2/13 17:36, Zhao Liu 写道:
> From: Zhao Liu <zhao1.liu@intel.com>
>
> Currently, by default, the cache topology is encoded as:
> 1. i/d cache is shared in one core.
> 2. L2 cache is shared in one core.
> 3. L3 cache is shared in one die.
>
> This default general setting has caused a misunderstanding, that is, the
> cache topology is completely equated with a specific cpu topology, such
> as the connection between L2 cache and core level, and the connection
> between L3 cache and die level.
>
> In fact, the settings of these topologies depend on the specific
> platform and are not static. For example, on Alder Lake-P, every
> four Atom cores share the same L2 cache.
>
> Thus, we should explicitly define the corresponding cache topology for
> different cache models to increase scalability.
>
> Except legacy_l2_cache_cpuid2 (its default topo level is INVALID),
It seems like its default topo level is UNKNOWN in this case.
> explicitly set the corresponding topology level for all other cache
> models. In order to be compatible with the existing cache topology, set
> the CORE level for the i/d cache, set the CORE level for L2 cache, and
> set the DIE level for L3 cache.
>
> The field for CPUID[4].EAX[bits 25:14] or CPUID[0x8000001D].EAX[bits
> 25:14] will be set based on CPUCacheInfo.share_level.
>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
>   target/i386/cpu.c | 19 +++++++++++++++++++
>   target/i386/cpu.h | 16 ++++++++++++++++
>   2 files changed, 35 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 27bbbc36b11c..364534e84b1b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -433,6 +433,7 @@ static CPUCacheInfo legacy_l1d_cache = {
>       .sets = 64,
>       .partitions = 1,
>       .no_invd_sharing = true,
> +    .share_level = CORE,
>   };
>   
>   /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
> @@ -447,6 +448,7 @@ static CPUCacheInfo legacy_l1d_cache_amd = {
>       .partitions = 1,
>       .lines_per_tag = 1,
>       .no_invd_sharing = true,
> +    .share_level = CORE,
>   };
>   
>   /* L1 instruction cache: */
> @@ -460,6 +462,7 @@ static CPUCacheInfo legacy_l1i_cache = {
>       .sets = 64,
>       .partitions = 1,
>       .no_invd_sharing = true,
> +    .share_level = CORE,
>   };
>   
>   /*FIXME: CPUID leaf 0x80000005 is inconsistent with leaves 2 & 4 */
> @@ -474,6 +477,7 @@ static CPUCacheInfo legacy_l1i_cache_amd = {
>       .partitions = 1,
>       .lines_per_tag = 1,
>       .no_invd_sharing = true,
> +    .share_level = CORE,
>   };
>   
>   /* Level 2 unified cache: */
> @@ -487,6 +491,7 @@ static CPUCacheInfo legacy_l2_cache = {
>       .sets = 4096,
>       .partitions = 1,
>       .no_invd_sharing = true,
> +    .share_level = CORE,
>   };
>   
>   /*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
> @@ -509,6 +514,7 @@ static CPUCacheInfo legacy_l2_cache_amd = {
>       .associativity = 16,
>       .sets = 512,
>       .partitions = 1,
> +    .share_level = CORE,
>   };
>   
>   /* Level 3 unified cache: */
> @@ -524,6 +530,7 @@ static CPUCacheInfo legacy_l3_cache = {
>       .self_init = true,
>       .inclusive = true,
>       .complex_indexing = true,
> +    .share_level = DIE,
>   };
>   
>   /* TLB definitions: */
> @@ -1668,6 +1675,7 @@ static const CPUCaches epyc_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l1i_cache = &(CPUCacheInfo) {
>           .type = INSTRUCTION_CACHE,
> @@ -1680,6 +1688,7 @@ static const CPUCaches epyc_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l2_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1690,6 +1699,7 @@ static const CPUCaches epyc_cache_info = {
>           .partitions = 1,
>           .sets = 1024,
>           .lines_per_tag = 1,
> +        .share_level = CORE,
>       },
>       .l3_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1703,6 +1713,7 @@ static const CPUCaches epyc_cache_info = {
>           .self_init = true,
>           .inclusive = true,
>           .complex_indexing = true,
> +        .share_level = DIE,
>       },
>   };
>   
> @@ -1718,6 +1729,7 @@ static const CPUCaches epyc_rome_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l1i_cache = &(CPUCacheInfo) {
>           .type = INSTRUCTION_CACHE,
> @@ -1730,6 +1742,7 @@ static const CPUCaches epyc_rome_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l2_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1740,6 +1753,7 @@ static const CPUCaches epyc_rome_cache_info = {
>           .partitions = 1,
>           .sets = 1024,
>           .lines_per_tag = 1,
> +        .share_level = CORE,
>       },
>       .l3_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1753,6 +1767,7 @@ static const CPUCaches epyc_rome_cache_info = {
>           .self_init = true,
>           .inclusive = true,
>           .complex_indexing = true,
> +        .share_level = DIE,
>       },
>   };
>   
> @@ -1768,6 +1783,7 @@ static const CPUCaches epyc_milan_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l1i_cache = &(CPUCacheInfo) {
>           .type = INSTRUCTION_CACHE,
> @@ -1780,6 +1796,7 @@ static const CPUCaches epyc_milan_cache_info = {
>           .lines_per_tag = 1,
>           .self_init = 1,
>           .no_invd_sharing = true,
> +        .share_level = CORE,
>       },
>       .l2_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1790,6 +1807,7 @@ static const CPUCaches epyc_milan_cache_info = {
>           .partitions = 1,
>           .sets = 1024,
>           .lines_per_tag = 1,
> +        .share_level = CORE,
>       },
>       .l3_cache = &(CPUCacheInfo) {
>           .type = UNIFIED_CACHE,
> @@ -1803,6 +1821,7 @@ static const CPUCaches epyc_milan_cache_info = {
>           .self_init = true,
>           .inclusive = true,
>           .complex_indexing = true,
> +        .share_level = DIE,
>       },
>   };
>   
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 8668e74e0c87..5a955431f759 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1476,6 +1476,15 @@ enum CacheType {
>       UNIFIED_CACHE
>   };
>   
> +enum CPUTopoLevel {
> +    INVALID = 0,
Maybe UNKNOWN?
> +    SMT,
> +    CORE,
> +    MODULE,
> +    DIE,
> +    PACKAGE,
Do we need a prefix like CPU_TOPO_LEVEL_* or shorter CPU_TL_*
to avoid possible naming conflicts with other micros or enums?
Not sure..

Thanks,
Yanan
> +};
> +
>   typedef struct CPUCacheInfo {
>       enum CacheType type;
>       uint8_t level;
> @@ -1517,6 +1526,13 @@ typedef struct CPUCacheInfo {
>        * address bits.  CPUID[4].EDX[bit 2].
>        */
>       bool complex_indexing;
> +
> +    /*
> +     * Cache Topology. The level that cache is shared in.
> +     * Used to encode CPUID[4].EAX[bits 25:14] or
> +     * CPUID[0x8000001D].EAX[bits 25:14].
> +     */
> +    enum CPUTopoLevel share_level;
>   } CPUCacheInfo;
>   
>